Skip to content

feat(ai-review): dual-AI maintainer review + BYOK (Phase C)#652

Merged
JSONbored merged 6 commits into
mainfrom
feat/ai-review-byok
Jun 13, 2026
Merged

feat(ai-review): dual-AI maintainer review + BYOK (Phase C)#652
JSONbored merged 6 commits into
mainfrom
feat/ai-review-byok

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Phase C — AI agent review (Workers AI default + BYOK)

Adds a gittensor-aware AI maintainer review to the public PR path. It is opt-in and dormant by default — gated by per-repo aiReviewMode (default off) AND the existing AI_SUMMARIES_ENABLED / AI_PUBLIC_COMMENTS_ENABLED flags (both default false). Nothing changes in production until a maintainer enables it.

What it does

Two layers, both fail-safe (no AI / error / over-budget / unsafe output → no public text and no gate finding — gittensory never blocks because a model spoke):

  • Advisory notes — a concise maintainer-style write-up (assessment / suggestions / risks) rendered as an advisory Gittensory AI review section in the PR panel. Free Cloudflare Workers AI by default; the maintainer's frontier model when BYOK is configured.
  • Consensus defect (opt-in blocker) — a defect becomes a gate blocker only when aiReview: block is set and the two free Workers-AI models (gpt-oss-120b + nemotron, adapted from reviewbot's proven pair) independently agree on a high-confidence (≥0.9) critical defect. Still confirmed-contributor gated — only confirmed Gittensor contributors can ever be hard-blocked. BYOK never drives this path, so it never changes who can be blocked.

Config-as-code

Everything is controllable from .gittensory.yml (precedence yml > DB > defaults):

gate:
  aiReview:
    mode: advisory   # off | advisory | block
    byok: false

Also settable via the dashboard settings route and the generic settings: override.

BYOK (encrypted at rest)

  • New encryptSecret/decryptSecret (AES-256-GCM, PBKDF2-derived key, random per-write IV) — gittensory's first reversible-encryption primitive — keyed by a new TOKEN_ENCRYPTION_SECRET worker secret.
  • Isolated repository_ai_keys table (migration 0027) so ciphertext is never serialized by the settings surface. Stores ciphertext + iv + key_version + a display-only last4; the plaintext key is never persisted.
  • Write-only internal routes POST/GET/DELETE /v1/internal/repos/:owner/:repo/ai-keyGET returns only { configured, provider, last4, model }, never the key. The key is never logged and never reaches a public surface.

Privacy

Every public string (notes + defect title/detail) is forced through sanitizePublicComment; anything that trips the public/private boundary is dropped, not published. Every model call is metered against the shared daily neuron budget and audited (never with key material).

Operator setup (when enabling)

  1. wrangler secret put TOKEN_ENCRYPTION_SECRET (≥32 bytes) for BYOK.
  2. Flip AI_SUMMARIES_ENABLED + AI_PUBLIC_COMMENTS_ENABLED to true.
  3. Per repo: set aiReview.mode (and optionally a BYOK key via the dashboard/API).

Tests

  • Engine: gating, budget, advisory/consensus, dual-model fallback, parse/sanitize, BYOK provider dispatch.
  • Gate: ai_consensus_defect blocks only in block mode, only for confirmed contributors.
  • End-to-end: a confirmed contributor is blocked by a consensus defect through the real webhook path.
  • BYOK: AES round-trip, secret-free status surface, decrypt-at-call-time, write-only routes.
  • Full suite green; coverage holds above the 97% gate.

Notes

…layer

Plumb two new RepositorySettings fields through the DB (migration 0026),
the dashboard settings route, and the .gittensory.yml config-as-code layer:

- aiReviewMode: off | advisory | block (GateRuleMode), default off
- aiReviewByok: boolean, default false

Both are settable from .gittensory.yml via the friendly gate.aiReview
{ mode, byok } alias (wins) and the generic settings: override, resolved
through resolveEffectiveSettings (yml > DB > defaults). No behavior yet —
this is the config surface the AI review engine wires into next.
Adds the Workers-AI review engine and folds it into the public PR path,
all behind the opt-in aiReviewMode (default off) and the existing AI flags
(both default off — dormant until enabled):

- src/services/ai-review.ts: dual-model engine adapted from reviewbot's
  proven pair (gpt-oss-120b + nemotron, reliable fallbacks). Two layers:
  * advisory notes — a maintainer-style write-up (BYOK frontier model when
    a key is supplied, else free Workers AI);
  * consensus defect — a defect is reported ONLY when BOTH free Workers-AI
    models independently flag a high-confidence (>=0.9) critical defect.
    BYOK never drives this path, so it never changes who can be blocked.
  Every public string is forced through sanitizePublicComment; every call
  is metered against the shared daily neuron budget and audited.
- Gate: a new ai_consensus_defect finding becomes a hard blocker only when
  aiReview: block is opted in (advisory otherwise) — still confirmed-
  contributor gated by evaluateGateCheck.
- Processor: runAiReviewForAdvisory mutates the advisory with the consensus
  defect BEFORE the gate evaluates, and returns advisory notes for the panel.
  Fully fail-safe — disabled/non-confirmed/no-AI/error → no finding, no notes.
- Panel: an advisory 'Gittensory AI review' section (HTML-escaped, public-safe).

End-to-end test proves a confirmed contributor is blocked by a consensus
defect; coverage holds above the 97% gate. BYOK provider key storage lands
in the next commit (the engine already accepts a providerKey).
Lets a maintainer bring their own Anthropic/OpenAI key so the advisory AI
review is written by their frontier model instead of free Workers AI. The
consensus blocker always stays on the free Workers-AI pair, so BYOK never
changes who can be blocked.

- crypto.ts: encryptSecret/decryptSecret (AES-256-GCM, PBKDF2-derived key,
  random 12-byte IV per write) — gittensory's first reversible-encryption
  primitive. Keyed by a new TOKEN_ENCRYPTION_SECRET worker secret.
- New isolated repository_ai_keys table (migration 0027) so the ciphertext is
  never serialized by the repository-settings surface. Stores ciphertext + iv
  + key_version + a display-only last4; the plaintext key is never persisted.
- repositories.ts: getRepositoryAiKeyStatus (secret-free), upsertRepositoryAiKey
  (encrypts; throws if the secret is unconfigured), deleteRepositoryAiKey,
  getDecryptedRepositoryAiKey (decrypts at call time; null on any miss so the
  review silently falls back to Workers AI).
- Write-only internal routes POST/GET/DELETE /v1/internal/repos/:owner/:repo/ai-key
  — GET returns only { configured, provider, last4, model }, never the key.
- Processor decrypts the key only when aiReviewByok is on and passes it to the
  engine; the key is never logged or placed in usage/audit metadata.

The key is encrypted at rest, never returned by any GET, and never reaches a
public surface or log line.
… PAT policy

- .gittensory.yml + bundled manifest: a commented gate.aiReview example
  (mode + byok), kept byte-identical per the alignment test.
- CONTRIBUTING.md: clarify that a maintainer's own opt-in AI-provider key
  (encrypted, write-only, never returned) is a distinct credential class from
  the banned contributor GitHub PATs.
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #652 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 13, 2026
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 30 changed file(s) — two independent AI reviewers.

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
The PR adds a full AI maintainer‑review feature with optional BYOK keys, new DB tables, API endpoints, settings integration, UI rendering, and extensive tests. The implementation is largely clean and respects the public‑private sanitizer, but it introduces a privacy risk by sending PR diffs to AI models without confirming the repository is public, and the neuron‑budget estimation may under‑count actual model calls.

Suggestions

  • Add a guard before invoking any AI review (runAiReviewForAdvisory / runGittensoryAiReview) to ensure the repository is public (e.g. check a repo.private flag) or explicitly document that AI review is only safe for public repos.
  • Adjust the neuron‑budget estimation in runGittensoryAiReview to account for the possible fallback model calls and retries, or add a safety margin to avoid exceeding the daily budget after the calls have been made.
  • Consider adding a unit test that verifies AI review is skipped for a private repository (or when the new guard is in place).

Worth double-checking

  • Potential leakage of private code to external AI providers if AI review is enabled on private repositories.
  • Under‑estimated neuron usage could cause the service to exceed its configured daily budget.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
This PR implements Phase C of the AI review feature: dual-AI maintainer review with BYOK support. It adds database schema for AI review settings and encrypted API keys, new API routes for BYOK key management, AI review service with dual-model consensus checking, and integrates AI findings into the PR review process. The changes are extensive but well-structured, with proper encryption for BYOK keys, fail-safes for AI calls, and adherence to the public/private boundary via sanitization. Tests cover the new functionality.

Suggestions

  • Consider adding a comment in the migrations explaining why ai_review_byok is stored as INTEGER (0/1) rather than using SQLite's native BOOLEAN affinity for clarity.
  • In buildAiReviewDiff, the truncation message could include the exact character limit for better transparency (e.g., '… diff truncated at 60,000 chars').
  • The aiReviewGateMode in GateCheckPolicy defaults to 'advisory' - confirm this aligns with the intended behavior where AI never blocks by default (as stated in comments).
  • In runAiReviewForAdvisory, the AI call is wrapped in a try/catch that logs and returns undefined on error - ensure this doesn't silently hide configuration issues during debugging (maybe add a debug-level log).
  • The BYOK API routes return 503 when encryption secret is missing - verify this matches the project's error handling patterns for misconfiguration.

Worth double-checking

  • Ensure the encryption/decryption functions handle edge cases like ultra-long keys (though input is limited to 400 chars).
  • Verify the daily neuron budget calculation doesn't inadvertently allow bypass via many small PRs (the sumAiEstimatedNeuronsSince uses UTC day start).
  • Confirm that the AI review's public-safe sanitization doesn't inadvertently allow safe-looking strings that could still contain private terms via obfuscation (relies on sanitizePublicComment).

@ghost ghost added the gittensory-review label Jun 13, 2026

@superagent-security superagent-security Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superagent found 2 security concern(s).

Comment thread src/utils/crypto.ts
// AI-call time. The AES key is derived from the worker secret TOKEN_ENCRYPTION_SECRET via PBKDF2; a
// fresh random 12-byte IV is used per encryption so ciphertexts are unique and the GCM tag authenticates
// them. The plaintext key is never persisted, never logged, and never returned from the API.
const SECRET_KDF_SALT = new TextEncoder().encode("gittensory-secret-encryption-v1");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Fixed salt used in PBKDF2 key derivation for BYOK secrets

Reusing a hardcoded salt weakens key derivation resistance to precomputation.

Derive salt randomly per encryption and persist it alongside the ciphertext.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/utils/crypto.ts">
<violation number="1" location="src/utils/crypto.ts:49">
<priority>P2</priority>
<title>Fixed salt used in PBKDF2 key derivation for BYOK secrets</title>
<evidence>src/utils/crypto.ts defines SECRET_KDF_SALT as a constant string (&quot;gittensory-secret-encryption-v1&quot;) and passes it to crypto.subtle.deriveKey with PBKDF2. A fixed salt means the derived AES key is deterministic for the same master secret, weakening resistance to precomputation and rainbow-table attacks if the master secret is ever exposed or low-entropy.</evidence>
<recommendation>Replace the fixed salt with a cryptographically random salt (e.g., 16 bytes from crypto.getRandomValues) generated per call to encryptSecret. Store the salt alongside the IV and ciphertext so decryptSecret can reconstruct the key. Update the database schema or ciphertext format to include the salt if needed.</recommendation>
</violation>
</file>

Comment thread src/services/ai-review.ts
input.body ? `Description:\n${input.body.slice(0, 2000)}` : "Description: (none)",
"",
"Unified diff (truncated if large):",
input.diff.slice(0, 60000),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: PR diff sent to external AI models without redacting secrets

The unified diff containing raw PR changes is forwarded to third-party AI providers without stripping API keys, tokens, or other secrets.

Strip or mask high-entropy strings and known secret patterns from diffs before building the AI prompt.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/services/ai-review.ts">
<violation number="1" location="src/services/ai-review.ts:181">
<priority>P2</priority>
<title>PR diff sent to external AI models without redacting secrets</title>
<evidence>buildUserPrompt in src/services/ai-review.ts concatenates input.diff.slice(0, 60000) directly into the prompt sent to Anthropic, OpenAI, or Cloudflare Workers AI. The diff may contain committed secrets (e.g., API keys, database URLs, tokens). Without redaction or masking, these could be exfiltrated to external model providers or retained in model logs.</evidence>
<recommendation>Before including the diff in the AI prompt, run a redaction pass that masks high-entropy strings, known secret patterns (sk-..., ghp_..., etc.), and any values adjacent to keys like password, token, or secret. Alternatively, use a git-secrets style scanner or regex to replace matches with [REDACTED].</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jun 13, 2026
@JSONbored JSONbored self-assigned this Jun 13, 2026
JSONbored added a commit that referenced this pull request Jun 13, 2026
…get query (#657)

sumAiEstimatedNeuronsSince runs SELECT sum(estimated_neurons) WHERE status='ok'
AND created_at >= ? — verified via EXPLAIN QUERY PLAN on prod to be a full
`SCAN ai_usage_events` (no covering index; existing indexes are on feature/actor).
It runs on every AI review/summary (the daily-budget check) and the table grows
with each AI call, so the scan cost grows unbounded once AI is enabled.

Add the covering index (status equality + created_at range). EXPLAIN confirms
the query now SEARCHes via it. Investigation found this is the ONLY un-indexed
read in the codebase — the per-webhook/gate hot path (issues/pull_requests/
files/bounties by repo, official_miner_detections by login, settings/repos by
PK) is already fully indexed, so the recent prod D1 overload is capacity/DB-size
related, not a missing-index problem on the hot path.

Migration numbered 0028 to avoid colliding with 0026/0027 reserved by #652.
# Conflicts:
#	src/queue/processors.ts
#	test/unit/queue.test.ts
@JSONbored
JSONbored merged commit dab0ce4 into main Jun 13, 2026
7 of 8 checks passed
@JSONbored
JSONbored deleted the feat/ai-review-byok branch June 13, 2026 23:26
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
JSONbored added a commit that referenced this pull request Jun 14, 2026
…e BYOK routes (#664)

Makes BYOK AI review usable end-to-end by maintainers (backend for #662).

Config-as-code provider/model:
- RepositorySettings gains aiReviewProvider (anthropic|openai|null) + aiReviewModel
  (migration 0029), settable via .gittensory.yml gate.aiReview { provider, model }
  and the dashboard. The secret key is never in the yml.
- The engine uses the effective provider/model: the stored key's provider is
  authoritative; a declared provider must match it (else BYOK is skipped →
  Workers-AI fallback); a declared model overrides the stored/default model.

Maintainer self-serve (the missing enabler — keys can't live in repo secrets,
since a GitHub App can't read them):
- PUT /v1/repos/:owner/:repo/ai-review — set mode/byok/provider/model (merges
  onto current settings; preserves unrelated settings).
- POST/GET/DELETE /v1/repos/:owner/:repo/ai-key — write-only encrypted key;
  GET returns only {configured, provider, last4, model}.
- All session-authenticated + scoped to repos the maintainer owns/maintains
  (requireRepoMaintainer helper) and allowlisted in canSessionAccessPath.
  Fixes a real gap: the new repo subpaths must be added to the session path
  allowlist or every maintainer is wrongly 403'd.

The internal-token routes from #652 remain for operator/backend use. Coverage
holds above the 97% gate; alignment + OpenAPI + Workers checks pass.

Closes #662
@github-actions github-actions Bot mentioned this pull request Jun 14, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:flagged PR flagged for review by security analysis.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant