diff --git a/CONVERGENCE_RUNBOOK.md b/CONVERGENCE_RUNBOOK.md new file mode 100644 index 0000000000..c415a0fe7e --- /dev/null +++ b/CONVERGENCE_RUNBOOK.md @@ -0,0 +1,110 @@ +# Convergence migration runbook (reviewbot → gittensory) +_Salvaged from the analysis workflow 2026-06-22. 5 stages A–E, each with a verification gate._ + + + +## gittensory current-state re-ground (the integration map) + +**Summary:** Re-grounded the gittensory host at /Users/shadowbook/Documents/.gittensory-convergence (worktree at main #988, 363c0fb9). The reviewbot engine plugs into ONE function: maybePublishPrPublicSurface in src/queue/processors.ts — that is where the gate verdict (evaluateGateCheck) is produced (processors.ts:1512) AND where the public comment is posted via createOrUpdatePrIntelligenceComment (processors.ts:1621), and the body it posts is buildPublicPrIntelligenceComment (processors.ts:1619 / signals/engine.ts:3926). The cleanest cutover seam is: keep gittensory's gate/settings/check-run as-is, but replace the deterministicBody at processors.ts:1619 with renderUnifiedReviewComment(reviewbot input, gittensory ctx) — the renderer is already built in reviewbot's src/core/unified-comment-render.ts and exported from engine.ts:30. Bindings: D1/Queue(+DLQ)/DO/AI are present; KV(REVIEW_CONFIG), Vectorize, R2(AUDIT), Browser, and a dedicated REVIEW_QUEUE/LOCK-DO are ABSENT and must be added or adapted. Settings are a flat RepositorySettings object resolved by resolveRepositorySettings (.gittensory.yml > DB > defaults); a convergence feature flag is read exactly like the existing gate modes. Tests run on vitest with an in-memory node:sqlite D1 (test/helpers/d1.ts) that auto-applies migrations/*.sql — there is NO existing processGitHubWebhook integration test, so a new one is needed for verification. + +- **(1a) Webhook→queue flow: where the gate verdict is produced:** handleGitHubWebhook (src/github/webhook.ts:8) verifies the signature, dedups via getWebhookEvent (webhook.ts:39-46), records the event, and enqueues a JobMessage {type:"github-webhook"} onto env.JOBS (webhook.ts:65). The queue consumer calls processJob (processors.ts:180) → case "github-webhook" (processors.ts:309) → processGitHubWebhook (processors.ts:827). For a PR event (processors.ts:947 `if (payload.repository?.full_name && payload.pull_request)`), it upserts the PR (processors.ts:949), resolves [repo, settings, otherOpenPullRequests] (processors.ts:950-954), builds the advisory via buildPullRequestAdvisory (processors.ts:955), then calls maybePublishPrPublicSurface (processors.ts:964). The GATE VERDICT is produced INSIDE that function: gateEvaluation = evaluateGateCheck(advisory, gatePolicy) at processors.ts:1512, where gatePolicy = gateCheckPolicy(settings, readiness.total, confirmedContributor, slopRisk, authorHistory) at processors.ts:1511. The verdict is returned up to processGitHubWebhook and reused by maybeRunAgentMaintenance (processors.ts:984). +- **(1b) Where the public comment is POSTED:** Still inside maybePublishPrPublicSurface: when decision.willComment (processors.ts:1614), it loads reviewConfig from the focus manifest (processors.ts:1617), assembles commentArgs (processors.ts:1618) INCLUDING the gate verdict and the AI review notes (`gate: gateEvaluation, review: reviewConfig, aiReview`), builds the body deterministicBody = buildPublicPrIntelligenceComment(commentArgs) (processors.ts:1619), then POSTS via createOrUpdatePrIntelligenceComment(env, installationId, repoFullName, pr.number, deterministicBody) at processors.ts:1621. THIS line is the single cutover point — swap deterministicBody for the unified renderer output. A second call exists at processors.ts:1339 for the closed-PR skip panel (buildClosedPrPanelUpdate, processors.ts:1282). createOrUpdatePrIntelligenceComment itself (src/github/comments.ts:21) is a find-by-marker upsert keyed on `` (comments.ts:4) under the bot login — so the unified comment must keep that marker as its first line (reviewbot's renderer currently emits a `> [!ALERT]` alert as line 1, NOT the marker — the host must prepend the marker or the upsert will create a duplicate). +- **(2) Existing gate + AI review structured output:** GATE — evaluateGateCheck (src/rules/advisory.ts:329) returns GateCheckEvaluation { enabled, conclusion: "success"|"failure"|"action_required"|"neutral"|"skipped", title, summary, blockers: AdvisoryFinding[], warnings: AdvisoryFinding[] } (advisory.ts:54-61). It operates on an Advisory whose findings are AdvisoryFinding { code, severity:"info"|"warning"|"critical", title, detail, action?, publicText? }. Only confirmed contributors can be hard-blocked (advisory.ts:354); newcomer grace softens to neutral (advisory.ts:372). AI REVIEW — runGittensoryAiReview (src/services/ai-review.ts:341) returns GittensoryAiReviewResult, the ok variant being { status:"ok", advisoryNotes: string|null, consensusDefect: {title,detail,confidence}|null, estimatedNeurons } (ai-review.ts:78). The wrapper runAiReviewForAdvisory (processors.ts:1149) MUTATES advisory.findings by pushing an `ai_consensus_defect` critical finding (processors.ts:1185-1192) BEFORE the gate runs, and returns {notes} for the panel. Dual free Workers-AI pair is gpt-oss-120b + nemotron (ai-review.ts:26), consensus floor 0.9 (ai-review.ts:35) — same pair/architecture as reviewbot, so reviewbot's review can either replace this or feed it. +- **(3) Settings resolution + how a convergence feature flag is read:** resolveRepositorySettings (src/settings/repository-settings.ts:7) returns the EFFECTIVE RepositorySettings by overlaying the parsed .gittensory.yml manifest onto DB settings via resolveEffectiveSettings (precedence: .gittensory.yml > DB > defaults). RepositorySettings (src/types.ts:404-483) is a FLAT object — relevant fields: gateCheckMode "off"|"enabled" (types.ts:411), gatePack "gittensor"|"oss-anti-slop" (types.ts:414), the five GateRuleMode ("off"|"advisory"|"block") gates linkedIssueGateMode/duplicatePrGateMode/qualityGateMode/slopGateMode/mergeReadinessGateMode/manifestPolicyGateMode (types.ts:415-428), aiReviewMode (types.ts:443), aiReviewByok (types.ts:448), publicSurface (types.ts:459), commentMode (types.ts:406), publicAudienceMode (types.ts:407). A convergence flag (e.g. `unifiedReviewMode: "off"|"shadow"|"on"` or reuse aiReviewMode) is read exactly like these: add the field to RepositorySettings, default it in the DB layer + resolveEffectiveSettings, then branch on settings. at processors.ts:1619 to choose buildPublicPrIntelligenceComment vs renderUnifiedReviewComment. No new resolver plumbing is needed — settings is already passed into maybePublishPrPublicSurface (processors.ts:1305). +- **(4) wrangler.jsonc bindings — present vs reviewbot's needs:** PRESENT (wrangler.jsonc): AI binding (line 56-58), D1 `DB` (59-66, db b2c79dd6…), Durable Object `RATE_LIMITER`/class RateLimiter (67-74) + migration tag (75-80), Queue producer `JOBS`→gittensory-jobs (81-87) and consumer with max_retries 3 + dead_letter_queue gittensory-jobs-dlq (88-99), cron */30 (101-103). Env (src/env.d.ts) declares DB, JOBS, RATE_LIMITER?, AI?, GITHUB_* secrets, GITTENSORY_API_TOKEN/MCP_TOKEN, INTERNAL_JOB_TOKEN, TOKEN_ENCRYPTION_SECRET. ABSENT (reviewbot needs): KV `REVIEW_CONFIG` — ABSENT (gittensory uses D1 settings + .gittensory.yml; reviewbot's KV-stored per-project config/guardrail globs have no home — must map to D1/manifest or add a KV namespace); Vectorize (RAG/content-index) — ABSENT; R2 `AUDIT` — ABSENT (gittensory audits to D1 via recordAuditEvent, processors.ts:171); Browser (screenshot/visual capture) — ABSENT; dedicated `REVIEW_QUEUE`+DLQ — ABSENT but the single multiplexed `JOBS` queue (+gittensory-jobs-dlq) can carry a new review JobMessage type; `LOCK` DO (reviewbot's SubmissionLock) — ABSENT, only RATE_LIMITER DO exists, so a lock DO class+binding must be added for any per-PR mutex. Net: the comment/gate path needs NO new binding (D1+AI+Queue suffice); RAG, visual capture, R2 audit, KV config, and the lock DO are the gaps. +- **(5) Local test setup for verification:** scripts: test=`vitest run`, typecheck=`tsc --noEmit`, dev=`wrangler dev`. vitest.config.ts: node env, globals on, include `test/**/*.test.ts`, EXCLUDES test/workers/** (those are workers-pool tests run separately), coverage v8 with a loose 90% global backstop (Codecov patch gate is the real 97% bar; COVERAGE_NO_THRESHOLDS disables for sharding). Test D1 = TestD1Database in test/helpers/d1.ts — an in-memory node:sqlite (DatabaseSync) that auto-applies every migrations/*.sql on construction and shims .prepare/.bind/.first/.all/.run, so tests exercise REAL SQL against REAL migrations. Fixtures live in test/fixtures (only local-scorer today) + test/stubs (cloudflare-email, cloudflare-workers aliased in vitest.config). GAP: there is NO existing integration test that drives processGitHubWebhook / maybePublishPrPublicSurface / buildPublicPrIntelligenceComment end-to-end (grep found only signals unit tests referencing the names). Verification command per step: `npm run typecheck` then `npm test`; for the comment specifically, add a unit test that calls renderUnifiedReviewComment with a fixed input and snapshot-asserts the marker + table, runnable via `npx vitest run test/unit/.test.ts`. +- **(6) Readiness-signal computations mapping to the unified comment table rows:** gittensory's table rows are built in buildPublicPrIntelligenceComment (signals/engine.ts:4020-4030) as allRows with stable keys, each toggleable via .gittensory.yml review.fields: `linkedIssue` (linkedIssuePanelResult, engine.ts:4021) ← pr.linkedIssues; `relatedWork` (relatedWorkPanelResult, engine.ts:4022) ← linkedIssueDuplicatePullRequests + unionScopedOverlapClusters over the CollisionReport; `reviewLoad`/change_scope, `validationEvidence`/validation, `openPrQueue`/queue_pressure — all from buildPublicReadinessScore (engine.ts:3832) components (traceability/related_work/change_scope/validation/pr_state/queue_pressure, engine.ts:3846-3905); `contributorContext` (contributorContextPanelResult, engine.ts:4028) ← profile+detection; `gateResult` (engine.ts:4029) ← the gate conclusion. These map to the unified renderer's UnifiedSignalRow[] (unified-comment-render.ts:51, {label,state:ok|warn|fail,result?,evidence?}) passed as ctx.signals — the engine prepends its own "Code review" row (signalTable, unified-comment-render.ts:189-204), so the host must convert each gittensory row's result-icon to ok/warn/fail and pass readinessScore=readiness.total (engine.ts:3907) as ctx.readinessScore. Validation/CI specifically maps to reviewbot's MergeReadiness {ciState:"passed"|"failed"|"unverified", mergeStateLabel?} (advisory-render.ts:11-12) consumed in statusChips (unified-comment-render.ts:142-146) and deriveUnifiedStatus (unified-comment-render.ts:113). + +**Steps:** + - Add a convergence feature flag to RepositorySettings (src/types.ts:404) defaulted in the DB layer + resolveEffectiveSettings; verify: npm run typecheck + - Import reviewbot's renderUnifiedReviewComment + buildUnifiedReviewInput (from the reviewbot engine, exported at src/engine.ts:30) into the gittensory worker and branch on settings. at processors.ts:1619 to choose it over buildPublicPrIntelligenceComment; PREPEND the `` marker so the comments.ts:68 upsert finds it; verify: npm run typecheck + - Map gittensory's allRows (engine.ts:4020) + readiness.total (engine.ts:3907) + gate conclusion (processors.ts:1512) into UnifiedCommentContext.signals/readinessScore and the gate verdict into UnifiedReviewInput.decision; verify: new vitest unit snapshot of renderUnifiedReviewComment output + - Add the absent infra incrementally ONLY as features land: a LOCK DO class+binding for per-PR mutex; map reviewbot KV REVIEW_CONFIG to D1/.gittensory.yml; defer Vectorize/R2/Browser until RAG/visual capture converge; verify each: npm test + - Write the first processGitHubWebhook→comment integration test (none exists) using TestD1Database (test/helpers/d1.ts) + a stub AI binding; verify: npx vitest run on the new test, then full npm test + npm run typecheck before any push + +**Risks:** + - Marker mismatch: reviewbot's renderUnifiedReviewComment emits a `> [!ALERT]` block as line 1, but createOrUpdatePrIntelligenceComment upserts by the literal `` marker (comments.ts:4,68). Without prepending the marker the host will POST a DUPLICATE comment every webhook instead of updating in place. + - No existing end-to-end test for processGitHubWebhook/maybePublishPrPublicSurface/buildPublicPrIntelligenceComment — the comment/gate path is currently only covered indirectly. Cutover changes there are unverified until a new integration test is written; high regression risk on the single most user-visible surface. + - Binding gaps are silent: KV REVIEW_CONFIG, Vectorize, R2 AUDIT, Browser, and a LOCK DO are all absent from wrangler.jsonc. Any reviewbot code path that dereferences env.REVIEW_CONFIG / env.AUDIT / env.BROWSER / a lock DO will throw at runtime in the Worker (not at typecheck if Env is loosely typed). Each reviewbot feature must be checked for these before it is wired in. + - Double verdict / status divergence: the gate already mutates advisory.findings with ai_consensus_defect (processors.ts:1185) and produces its own conclusion (processors.ts:1512). If reviewbot's review ALSO decides a verdict, the unified comment must derive ONE status (deriveUnifiedStatus, unified-comment-render.ts:94) from a single authoritative source or contributors see conflicting signals. + - The closed-PR path posts a SEPARATE panel (buildClosedPrPanelUpdate at processors.ts:1282 via createOrUpdatePrIntelligenceComment processors.ts:1339 with createIfMissing:false). The unified renderer must also handle the closed/skipped state, or closed PRs will keep the old gittensory panel while open PRs show the unified one — an inconsistent surface. + +**Decisions:** + - Cutover is a single-line swap at processors.ts:1619 (deterministicBody) gated behind a settings flag — this preserves gittensory's gate, settings resolution, check-run, label, and audit paths untouched, satisfying the strangler-fig / behavior-preserving constraint. + - No new Cloudflare binding is required for the unified comment itself — D1 (DB) + AI + the JOBS queue already present cover the gate + dual-model review + comment post. KV/Vectorize/R2/Browser/LOCK-DO are deferred to the specific reviewbot features that need them. + - The unified comment must reuse gittensory's marker `` (comments.ts:4) as its first line so the existing find-by-marker upsert (comments.ts:68) updates in place — guaranteeing ONE in-place comment per the locked goal. + - Verification per step is `npm run typecheck` (tsc --noEmit) + `npm test` (vitest run) locally against the in-memory-SQLite TestD1Database that auto-applies real migrations — full local verification is achievable before any push/deploy, satisfying the local-first constraint. + + +## reviewbot compute/act SEAM — extract reviewTarget() from processTarget + +**Summary:** The seam is real but NOT where the task assumed. Today `src/core/runtime.ts` processTarget (line 529) interleaves three things: (1) PRE-REVIEW setup that mutates `config` (freeze/release-please skip, tunable override, private review-config overlay, setStatus reviewing) at lines 530-569; (2) COMPUTE — building `context` + getting the verdict via `config.capabilities.review(...)` (lines 584-629); (3) ACT — comment/approve/merge/close/label/notify (lines 635-1051). The CRITICAL finding: the structured trio the host needs (DualReviewNote[] reviews, MergeReadiness readiness) does NOT exist at the processTarget level — `capabilities.review()` returns ONLY a `GateDecision` (types.ts:332), and `reviews`/`readiness` are computed INSIDE the agent capability (applyNonContentGate in non-content-gate.ts) and folded down into GateDecision. So `processTarget` can cleanly return `{decision: GateDecision, context: ReviewContext, changedFiles: string[]}` byte-for-byte today, but `reviews`/`readiness` require a SEPARATE, additive change to the capability surface. The minimal behavior-preserving refactor is a pure cut, not a logic change: split processTarget at line ~630 into `reviewTarget()` (setup+compute, returns the structured result) and an `actOnReview()` (everything from line 635 down), with processTarget calling both. The live path stays byte-identical because no lines move across the compute/act boundary — only the function wrapper changes. + +- **The act path has exactly ONE live entry: consumeQueueMessage → reviewUnderLock → processTarget:** processTarget (runtime.ts:529) is private and called from exactly one place: reviewUnderLock at line 1113, inside consumeQueueMessage (line 1076). The cron sweepDueTargets (line 1240) does NOT call processTarget directly — it re-enqueues via queue(env).send (line 1291), so every review re-enters through consumeQueueMessage under the per-target lock (lines 1109-1136). This means there is a single chokepoint to preserve: as long as processTarget(env,config,fresh) keeps its exact current behavior, the live path is unchanged. reviewTarget can be introduced WITHOUT touching consumeQueueMessage, sweep, or the lock. +- **capabilities.review returns GateDecision only — reviews[] and readiness are NOT visible to processTarget today:** The capability signature (types.ts:332) is `review(target, context, ctx): Promise`. processTarget calls it at runtime.ts:611 and only ever holds `decision: GateDecision`. The DualReviewNote[] (`reviews`) and MergeReadiness (`readiness`) the host's buildUnifiedReviewInput needs live one layer deeper: they are produced inside applyNonContentGate (non-content-gate.ts:164, via AdvisoryResult.notes + the ciState arg) and the agent's advisory path, then COLLAPSED into GateDecision (it carries `reviewers` — the compact form — `commentBody`, `gateAction`, but NOT the raw DualReviewNote[] or a MergeReadiness object). buildUnifiedReviewInput (unified-comment-render.ts:264) explicitly takes `reviews: DualReviewNote[]` + `readiness?: MergeReadiness`. So returning these from reviewTarget is a genuine surface ADDITION, not a free byproduct of the cut. +- **changedFiles IS already available at the seam — from context, not a refetch:** After the prepareContext/grounding block (runtime.ts:584-598), `context.changedFiles` holds the file list (seeded from target.changedFiles at line 584, set by the classify phase via setChangedFiles, db.ts). buildUnifiedReviewInput accepts `changedFiles: string[] | number` and just takes .length. So reviewTarget can return `changedFiles: context.changedFiles ?? target.changedFiles ?? []` (or `context.prFiles?.map(f=>f.filename)`) with zero new I/O — the value the act path already used. +- **Exact function boundary: cut at line 630 (after the decision/cache/audit block, before the redactor):** COMPUTE ends and ACT begins between line 629 (end of the cache-decision + gate_decision audit block) and line 635 (`const redact = ...`). reviewTarget should own lines 530-629 (freeze skip, release-please skip, override+reviewConfig overlay, setStatus reviewing, runContextFor, auto-label, context build, grounding, capture, cached-decision reuse, review call, cacheDecision, gate_decision audit) and RETURN the structured result. Note three of those (the freeze/release-please early returns at 532-549, and the ignore early-return at 640-644) are TERMINAL no-act exits — reviewTarget must signal them (e.g. return a discriminated `{kind:'skip'}` / decision.verdict==='ignore') so processTarget knows to stop. The act half (lines 635-1051) plus the outer try/catch (1052-1072) wrap the call. The redactor (635-637) is ACT-side (it's only used by posting code), so it moves DOWN into the act half. +- **Proposed signature + return shape:** `export async function reviewTarget(env: Env, config: AgentConfig, target: LoadedTarget): Promise` where `interface ReviewResult { status: 'skip' | 'ignore' | 'defer' | 'decided'; config: AgentConfig; /* the OVERLAID config — see risk */ decision: GateDecision; context: ReviewContext; changedFiles: string[]; reviews: DualReviewNote[]; readiness?: MergeReadiness; reusedDecision: boolean; }`. The `status` discriminant captures the four control-flow exits processTarget has before the terminal-action block (frozen/release-please→skip, ignore→ignore, deferSeconds→defer, else→decided). processTarget then switches on it: skip/ignore/defer do exactly what lines 532-549 / 640-644 / 648-676 do today; decided runs the act half. Because reviewTarget RETURNS config (it reassigns it via applyOverrideToConfig at 556 + applyReviewConfigOverlay at 564), the act half uses the overlaid config exactly as today. +- **reviews[] + readiness must be threaded out of the capability — the one new wire:** To populate ReviewResult.reviews/readiness, applyNonContentGate (and the advisory composeUnifiedReview path) already HAVE both: `result.notes: ReviewNotes[]` + `result.failedReviewers` (AdvisoryResult, non-content-gate.ts:146-151) map directly to DualReviewNote[] (the same `[...notes.map(n=>({model:'',notes:n})), ...Array(failedReviewers).fill(null).map(()=>({model:'',notes:null}))]` shape already built at line 227 for composeUnifiedReview), and the `ciState`/`mergeStateLabel` args (line 167,170) map to MergeReadiness. The behavior-preserving way to expose them WITHOUT changing the verdict: have the capability stash them on the returned GateDecision (additive optional fields, e.g. `decision.reviews?`, `decision.readiness?`) OR widen capabilities.review to return `{decision, reviews, readiness}`. The former is lower-blast-radius (GateDecision already carries review-adjacent optional fields like `reviewers`, reviewBody at types.ts:240-244) and keeps the capability signature untouched. reviewTarget reads them off the decision and surfaces them on ReviewResult. +- **engine.ts export is a one-line addition to a re-export block:** engine.ts:16 already re-exports from ./core/runtime: `export { consumeQueueMessage, handleDeadLetter, handleWebhook, handleWebhookByRepo, reconcileErroredTargets, runCloseAudit, sweepDueTargets } from './core/runtime';`. Add `reviewTarget` to that list (and its ReviewResult type to the type-export block at engine.ts:31-40). The host (gittensory) then imports `reviewTarget` to COMPUTE, calls buildUnifiedReviewInput(opts) (already exported, engine.ts:30) with the returned {changedFiles, reviews, readiness, decision}, renders via renderUnifiedReviewComment, and posts via its own createOrUpdatePrIntelligenceComment — the engine never posts. + +**Steps:** + - Step 1 — Add the ReviewResult type. In src/core/runtime.ts, define `export interface ReviewResult { status: 'skip'|'ignore'|'defer'|'decided'; config: AgentConfig; decision: GateDecision; context: ReviewContext; changedFiles: string[]; reviews: DualReviewNote[]; readiness?: MergeReadiness; reusedDecision: boolean; }`. Import DualReviewNote from ./ai-review and MergeReadiness from ./advisory-render. VERIFY: `npm run typecheck`. + - Step 2 — Thread reviews+readiness out of the capability (additive, verdict-unchanged). In non-content-gate.ts applyNonContentGate, attach the already-built DualReviewNote[] (the `[...notes, ...failed]` array constructed at line 227) and a MergeReadiness `{ciState, mergeStateLabel: opts?.mergeStateLabel}` to the returned GateDecision as new OPTIONAL fields `reviews?`/`readiness?` on GateDecision (types.ts:219). These are ignored by every existing act-path read, so behavior is unchanged. VERIFY: `npm run test` (the existing non-content-gate + unified-review suites must stay green) then `npm run typecheck`. + - Step 3 — Extract reviewTarget. Move runtime.ts lines 530-629 verbatim into `export async function reviewTarget(env, config, target): Promise`. Convert the three early returns: frozen/release-please (532-549) → `return {status:'skip', ...}`; the ignore branch (move the verdict==='ignore' detection here) → `return {status:'ignore', ...}`; deferSeconds present → `return {status:'defer', ...}`; else `return {status:'decided', decision, context, changedFiles: context.changedFiles ?? target.changedFiles ?? [], reviews: decision.reviews ?? [], readiness: decision.readiness, reusedDecision, config}`. Do NOT move any act-side line (redactor at 635 stays in the act half). VERIFY: `npm run typecheck`. + - Step 4 — Rebuild processTarget as a thin caller. processTarget calls `const r = await reviewTarget(env, config, target)` then switches on r.status: skip→(setStatus ignored + audit, exactly lines 545-548)/return; ignore→(lines 641-643)/return; defer→(lines 648-676 body, using r.decision/r.config)/return; decided→run the act half (the EXISTING lines 635-1051) with `config = r.config`, `decision = r.decision`, `context = r.context`. Keep the outer try/catch (1052-1072) wrapping the whole thing. The act-half code is moved unchanged (cut/paste, no edits). VERIFY: `npm run test` — the conflict-selfheal.test.ts processTarget suites (test/conflict-selfheal.test.ts:114,150 — release-please no-action + forced re-trigger) and runtime-reconcile.test.ts must pass unchanged, PROVING the live act path is byte-identical. + - Step 5 — Export via engine.ts. Add `reviewTarget` to the `export { ... } from './core/runtime'` list at engine.ts:16 and `ReviewResult` to the type block (engine.ts:31-40). VERIFY: `npm run test` (test/engine.test.ts:7 asserts the runtime surface — extend it to assert `reviewTarget` is exported) then the FULL gate `npm run check` (typecheck + biome lint + coverage) must pass with no behavior diff. + +**Risks:** + - GateDecision is CACHED to D1 (cacheDecision, runtime.ts:615) and parsed back (parseCachedDecision, line 483, VALID_CACHED_VERDICTS at 477). If reviews[]/readiness are added as GateDecision fields, they get serialized into decision_json — bloating the cache and, on cache-reuse (line 606-609), the reused decision will carry STALE reviews/readiness from the original commit. Mitigation: either strip reviews/readiness before cacheDecision (JSON.stringify a slimmed copy) OR recompute readiness fresh on the reuse path. The host's unified comment on a cache-reuse re-review (e.g. a later CI webhook) must get CURRENT CI readiness, not the cached snapshot — mirror the existing renderCaptureTable pattern (line 358) which is deliberately rendered FRESH at post time, never baked into the cache. + - On the cached-decision reuse path (reusedDecision=true), reviewTarget will have NO fresh DualReviewNote[] (the capability.review call is skipped at line 607). reviews[] will be whatever was cached (or empty). The host must tolerate reviews=[] / reviewerCount=0 on a reused decision — buildUnifiedReviewInput already does (reviewerCount derives from reviews.filter(r=>r.notes), and renderUnifiedReviewComment hides the chip at 0). Confirm the host doesn't hard-require reviews on reuse. + - applyNonContentGate is ONE gate path; the advisory-only agents (awesome-claude content lane, etc.) build their GateDecision elsewhere and may NOT populate decision.reviews/readiness. reviewTarget must default reviews:[] / readiness:undefined so those lanes still return a valid ReviewResult (the host then renders an advisory-status comment with no Code-review blockers). Audit each capabilities.review implementation to decide which ones should populate the new fields — for the 3 converging repos (gittensory/awesome-claude/metagraphed) verify each lane's review() either sets them or is acceptably empty. + - The move must be a pure cut: if any line in the act half (635-1051) is accidentally edited (not just relocated), the byte-identical guarantee breaks and only coverage diffs would catch it. Do step 4 as a literal cut/paste and rely on `git diff` showing ONLY wrapper/indentation changes in the act region, plus the full test suite, as the proof. + +**Decisions:** + - The compute/act cut falls at runtime.ts line ~630 (end of the cache+audit block, before the `const redact` at 635). Everything above is reviewTarget (setup+compute); everything below + the outer try/catch is the act half. + - reviewTarget returns the OVERLAID AgentConfig (after applyOverrideToConfig:556 + applyReviewConfigOverlay:564), because the act half and the host both must use the same tunable/private-config view the review was computed under. Returning config is required for byte-identical behavior. + - reviews[]/readiness are surfaced as NEW OPTIONAL fields on GateDecision (set inside applyNonContentGate from data it already has: result.notes/failedReviewers + ciState/mergeStateLabel), NOT by widening the capabilities.review signature. This keeps the capability contract and every agent untouched — lowest blast radius. + - The four control-flow exits (frozen/release-please skip, ignore, defer, decided) become a `status` discriminant on ReviewResult so processTarget reproduces today's early-return behavior exactly; reviewTarget itself takes NO GitHub action. + - Verification that the live path is byte-unchanged = the existing suites (conflict-selfheal.test.ts processTarget tests at :114/:150, runtime-reconcile.test.ts, unified-review.test.ts) passing un-modified, plus `npm run check`. No test asserting a posted side-effect should change — that is the proof the act half moved without edits. + + +## Staged local migration runbook + adversarial completeness + +**Summary:** I read both trees on disk. The convergence worktree (/Users/shadowbook/Documents/.gittensory-convergence, branch convergence/reviewbot-migration, clean, at gittensory main #988) is the host; the reviewbot engine is at the cwd. The embed seam already exists and is sound: src/engine.ts re-exports everything the live Worker drives, the renderer (src/core/unified-comment-render.ts) is a pure function, and reviewbot core imports Env as a NAMED export from ./core/types (not a global), so vendoring keeps it self-contained. The single biggest blocker to "confirm it's perfect locally" is NOT logic but BUILD config: gittensory's tsconfig enables four strict flags reviewbot's does not (noUncheckedIndexedAccess, exactOptionalPropertyTypes, noImplicitOverride, noFallthroughCasesInSwitch), so vendored reviewbot source WILL throw hundreds of tsc errors under gittensory's typecheck unless scoped out. Second: there is NO published reviewbot npm package, so consumption must be vendored-source-under-a-subpath with its own tsconfig (path-import across worktrees and npm-dep are both unviable). Third: live-3-repo behavior depends on KV REVIEW_CONFIG + D1 review_audit + Vectorize that cannot exist locally, so "parity" locally is structural only, never decision-identical. Below is the ordered runbook with a verification command per stage, then a ranked risk list. + +- **Consumption model: must be vendored source under a subpath — npm-dep and path-import are both unviable:** There is NO published reviewbot package (package.json name 'reviewbot', private:true, version 0.1.0, no build/exports field, no dist). Options ranked: (1) VENDOR reviewbot src/{engine.ts,registry.ts,core,platform,agents} into /Users/shadowbook/Documents/.gittensory-convergence/vendor/reviewbot with its own tsconfig — RECOMMENDED, self-contained, no cross-worktree coupling. (2) npm dependency on a git URL — needs a real build/exports + the engine to compile under consumers; not set up. (3) TS path-import across worktrees (e.g. paths to ../reviewbot/src) — REJECT: couples the gittensory build to a sibling worktree path, breaks CI/clean-checkout, and re-lints reviewbot under gittensory's stricter tsconfig. engine.ts:20 imports ./agents/awesome-claude/content-rag and registry.ts imports all 3 agent configs, so vendoring MUST include the whole src/agents tree, not just core. +- **BUILD INCOMPATIBILITY is the dominant local blocker: 4 extra strict tsconfig flags on the gittensory side:** reviewbot tsconfig: strict only (target ES2022, moduleResolution Bundler, types [@cloudflare/workers-types]). gittensory tsconfig ADDS noUncheckedIndexedAccess, exactOptionalPropertyTypes, noImplicitOverride, noFallthroughCasesInSwitch, and types [vitest/globals, node, @cloudflare/vitest-pool-workers/types]. ~30 core files use optional props (?:) and array indexing; under exactOptionalPropertyTypes + noUncheckedIndexedAccess vendored reviewbot will throw many tsc errors if compiled by the host's root tsconfig. MITIGATION (Stage C2): vendor/reviewbot/tsconfig.json extends the host but turns those 4 flags OFF, and root tsconfig excludes vendor/** so `npm run typecheck` never lints vendored code under the stricter rules. Also note the types-array delta: reviewbot wants @cloudflare/workers-types ambient (D1Database/Vectorize/R2Bucket/KVNamespace/Ai used in core/lock.ts, core/rag.ts, core/repo-index.ts, core/types.ts); the vendor tsconfig must include that types entry. +- **Env type collision: reviewbot exports `interface Env` from core/types; gittensory declares `interface Env` GLOBALLY:** gittensory src/env.d.ts does `declare global { interface Env {...} }` (DB/JOBS/AI/...). reviewbot core imports Env as a NAMED type from ./types (e.g. ai-review.ts:4 `import type { Env } from "./types"`) — GOOD: vendored reviewbot keeps referencing its own Env, NOT gittensory's global. The risk is only if wiring code in gittensory tries to pass gittensory's global Env into a reviewbot function expecting reviewbot's Env (REVIEW_QUEUE, AUDIT_SOURCE, platform.adapters, REVIEW_CONFIG missing on the gittensory side). The renderer (renderUnifiedReviewComment/buildUnifiedReviewInput) takes NO Env, so Stage D wiring is Env-free and safe; only deeper engine calls (runtime/queue) would need an adapter Env, which is a later phase, not needed for the unified comment. +- **Codex agent is active in gittensory but NOT in the convergence worktree — conflict risk is on the shared branch, not the file tree:** git worktree list shows codex on 6 other worktrees (codex/pr-184-fix, codex/pr-207-fix, codex/review-automation-depth, codex/codecov-test-results, codex/fix-public-safe-remediation-plan-vulnerability) plus a claude worktree (feat/agent-784-cli-setlevel-mcp-propose). The convergence worktree is on its own branch convergence/reviewbot-migration at 363c0fb9 (#988), working tree CLEAN. Codex churns processors.ts/signals/scoring/comments heavily (recent log #988/#970/#946 touch signals+comments+notifications). RISK: when codex's branches merge to main, the convergence branch's edits to processors.ts (maybePublishPrPublicSurface) and comments.ts will need rebase, with a high collision probability precisely on the files Stage D edits. MITIGATION: keep Stage D edits minimal + flag-gated, rebase convergence onto main frequently, and do NOT let convergence land until the in-flight codex PRs touching processors.ts/comments.ts settle. +- **Live-3-repo behavior depends on KV/D1/Vectorize that cannot exist locally — 'parity' locally is structural only:** reviewbot's REAL reviewer prompts/knowledge load at runtime from KV REVIEW_CONFIG (per-agent slug override; missing key => committed SAMPLE config). Its gate decisions write to D1 review_audit tagged by AUDIT_SOURCE, and RAG uses Vectorize. NONE of these exist in a local D1/wrangler --local run. CONSEQUENCE: locally you can prove the comment RENDERS and the gate WIRES, but you cannot reproduce the live decision (the local engine uses sample config, not the private KV rubric). computeGateParity (eval.ts:181, floor 0.98, min sample 30) is explicitly a SHADOW-DEPLOY check over the shared audit store, not a local check. The runbook must state: local = structural/behavioral-shape parity; decision parity = post-deploy shadow with AUDIT_SOURCE='gittensory' before any per-repo cutover. +- **Workers-AI + GitHub calls cannot run locally — both gates' AI review and the comment publish are network-bound:** runGittensoryAiReview (processors.ts:1173, services/ai-review.ts:341) calls the AI binding (free Workers-AI gpt-oss-120b+nemotron or BYOK); reviewbot's reviewers likewise. createOrUpdatePrIntelligenceComment + createOrUpdateGateCheckRun (app.ts:123) call the GitHub App API with an installation token. Locally none of these are reachable. MITIGATION for Stage D/E: unit-test the wiring with STUBBED DualReviewNote[] fixtures and a stubbed installation token (gittensory already has this pattern — processJob tests use requestedBy:'test' short-circuits, e.g. processors.ts:186/206/292). End-to-end 'confirm perfect' for the AI + GitHub legs is only achievable on a shadow deploy, not locally. +- **vitest environment mismatch: reviewbot uses node pool, gittensory uses workers pool:** reviewbot vitest.config.ts sets environment:'node', include test/**/*.test.ts, coverage floors 70/62/71/71 over src/**. gittensory has vitest.config.ts AND vitest.workers.config.ts (test:workers runs the @cloudflare/vitest-pool-workers pool) and a big test:ci chain. reviewbot's 112 test files assume the node env + reviewbot's source layout; they should NOT be vendored into gittensory's test run (they'd pull reviewbot's whole src and break coverage include globs). Vendor ONLY src, not test; rely on NEW gittensory-side tests (Stage D/E) for the converged path. Keep reviewbot's own 112 tests running in the reviewbot repo as the engine's unit guarantee. +- **The embed seam itself is well-built and low-risk — the renderer is pure and the facade is complete:** engine.ts re-exports the full runtime surface (webhook/queue/dead-letter/index kinds/cron/HTTP/registry/config/adapters/SubmissionLock) plus the unified-comment trio. renderUnifiedReviewComment (unified-comment-render.ts:224) is a pure function: it only emits passed fields, no guardrail paths/thresholds, public-safe-by-construction, host redacts after. buildUnifiedReviewInput (line 264) reuses extractReviewSummary so the converged comment surfaces exactly reviewbot's own blockers/nits/summary/consensus — no divergent second synthesis. deriveUnifiedStatus (line 94) makes an explicit gate decision authoritative over reviewer recs, which is exactly the two-gate reconciliation Stage D needs. This means Stage D is mostly a MAPPING exercise (gittensory advisory+AI-result -> UnifiedReviewInput/Context), not new logic. + +**Steps:** + - STAGE A — Freeze the reviewbot compute/act seam (in reviewbot cwd, behavior-unchanged). Goal: prove engine.ts + the renderer are a clean, importable surface before touching gittensory. (A1) Read-confirm engine.ts re-exports everything the live index.ts drives (it does: runtime/alerts/auto-apply/repo-index/content-rag/db-prune/draft/ops/stats/lock/adapters/registry/review-config/unified-comment). (A2) Confirm the renderer is pure I/O-free: renderUnifiedReviewComment + buildUnifiedReviewInput in src/core/unified-comment-render.ts call only extractReviewSummary (src/core/advisory-render.ts) — no env, no fetch. (A3) Make NO edits. VERIFY: `cd /Users/shadowbook/Documents/reviewbot/.claude/worktrees/intelligent-hermann-715515 && npm run check` (typecheck + biome lint + coverage) is green and the live src/index.ts is byte-unchanged (`git status --short` clean). This proves Stage A added zero risk to the live worker. + - STAGE B — gittensory bindings + local D1 migrations (host worktree, no reviewbot code yet). Goal: stand up the host locally so later stages have a runnable target. (B1) `cd /Users/shadowbook/Documents/.gittensory-convergence && npm ci`. (B2) Apply local D1: `npm run db:migrate:local` (wrangler d1 migrations apply gittensory --local; current head is migrations/0045_agent_pending_actions.sql). (B3) Decide reviewbot's binding needs against gittensory's wrangler.jsonc: gittensory already has DB(D1), JOBS(queue), AI, RATE_LIMITER(DO) — but reviewbot's engine optionally wants AUDIT(R2), VECTORIZE, BROWSER, REVIEW_CONFIG(KV), LOCK(DO). For the OFF-flag path none are required (engine code is fail-safe without them); document that they stay UNBOUND in the convergence wrangler until a later cutover phase. VERIFY: `npm run typecheck && npm run test:unit` green on the untouched host, and `npx wrangler d1 migrations list gittensory --local` shows all 45 applied — establishes a clean baseline before embedding. + - STAGE C — Embed reviewbot/engine in gittensory behind a default-OFF flag (host worktree). Goal: reviewbot source compiles and ships inside gittensory but executes nothing until flipped. (C1) VENDOR, do not npm-link: copy reviewbot's src/{engine.ts,registry.ts,core/**,platform/**,agents/**} into a quarantined subpath e.g. /Users/shadowbook/Documents/.gittensory-convergence/vendor/reviewbot/. engine.ts pulls ./agents/awesome-claude/content-rag and registry.ts pulls all three agent configs, so the agents tree comes with it. (C2) Add vendor/reviewbot/tsconfig.json that EXTENDS gittensory's but DISABLES the four extra flags (noUncheckedIndexedAccess/exactOptionalPropertyTypes/noImplicitOverride/noFallthroughCasesInSwitch:false) and sets types:["@cloudflare/workers-types"]; add `"references"` or an `"exclude": ["vendor/**"]` to the root tsconfig so the host typecheck does NOT re-lint vendored code under its stricter flags. (C3) Add an env flag REVIEWBOT_ENGINE_ENABLED (default unset/"false") to src/env.d.ts and the convergence wrangler vars; gate every new call site on `env.REVIEWBOT_ENGINE_ENABLED === "true"`. (C4) Do NOT call the engine yet — only import-type. VERIFY: `cd /Users/shadowbook/Documents/.gittensory-convergence && npm run typecheck && npm run test` green, AND grep proves the flag default is OFF so processGitHubWebhook behavior is byte-identical (no new runtime branch taken). This is the gate that proves OFF = zero behavior change. + - STAGE D — Two-gate reconciliation + unified-comment wiring (host worktree, still flag-gated). Goal: when the flag is ON, gittensory's existing Gate + dual-model review (runGittensoryAiReview, processors.ts:1173) feeds reviewbot's renderer to emit ONE comment. (D1) In maybePublishPrPublicSurface (src/queue/processors.ts:1299) add a flag-gated branch: map gittensory's advisory + AI-review result into buildUnifiedReviewInput({changedFiles, reviews, readiness, decision, merged}); map gittensory readiness signals into UnifiedCommentContext.signals + footer (gittensoryFooter, src/github/footer.ts). (D2) Reconcile the two gates: gittensory's evaluateGateCheck (src/rules/advisory.ts) stays AUTHORITATIVE for the Gittensory Gate check-run (app.ts:123); reviewbot's verdict maps into deriveUnifiedStatus only for the COMMENT color — pass decision so an explicit gate verdict overrides reviewer recs (the renderer already does this, unified-comment-render.ts:97). (D3) Replace the body passed to createOrUpdatePrIntelligenceComment (comments.ts:21, marker gittensory-pr-panel:v1) with renderUnifiedReviewComment output; apply gittensory's existing public-safe redaction (sanitizePublicComment, src/github/commands.ts) AFTER — the renderer is pure/not-redacted by design. VERIFY: a new vitest unit test in test/unit that (a) calls the wiring with a fixture advisory+DualReviewNote[] and asserts deriveUnifiedStatus/decision precedence, and (b) snapshots renderUnifiedReviewComment for a fixture PR producing one in-place comment in gittensory shape with reviewbot's Code-review row. Run `npm run test:unit -- ` green; assert the comment contains exactly one PR_PANEL_COMMENT_MARKER. + - STAGE E — Local end-to-end verification + parity (host worktree). Goal: prove the whole converged path runs locally and is structurally parity-correct, accepting that decision-identical parity needs live KV/D1/Vectorize. (E1) Run the host's full CI locally with the flag OFF then ON: `npm run typecheck && npm run test:coverage && npm run test:workers`. (E2) Drive a fixture github-webhook JobMessage through processJob (processors.ts:180 -> processGitHubWebhook:827) in a vitest workers-pool test with a stubbed installation token; assert with flag OFF the legacy deterministic body is posted and with flag ON the unified body is posted, and the Gittensory Gate conclusion is identical in both (gate stays authoritative). (E3) Parity tooling: reviewbot's computeGateParity (src/core/eval.ts:181, floor PARITY_AGREEMENT_FLOOR 0.98, MIN_PARITY_SAMPLE 30) compares AUDIT_SOURCE-tagged rows in the SHARED review_audit store — note in the runbook this is a POST-deploy shadow check, NOT runnable locally (no shared D1, no live commits). VERIFY: `cd /Users/shadowbook/Documents/.gittensory-convergence && npm run test:ci` green end-to-end; document that live parity is gated on a shadow deploy writing AUDIT_SOURCE='gittensory' before any per-repo cutover. + +**Risks:** + - RANK 1 (build): vendored reviewbot fails tsc under gittensory's 4 extra strict flags (exactOptionalPropertyTypes, noUncheckedIndexedAccess, noImplicitOverride, noFallthroughCasesInSwitch). Blocks Stage C green. MUST quarantine with a vendor tsconfig + root exclude vendor/**. + - RANK 2 (consumption): no published reviewbot package and no build/exports — the ONLY viable local path is vendored source; getting this wrong (npm-dep or cross-worktree path-import) breaks clean-checkout CI. Decided: vendor under vendor/reviewbot. + - RANK 3 (merge): codex agent active across 6 gittensory worktrees churns the exact files Stage D edits (processors.ts maybePublishPrPublicSurface, comments.ts). High rebase-collision probability on cutover; keep Stage D edits minimal+flag-gated and rebase often. + - RANK 4 (parity illusion): local verification proves comment SHAPE + gate WIRING only, never live decision parity (private KV rubric, shared review_audit, Vectorize absent locally). Stating 'perfect locally' must explicitly scope to structural correctness; decision parity is a post-deploy shadow (computeGateParity floor 0.98 / min 30) before per-repo cutover. + - RANK 5 (network legs uncovered): Workers-AI review + GitHub check/comment publish can't run locally; Stage D/E must stub DualReviewNote[] + installation token (use gittensory's existing requestedBy:'test' short-circuit pattern). The AI/GitHub legs are only end-to-end-verifiable on a shadow deploy. + - RANK 6 (regression on flag-ON): Stage D changes the body of the single PR_PANEL comment; if the gate-vs-renderer status precedence is mis-mapped, the comment could show a verdict diverging from the authoritative Gittensory Gate check. Mitigate with the Stage D unit test asserting decision overrides recs and exactly one marker. + - RANK 7 (binding gaps): reviewbot engine optionally wants AUDIT(R2)/VECTORIZE/REVIEW_CONFIG(KV)/LOCK(DO)/BROWSER that the convergence wrangler does not bind. Fail-safe for the renderer path (no Env needed) but any DEEPER engine call (runtime/queue/RAG/lock) would silently degrade or error if invoked before those bindings are added — keep deeper engine OFF until a later phase. + - RANK 8 (Env type leak): if wiring passes gittensory's GLOBAL Env into a reviewbot function typed against reviewbot's exported Env (REVIEW_QUEUE/AUDIT_SOURCE/REVIEW_CONFIG/platform missing), tsc errors. The renderer is Env-free so Stage D is safe; this risk only materializes if a later phase calls runtime/queue handlers — needs an adapter Env or a platform-adapter bridge. + +**Decisions:** + - Consume reviewbot as VENDORED SOURCE under /Users/shadowbook/Documents/.gittensory-convergence/vendor/reviewbot (engine.ts + registry.ts + core/** + platform/** + agents/**), NOT an npm dependency and NOT a cross-worktree path-import — there is no published package and the agents tree is a hard transitive dependency of engine.ts/registry.ts. + - Give the vendored tree its OWN tsconfig that disables the 4 strict flags gittensory adds, and exclude vendor/** from the host root tsconfig, so gittensory's `npm run typecheck` stays green without rewriting reviewbot source. + - Gate every new call site on a default-OFF env flag (REVIEWBOT_ENGINE_ENABLED !== 'true' => byte-identical legacy behavior); OFF is the verification baseline at each stage. + - Keep gittensory's evaluateGateCheck authoritative for the Gittensory Gate check-run; use reviewbot's deriveUnifiedStatus only to color the single unified COMMENT, with the explicit gate decision overriding reviewer recs (the renderer already enforces this). + - Wire the unified comment purely through renderUnifiedReviewComment/buildUnifiedReviewInput (Env-free, pure) and apply gittensory's existing sanitizePublicComment redaction AFTER — do not invoke reviewbot's deeper runtime/queue/RAG engine in this phase. + - Scope local verification to structural parity (comment shape + gate wiring + flag-OFF no-op), and defer decision-parity to a post-deploy shadow that writes AUDIT_SOURCE='gittensory' into the shared review_audit store, checked by computeGateParity (floor 0.98, min sample 30) per-repo before any cutover. + - Do not vendor reviewbot's 112 test files into gittensory; rely on new host-side Stage D/E tests for the converged path and keep reviewbot's own tests as the engine's unit guarantee in the reviewbot repo. + - Rebase convergence/reviewbot-migration onto gittensory main frequently and hold the merge until in-flight codex PRs touching processors.ts/comments.ts settle, to minimize the high-probability collision on the exact Stage D edit sites. diff --git a/src/env.d.ts b/src/env.d.ts index c5e11d5bbc..cbbfef7ad6 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -46,6 +46,10 @@ declare global { TOKEN_ENCRYPTION_SECRET?: string; RATE_LIMIT_TRUSTED_PROXIES?: string; RATE_LIMIT_TRUSTED_PROXY_COUNT?: string; + /** Convergence (Stage D): when truthy, the public PR comment is rendered by the unified-comment bridge + * (ONE in-place comment in the converged shape) instead of the legacy `buildPublicPrIntelligenceComment` + * panel. Default OFF — unset/false keeps the legacy panel byte-identical. */ + UNIFIED_REVIEW_COMMENT?: string; } } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ba2b5adcca..4ecbffa331 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -136,6 +136,7 @@ import { buildMaintainerLaneReport, buildPreflightResult, buildPublicPrIntelligenceComment, + buildPublicPrPanelSignalRows, buildPublicReadinessScore, buildQueueHealth, buildRoleContext, @@ -144,6 +145,7 @@ import { unionScopedOverlapClusters, type ContributorProfile, } from "../signals/engine"; +import { buildUnifiedCommentBody, isUnifiedReviewCommentEnabled } from "../review/unified-comment-bridge"; import { buildIssueSlopAssessment, buildSlopAssessment, type SlopBand } from "../signals/slop"; import { runGittensoryAiSlopAdvisory } from "../services/ai-slop"; import { decidePublicSurface } from "../signals/settings-preview"; @@ -1616,7 +1618,31 @@ async function maybePublishPrPublicSurface( // Cached, so this is a DB read after the settings resolution already loaded the manifest. const reviewConfig = (await loadRepoFocusManifest(env, repoFullName)).review; const commentArgs = { repo, pr, profile, detection, queueHealth, collisions, preflight, settings, gate: gateEvaluation, review: reviewConfig, aiReview }; - const deterministicBody = buildPublicPrIntelligenceComment(commentArgs); + let deterministicBody: string; + // Convergence (Stage D): when the unified-review-comment flag is ON, render the single converged comment + // (gittensory shape + reviewbot's review folded in). The gate stays authoritative (passed as `decision`), + // and the body carries the SAME panel marker so the upsert updates in place. Flag-OFF (default) keeps the + // legacy panel byte-identical. Only the comment lane is affected; the gate check-run/labels/audit are not. + if (isUnifiedReviewCommentEnabled(env) && gateEvaluation) { + const { rows, readinessTotal } = buildPublicPrPanelSignalRows({ repo, pr, profile, detection, queueHealth, collisions, preflight, settings, gate: gateEvaluation }); + const unifiedFiles = await listPullRequestFiles(env, repoFullName, pr.number); + deterministicBody = buildUnifiedCommentBody({ + gate: gateEvaluation, + ...(aiReview !== undefined ? { aiReview } : {}), + advisoryFindings: advisory.findings, + panelRows: rows, + ...(reviewConfig?.fields !== undefined ? { reviewFields: reviewConfig.fields } : {}), + readinessTotal, + changedFiles: unifiedFiles.length, + footerMarkdown: gittensoryFooter({ + earnUrl: repo?.isRegistered ? gittensorRepoEarnUrl(repoFullName) : undefined, + ...(reviewConfig?.footerText ? { customText: reviewConfig.footerText } : {}), + }), + reRunLabel: `${PR_PANEL_RETRIGGER_MARKER} Re-run Gittensory review`, + }); + } else { + deterministicBody = buildPublicPrIntelligenceComment(commentArgs); + } try { await createOrUpdatePrIntelligenceComment(env, installationId, repoFullName, pr.number, deterministicBody); publishedOutputs.push("comment"); diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts new file mode 100644 index 0000000000..5734c0b139 --- /dev/null +++ b/src/review/unified-comment-bridge.ts @@ -0,0 +1,207 @@ +// Unified-comment bridge (reviewbot→gittensory convergence, Stage D). +// +// A PURE, testable mapping from gittensory's live PR-review data (the gate `GateCheckEvaluation`, the AI +// `advisoryNotes` + consensus defect, the readiness signal rows + total, the footer) onto the ported +// unified renderer (`renderUnifiedReviewComment`). Flag-gated and default-OFF in the processor; flag-OFF +// keeps the legacy `buildPublicPrIntelligenceComment` path byte-identical. +// +// gittensory's GATE stays authoritative: we pass the gate-derived `decision` into `buildUnifiedReviewInput` +// so `deriveUnifiedStatus` lets it override the reviewer recommendations (the renderer already enforces +// this). The output PREPENDS the exact panel marker the legacy body carries, so the existing in-place +// upsert (`createOrUpdatePrIntelligenceComment`) updates the same comment instead of posting a duplicate. +// +// This module is pure (no I/O, no redaction). The caller applies gittensory's public-safe handling the +// same way it does for the legacy body. The data fed in is already public-safe by construction (the AI +// notes via `composeAdvisoryNotes`→`toPublicSafe`; the gate blockers via `sanitizeForCheckRun`; the signal +// rows via the panel helpers' `sanitizePanelText`). + +import type { AdvisoryFinding } from "../types"; +import type { GateCheckConclusion, GateCheckEvaluation } from "../rules/advisory"; +import type { PublicPrPanelSignalRow } from "../signals/engine"; +import { + buildUnifiedReviewInput, + renderUnifiedReviewComment, + type DualReviewNote, + type MergeReadiness, + type ReviewNotes, + type ReviewRecommendation, + type UnifiedCollapsible, + type UnifiedSignalRow, + type Verdict, +} from "./unified-comment"; + +/** The exact marker the legacy panel carries (engine.ts `buildPublicPrIntelligenceComment` / + * `comments.ts` PR_PANEL_COMMENT_MARKER). The unified body MUST prepend this verbatim or the upsert + * posts a DUPLICATE instead of updating in place. */ +export const PR_PANEL_COMMENT_MARKER = ""; + +/** Map gittensory's gate conclusion to the renderer's authoritative `Verdict`. + * success → merge · failure → close · action_required/neutral → manual · skipped → comment. */ +export function gateConclusionToVerdict(conclusion: GateCheckConclusion): Verdict { + switch (conclusion) { + case "success": + return "merge"; + case "failure": + return "close"; + case "action_required": + case "neutral": + return "manual"; + case "skipped": + return "comment"; + } +} + +/** A reviewer recommendation aligned with the gate verdict (advisory; the gate `decision` overrides it). */ +export function verdictToRecommendation(verdict: Verdict): ReviewRecommendation { + switch (verdict) { + case "merge": + return "merge"; + case "close": + return "close"; + case "manual": + return "manual_review"; + case "comment": + case "ignore": + return "manual_review"; + } +} + +/** Derive an ok/warn/fail state from a legacy panel result cell's leading status icon (✅/⚠️/❌). */ +function rowState(resultCell: string): UnifiedSignalRow["state"] { + if (resultCell.startsWith("✅")) return "ok"; + if (resultCell.startsWith("❌")) return "fail"; + return "warn"; +} + +/** Strip the leading status icon from a result cell so it is not duplicated next to the unified icon. */ +function rowResultText(resultCell: string): string { + return resultCell.replace(/^[✅⚠️❌]+\s*/u, "").trim(); +} + +/** Map the legacy panel signal rows → the unified table's rows (label/state/result/evidence). The + * unified renderer adds its own "Code review" row first; these follow it (gittensory's gate row included). */ +export function panelRowsToSignalRows(rows: PublicPrPanelSignalRow[]): UnifiedSignalRow[] { + return rows.map((row) => { + const [label, result, evidence] = row.cells; + return { label, state: rowState(result), result: rowResultText(result), evidence }; + }); +} + +/** Build the single AI reviewer note from gittensory's AI output: the composed advisory write-up becomes + * the assessment; a consensus defect (recovered from the advisory findings) becomes a blocker; the gate's + * non-blocking warnings become nits. Returns `[]` when there is nothing reviewer-side to surface (no AI + * notes, no consensus defect) so the renderer hides the reviewer chip. The gate `decision` (passed + * separately) stays authoritative over `recommendation` — this is advisory framing only. */ +export function buildDualReviewNotes(args: { + aiReview?: { notes: string } | undefined; + consensusDefect?: { title: string; detail: string } | undefined; + warnings?: AdvisoryFinding[] | undefined; + recommendation: ReviewRecommendation; + verdict: Verdict; + reviewerModel?: string; +}): DualReviewNote[] { + const assessment = args.aiReview?.notes?.trim() ?? ""; + const blockers = args.consensusDefect ? [`${args.consensusDefect.title}${args.consensusDefect.detail ? `: ${args.consensusDefect.detail}` : ""}`.trim()] : []; + const nits = (args.warnings ?? []).map((warning) => `${warning.title}${warning.action ? ` — ${warning.action}` : ""}`.trim()).filter(Boolean); + if (!assessment && blockers.length === 0 && nits.length === 0) return []; + const notes: ReviewNotes = { + assessment, + suggestions: [], + risks: [], + verdict: args.verdict, + recommendation: args.recommendation, + confidence: 0.9, + blockers, + nits, + }; + return [{ model: args.reviewerModel ?? "Gittensory AI review", notes }]; +} + +/** Recover a consensus defect (the dual-model agreement the gate already folded into its findings) from + * the advisory findings so the bridge can surface it as a structured blocker. */ +export function consensusDefectFromFindings(findings: AdvisoryFinding[] | undefined): { title: string; detail: string } | undefined { + const found = (findings ?? []).find((finding) => finding.code === "ai_consensus_defect"); + if (!found) return undefined; + return { title: found.title, detail: found.detail }; +} + +export type UnifiedCommentBridgeArgs = { + /** gittensory's authoritative gate verdict (drives the unified status + the Gate row). */ + gate: GateCheckEvaluation; + /** The AI maintainer-review advisory notes (already public-safe), if any. */ + aiReview?: { notes: string } | undefined; + /** The advisory findings — the bridge recovers the `ai_consensus_defect` consensus blocker from here. */ + advisoryFindings?: AdvisoryFinding[] | undefined; + /** The legacy panel readiness signal rows (from `buildPublicPrPanelSignalRows`). */ + panelRows: PublicPrPanelSignalRow[]; + /** Which rows the maintainer kept visible (`.gittensory.yml review.fields`); a key set to `false` is hidden. */ + reviewFields?: Partial> | undefined; + /** The gittensory readiness total (0–100) → the readiness chip. */ + readinessTotal: number; + /** Number of changed files reviewed. */ + changedFiles: number; + /** Number of independent AI reviewers synthesized (0 hides the reviewer chip/row evidence count). */ + reviewerCount?: number | undefined; + /** CI + merge-state readiness, when the caller resolved it (gittensory's panel omits it today). */ + mergeReadiness?: MergeReadiness | undefined; + /** Whether the PR was auto-merged (only changes the ready-state verdict wording). */ + merged?: boolean | undefined; + /** The footer markdown (earn CTA + attribution) — rendered under a divider. */ + footerMarkdown: string; + /** The re-run checkbox label. */ + reRunLabel?: string | undefined; + /** Extra collapsed sections (e.g. signal definitions / contributor next steps). */ + extraCollapsibles?: UnifiedCollapsible[] | undefined; + /** Headline brand (default "Gittensory review"). */ + brand?: string | undefined; +}; + +/** + * Build the unified PR-review comment body from gittensory's live data. Returns a string that STARTS with + * the panel marker (so the existing upsert updates in place) followed by the rendered unified comment. + * The gate verdict is authoritative: it is passed as `decision` so the renderer's `deriveUnifiedStatus` + * lets it override the reviewer recommendation. + */ +export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string { + const verdict = gateConclusionToVerdict(args.gate.conclusion); + const consensusDefect = consensusDefectFromFindings(args.advisoryFindings); + const reviews = buildDualReviewNotes({ + aiReview: args.aiReview, + consensusDefect, + warnings: args.gate.warnings, + recommendation: verdictToRecommendation(verdict), + verdict, + }); + const input = buildUnifiedReviewInput({ + changedFiles: args.changedFiles, + reviews, + decision: verdict, + ...(args.mergeReadiness !== undefined ? { readiness: args.mergeReadiness } : {}), + ...(args.merged !== undefined ? { merged: args.merged } : {}), + }); + // The gate already produced 0/1 reviewer notes from a synthesis of the model pair; reflect the caller's + // actual reviewer count (for the chip + the "N reviewers, synthesized" evidence) without re-deriving it. + if (typeof args.reviewerCount === "number") input.reviewerCount = args.reviewerCount; + + // Honor `.gittensory.yml review.fields` row visibility, exactly as the legacy panel does. + const visibleRows = args.panelRows.filter((row) => args.reviewFields?.[row.key] !== false); + const signals = panelRowsToSignalRows(visibleRows); + + const body = renderUnifiedReviewComment(input, { + brand: args.brand ?? "Gittensory review", + readinessScore: args.readinessTotal, + signals, + footerMarkdown: args.footerMarkdown, + ...(args.reRunLabel !== undefined ? { reRunLabel: args.reRunLabel } : {}), + ...(args.extraCollapsibles !== undefined ? { extraCollapsibles: args.extraCollapsibles } : {}), + }); + + // Prepend the marker verbatim (matching the legacy body, which leads with the marker then a blank line) + // so `createOrUpdatePrIntelligenceComment` finds and updates the SAME comment in place. + return `${PR_PANEL_COMMENT_MARKER}\n\n${body}`; +} + +/** Truthy-env flag check, matching the codebase convention (e.g. SCORING_TIME_DECAY_ENABLED). */ +export function isUnifiedReviewCommentEnabled(env: { UNIFIED_REVIEW_COMMENT?: string | undefined }): boolean { + return /^(1|true|yes|on)$/i.test(env.UNIFIED_REVIEW_COMMENT ?? ""); +} diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts new file mode 100644 index 0000000000..b6019fcee5 --- /dev/null +++ b/src/review/unified-comment.ts @@ -0,0 +1,409 @@ +// Unified PR review comment renderer (convergence — see docs/UNIFIED_REVIEW_COMMENT.md). +// +// Produces ONE in-place comment in the gittensory SHAPE (colored alert sidebar + readiness +// signal table + collapsibles + re-run + earning footer) with reviewbot's deep review folded +// in (the verdict, the synthesized summary, a "Code review" signal row, nits/blockers), deduped. +// +// ADDITIVE + DORMANT: the live Worker keeps composeUnifiedReview() (advisory-render.ts). This +// renderer is exposed via engine.ts for the host (the gittensory app) to call at cutover — it is +// a PURE function (no I/O, no redaction). The host applies its public-safe redaction AFTER, the +// same way the runtime does today (makePublicRedactor / redactOutsideCodeFences). +// +// The host provides gittensory's readiness signals + footer + collapsibles in UnifiedCommentContext; +// reviewbot's review data comes in UnifiedReviewInput. The whole comment recolors by one unified +// status so there is a single authoritative verdict, never two. +// +// SELF-CONTAINED NATIVE PORT (reviewbot→gittensory convergence): every type + helper this module +// needs is defined HERE. No imports from reviewbot. The logic is byte-faithful to the reviewbot +// source (src/core/unified-comment-render.ts + src/core/advisory-render.ts); the only deltas are +// mechanical guards for gittensory's stricter tsconfig (noUncheckedIndexedAccess + +// exactOptionalPropertyTypes), which do not change behavior. + +// ── Inlined minimal types (ported from reviewbot src/core/{ai-review,types,checks-gate}.ts) ───── + +/** A reviewer's decision (a recommendation, not an enforced action). Always one of four — no neutral "comment". */ +export type ReviewRecommendation = "merge" | "request_changes" | "close" | "manual_review"; + +/** The gate's final verdict (reviewbot src/core/types.ts). */ +export type Verdict = "merge" | "close" | "manual" | "comment" | "ignore"; + +/** A maintainer-style review: assessment + actionable notes (not a pass/fail gate). + * Inlined from reviewbot's ReviewNotes — only the fields this renderer's extraction reads + * are load-bearing, but the full shape is preserved for a faithful port. */ +export interface ReviewNotes { + assessment: string; + suggestions: string[]; + risks: string[]; + verdict: Verdict | "manual"; + /** This reviewer's recommended outcome for the human merger. */ + recommendation: ReviewRecommendation; + confidence: number; + /** Tier-1 (prSummary): a brief file-by-file walkthrough of the change. */ + walkthrough?: string; + /** Change MAGNITUDE for the non-content auto-merge gate (#non-content-gate): a `fundamental` change — + * or one that `touchesImportantLogic` (backend/frontend logic, CI, a feature/contract) — is HELD for a + * human even when correct; a `trivial`/`moderate` fix may auto-merge. Optional: only gated lanes ask. */ + changeClass?: "trivial" | "moderate" | "fundamental"; + touchesImportantLogic?: boolean; + /** Unified review (CodeRabbit-style Changes table): a per-file one-line summary of what changed. */ + changes?: Array<{ file: string; summary: string }>; + /** Tier-1 (inlineComments): line-level findings. `line` is the NEW-file line; `suggestion` (when + * suggestedEdits is on) is replacement code rendered as a committable ```suggestion block. + * `severity` tiers the finding (critical=bug/security/breakage, major=should fix before merge, + * minor=small improvement, nitpick=trivial/style); `title` is a short headline. */ + findings?: Array<{ + file: string; + line: number; + comment: string; + suggestion?: string; + severity?: "critical" | "major" | "minor" | "nitpick"; + title?: string; + }>; + /** Unified-review comment (#unified-comment): the reviewer's concerns split by severity — `blockers` are + * concrete must-fix defects (a blocker present ⇒ don't auto-merge); `nits` are non-blocking suggestions. */ + blockers?: string[]; + nits?: string[]; +} + +/** One model's advisory review (or null when that model was unavailable/unparseable). */ +export interface DualReviewNote { + model: string; + notes: ReviewNotes | null; +} + +/** A failing check with the WHY, not just the name — so a review can factor the specific failure in (e.g. + * codecov's "60% of diff hit (target 97%)") instead of a bare "codecov/patch failed". `summary` comes from + * a check-run's output.title/summary or a commit-status's description; `detailsUrl` links the logs/report. */ +export interface CheckFailureDetail { + name: string; + summary?: string; + detailsUrl?: string; +} + +// ── Ported merge-readiness + review-summary extraction (reviewbot src/core/advisory-render.ts) ── + +/** Merge-readiness facts the caller resolves from GitHub BEFORE the advisory runs: is the PR actually + * mergeable, and is every CI check green? The reviewers judge the DIFF; this judges whether the PR can land + * at all — so a clean diff verdict never becomes a formal APPROVE on a conflicting / red-CI PR (#3906/#3908). + * Canonical home (#288): was duplicated identically in the awesome-claude + metagraphed agents. */ +export interface MergeReadiness { + mergeStateLabel?: string; + ciState: "passed" | "failed" | "unverified"; + failingChecks?: string[]; + failingDetails?: CheckFailureDetail[]; +} + +/** The structured synthesis of the reviewers' notes that drives BOTH the legacy unified comment + * (composeUnifiedReview) and the converged renderer's input (buildUnifiedReviewInput) — so the two never + * diverge on which blockers/nits/summary are surfaced or what counts as a consensus blocker. (#unified-comment) */ +export interface ExtractedReviewSummary { + recommendations: ReviewRecommendation[]; + failedCount: number; + blockers: string[]; + nits: string[]; + summary: string; + consensusBlocker: boolean; +} + +/** Case-insensitive de-dup of concern lines (two reviewers often raise the same point). Preserves first wording. */ +function dedupeConcerns(items: string[]): string[] { + const seen = new Set(); + const out: string[] = []; + for (const raw of items) { + const t = raw.trim(); + if (!t) continue; + const key = t.toLowerCase().replace(/[\s.,;:!?]+/g, " ").trim(); + if (seen.has(key)) continue; + seen.add(key); + out.push(t); + } + return out.slice(0, 20); +} + +export function extractReviewSummary(reviews: DualReviewNote[]): ExtractedReviewSummary { + const valid = reviews.filter((r) => r.notes); + const failedCount = reviews.length - valid.length; + const recommendations = valid.map((r) => (r.notes as ReviewNotes).recommendation); + const blockers = dedupeConcerns(valid.flatMap((r) => (r.notes as ReviewNotes).blockers ?? [])); + // Nits = the reviewers' explicit nits + their free-form suggestions (both non-blocking). + const nits = dedupeConcerns(valid.flatMap((r) => [...((r.notes as ReviewNotes).nits ?? []), ...(r.notes as ReviewNotes).suggestions])); + // A CONSENSUS blocker = ≥2 reviewers flagged one (or the sole reviewer did). A lone blocker in a dual review is a + // split (held), not a hard block — matches the gate's severity discipline. + const reviewersWithBlockers = valid.filter((r) => ((r.notes as ReviewNotes).blockers ?? []).length > 0).length; + const consensusBlocker = reviewersWithBlockers >= 2 || (valid.length === 1 && reviewersWithBlockers === 1); + const summary = valid.map((r) => (r.notes as ReviewNotes).assessment).find((a) => a?.trim())?.trim() ?? ""; + return { recommendations, failedCount, blockers, nits, summary, consensusBlocker }; +} + +// ── Unified renderer (reviewbot src/core/unified-comment-render.ts) ────────────────────────────── + +/** The four visual states the comment recolors between (bar + GitHub alert sidebar together). */ +export type UnifiedCommentStatus = "ready" | "advisory" | "held" | "blocked"; + +/** reviewbot's review side of the comment (mapped by the host/runtime from the gate decision + notes). */ +export interface UnifiedReviewInput { + /** Number of changed files reviewed. */ + changedFiles: number; + /** Independent AI reviewers synthesized (e.g. 2). 0 hides the chip. */ + reviewerCount: number; + /** Per-reviewer recommendations (drives the derived status when no explicit decision). */ + recommendations: ReviewRecommendation[]; + /** The synthesized, already-public-safe summary prose. */ + summary: string; + /** Consensus blocking issues (shown expanded when present). */ + blockers?: string[]; + /** Non-blocking suggestions (collapsed). */ + nits?: string[]; + /** CI + merge-state readiness. */ + readiness?: MergeReadiness; + /** The gate's final verdict, if already decided. */ + decision?: Verdict; + /** Whether the PR was auto-merged (only changes the ready-state verdict wording). */ + merged?: boolean; + /** Optional short reason appended to the verdict line. */ + verdictReason?: string; + /** Whether blocker(s) are a consensus (≥2 reviewers / sole reviewer) — drives blocked vs held. */ + consensusBlocker?: boolean; + /** Reviewers that produced no parseable verdict (a partial review → held, not ready). */ + failedCount?: number; +} + +/** One row of the readiness signal table (gittensory side, host-provided; the engine adds Code review). */ +export interface UnifiedSignalRow { + label: string; + state: "ok" | "warn" | "fail"; + /** Short result text, e.g. "Linked", "25/25". */ + result?: string; + /** Evidence cell, e.g. "#1372". */ + evidence?: string; +} + +/** A collapsed section (gittensory side: signal definitions, contributor next steps, …). */ +export interface UnifiedCollapsible { + title: string; + body: string; +} + +/** The host (gittensory) side: brand, readiness score, signals, sections, re-run, footer. */ +export interface UnifiedCommentContext { + /** Headline brand, default "Gittensory review". */ + brand?: string; + /** gittensory readiness score 0–100 (omitted = no chip). */ + readinessScore?: number; + /** gittensory readiness signal rows (rendered after the Code review row). */ + signals?: UnifiedSignalRow[]; + /** Extra collapsed sections (rendered after Nits). */ + extraCollapsibles?: UnifiedCollapsible[]; + /** Re-run checkbox label, e.g. "Re-run Gittensory review" (omitted = no checkbox). */ + reRunLabel?: string; + /** Footer markdown (earning + branding), rendered under a divider. */ + footerMarkdown?: string; + /** Force the status (e.g. the host knows it auto-merged). */ + statusOverride?: UnifiedCommentStatus; +} + +const STATUS_META: Record = { + ready: { alert: "TIP", square: "🟩", icon: "✅" }, + advisory: { alert: "NOTE", square: "🟦", icon: "💡" }, + held: { alert: "WARNING", square: "🟨", icon: "⏸️" }, + blocked: { alert: "CAUTION", square: "🟥", icon: "🛑" }, +}; + +const SIGNAL_ICON: Record = { ok: "✅", warn: "⚠️", fail: "❌" }; + +/** Derive the single unified status from reviewbot's decision/recs/CI + the host override. */ +export function deriveUnifiedStatus(input: UnifiedReviewInput, ctx: UnifiedCommentContext = {}): UnifiedCommentStatus { + if (ctx.statusOverride) return ctx.statusOverride; + // An explicit gate verdict is authoritative — it already weighed the reviewers + guardrails. + switch (input.decision) { + case "merge": + return "ready"; + case "close": + return "blocked"; + case "manual": + return "held"; + case "comment": + case "ignore": + return "advisory"; + } + // No explicit decision → mirror reviewbot's unifiedStatus over the reviewers: a consensus blocker / close → + // blocked; a lone blocker, a split, or a partial (failed) review → held; an empty review → advisory; all-merge → ready. + const recs = input.recommendations ?? []; + const hasConsensusBlocker = input.consensusBlocker ?? (input.blockers ?? []).length > 0; + if (recs.includes("close") || hasConsensusBlocker) return "blocked"; + if (input.readiness?.ciState === "failed") return "held"; + if (recs.length === 0) return "advisory"; + if ((input.failedCount ?? 0) > 0 || recs.some((r) => r !== "merge")) return "held"; + return "ready"; +} + +function verb(status: UnifiedCommentStatus, input: UnifiedReviewInput): string { + switch (status) { + case "ready": + return "safe to merge"; + case "advisory": + return "advisory only"; + case "held": + return "held for maintainer review"; + case "blocked": + return input.decision === "close" ? "closed" : "blocked"; + } +} + +function plural(n: number, one: string): string { + return `${n} ${one}${n === 1 ? "" : "s"}`; +} + +function statusChips(input: UnifiedReviewInput, ctx: UnifiedCommentContext): string { + const chips: string[] = [`\`${plural(input.changedFiles, "file")}\``]; + if (input.reviewerCount > 0) chips.push(`\`${input.reviewerCount} AI reviewers\``); + const blockerCount = (input.blockers ?? []).length; + chips.push(blockerCount ? `\`${plural(blockerCount, "blocker")}\`` : "`no blockers`"); + if (typeof ctx.readinessScore === "number") chips.push(`\`readiness ${Math.round(ctx.readinessScore)}/100\``); + if (input.readiness) { + const ci = input.readiness.ciState; + chips.push(ci === "passed" ? "`CI green`" : ci === "failed" ? "`CI failing`" : "`CI pending`"); + if (input.readiness.mergeStateLabel) chips.push(`\`${input.readiness.mergeStateLabel}\``); + } + return chips.join(" · "); +} + +function verdictLine(status: UnifiedCommentStatus, input: UnifiedReviewInput): string { + const icon = STATUS_META[status].icon; + const reason = input.verdictReason ? ` — ${input.verdictReason}` : ""; + switch (status) { + case "ready": + return input.merged + ? `**${icon} Approved & auto-merged**${input.verdictReason ? reason : " — all checks passed"}` + : `**${icon} Approved**${input.verdictReason ? reason : " — safe to merge"}`; + case "advisory": + return `**${icon} Advisory only**${input.verdictReason ? reason : " — no action taken"}`; + case "held": + return `**${icon} Held for maintainer review**${reason}`; + case "blocked": + return `**${icon} ${input.decision === "close" ? "Closed" : "Blocked"}**${reason}`; + } +} + +/** Dedupe + cap a list of lines (case-insensitive), so blockers/nits never balloon the comment. */ +function dedupeLines(items: string[], cap = 12): string[] { + const seen = new Set(); + const out: string[] = []; + for (const raw of items) { + const line = raw.trim(); + if (!line) continue; + const key = line.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + out.push(line); + if (out.length >= cap) break; + } + return out; +} + +function bullets(items: string[]): string { + return dedupeLines(items) + .map((i) => `- ${i}`) + .join("\n"); +} + +function signalTable(input: UnifiedReviewInput, ctx: UnifiedCommentContext): string { + const blockerCount = (input.blockers ?? []).length; + const codeRow: UnifiedSignalRow = { + label: "Code review", + state: blockerCount ? "fail" : "ok", + result: blockerCount ? plural(blockerCount, "blocker") : "No blockers", + evidence: input.reviewerCount > 0 ? `${input.reviewerCount} reviewers, synthesized` : "synthesized", + }; + const rows = [codeRow, ...(ctx.signals ?? [])]; + const lines = rows.map((r, i) => { + const label = i === 0 ? `**${r.label}**` : r.label; + const result = `${SIGNAL_ICON[r.state]}${r.result ? ` ${r.result}` : ""}`; + return `| ${label} | ${result} | ${r.evidence ?? ""} |`; + }); + return ["| Signal | Result | Evidence |", "|---|---|---|", ...lines].join("\n"); +} + +function details(title: string, body: string, sub?: string): string { + return `
${title}${sub ? ` — ${sub}` : ""}\n\n${body}\n
`; +} + +/** Wrap the assembled body in a GitHub alert blockquote — this is the full-comment colored sidebar. */ +function asAlert(alert: string, inner: string): string { + const quoted = inner + .split("\n") + .map((l) => (l.length ? `> ${l}` : ">")) + .join("\n"); + return `> [!${alert}]\n${quoted}`; +} + +/** + * Render the unified PR review comment as GitHub markdown. Pure + public-safe-by-construction + * (it only emits the fields passed in; no guardrail paths / thresholds / rubric). The host applies + * its redactor to the result before posting, exactly as the runtime does for the legacy comment. + */ +export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: UnifiedCommentContext = {}): string { + const status = deriveUnifiedStatus(input, ctx); + const meta = STATUS_META[status]; + const brand = ctx.brand ?? "Gittensory review"; + + const blocks: string[] = [ + meta.square.repeat(12), + `### ${meta.icon} ${brand} — ${verb(status, input)}${status === "ready" && input.merged ? " · auto-merged" : ""}`, + statusChips(input, ctx), + verdictLine(status, input), + ]; + + if (input.summary.trim()) blocks.push(`**Review summary**\n${input.summary.trim()}`); + + const blockers = dedupeLines(input.blockers ?? []); + if (blockers.length) { + const heading = status === "blocked" ? "Why this is blocked" : "Concerns raised — review before merging"; + blocks.push(`**${heading}**\n${bullets(blockers)}`); + } + + blocks.push(signalTable(input, ctx)); + + const nits = dedupeLines(input.nits ?? []); + if (nits.length) blocks.push(details("Nits", bullets(nits), `${nits.length} non-blocking`)); + for (const c of ctx.extraCollapsibles ?? []) { + if (c.body.trim()) blocks.push(details(c.title, c.body.trim())); + } + + if (ctx.reRunLabel) blocks.push(`- [ ] ${ctx.reRunLabel}`); + if (ctx.footerMarkdown?.trim()) blocks.push(`---\n${ctx.footerMarkdown.trim()}`); + + return asAlert(meta.alert, blocks.join("\n\n")); +} + +/** + * Build the renderer's input from reviewbot's actual review output, reusing the shared extraction + * (extractReviewSummary) so the converged comment surfaces exactly the blockers / nits / summary / consensus + * reviewbot itself decided on — never a divergent second synthesis. The host then supplies its gittensory + * signals/footer in UnifiedCommentContext and calls renderUnifiedReviewComment. + */ +export function buildUnifiedReviewInput(opts: { + changedFiles: string[] | number; + reviews: DualReviewNote[]; + readiness?: MergeReadiness; + decision?: Verdict; + merged?: boolean; + verdictReason?: string; +}): UnifiedReviewInput { + const ex = extractReviewSummary(opts.reviews); + const changedFiles = typeof opts.changedFiles === "number" ? opts.changedFiles : opts.changedFiles.length; + return { + changedFiles, + reviewerCount: opts.reviews.filter((r) => r.notes).length, + recommendations: ex.recommendations, + summary: ex.summary, + blockers: ex.blockers, + nits: ex.nits, + consensusBlocker: ex.consensusBlocker, + failedCount: ex.failedCount, + ...(opts.readiness !== undefined ? { readiness: opts.readiness } : {}), + ...(opts.decision !== undefined ? { decision: opts.decision } : {}), + ...(opts.merged !== undefined ? { merged: opts.merged } : {}), + ...(opts.verdictReason !== undefined ? { verdictReason: opts.verdictReason } : {}), + }; +} diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 1eafb105a8..63bbbcc361 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -4144,6 +4144,59 @@ type PublicPrPanelGateEvaluation = { summary: string; }; +/** One readiness signal row of the public PR panel, with the cells the legacy table renders. The + * unified-comment bridge (convergence) consumes these — `result` carries the leading ✅/⚠️/❌ icon so + * the bridge can derive an ok/warn/fail state without re-running the readiness math. */ +export type PublicPrPanelSignalRow = { key: ReviewFieldKey; cells: [string, string, string, string] }; + +/** + * Build the public PR panel's readiness signal rows (the `allRows` table) as a PURE function, from the + * SAME inputs `buildPublicPrIntelligenceComment` uses. It calls the same private panel helpers, so the rows + * are byte-identical to the legacy panel's. Exposed for the unified-comment bridge (convergence) so the + * converged comment surfaces gittensory's exact signals; the legacy path is unchanged. The `key` lets the + * caller honor `.gittensory.yml review.fields` visibility the same way the legacy renderer does. + */ +export function buildPublicPrPanelSignalRows(args: { + repo: RepositoryRecord | null; + pr: PullRequestRecord; + profile: ContributorProfile; + detection: ContributorDetection; + queueHealth: QueueHealth; + collisions: CollisionReport; + preflight: PreflightResult; + settings: RepositorySettings; + gate?: PublicPrPanelGateEvaluation | undefined; +}): { rows: PublicPrPanelSignalRow[]; readinessTotal: number } { + const prCollisionClusters = pullRequestSpecificCollisionClusters(args.collisions, args.pr); + const linkedDuplicatePrs = linkedIssueDuplicatePullRequests(args.pr, prCollisionClusters); + const scopedOverlapClusters = unionScopedOverlapClusters(args.collisions, args.pr, args.preflight.collisions); + const scopedOverlapCount = scopedOverlapClusters.length; + const readiness = buildPublicReadinessScore({ pr: args.pr, preflight: args.preflight, queueHealth: args.queueHealth, linkedDuplicatePrs, scopedOverlapCount }); + const linkedIssueResult = linkedIssuePanelResult(args.pr); + const relatedWorkResult = relatedWorkPanelResult(linkedDuplicatePrs, scopedOverlapCount); + const gateEnabled = args.settings.gateCheckMode === "enabled"; + const hardLinkedIssueBlock = args.settings.linkedIssueGateMode === "block" && args.pr.linkedIssues.length === 0 && !hasClearNoIssueRationale(args.pr); + const hardDuplicateBlock = args.settings.duplicatePrGateMode === "block" && linkedDuplicatePrs.length > 0; + const fallbackGateConclusion = !gateEnabled ? "success" : !args.repo ? "neutral" : hardLinkedIssueBlock || hardDuplicateBlock ? "failure" : "success"; + const gateConclusion = args.gate?.conclusion ?? fallbackGateConclusion; + const confirmedMiner = isOfficialContributorDetection(args.detection); + const readinessByKey = new Map(readiness.components.map((component) => [component.key, component])); + const validationComponent = readinessByKey.get("validation"); + const changeScopeComponent = readinessByKey.get("change_scope"); + const queueComponent = readinessByKey.get("queue_pressure"); + const contributorContext = contributorContextPanelResult(args.pr, args.profile, args.detection, confirmedMiner); + const rows: PublicPrPanelSignalRow[] = [ + { key: "linkedIssue", cells: ["Linked issue", linkedIssueResult.result, linkedIssueResult.evidence, linkedIssueResult.action] }, + { key: "relatedWork", cells: ["Related work", relatedWorkResult.result, relatedWorkResult.evidence, relatedWorkResult.action] }, + { key: "reviewLoad", cells: ["Review load", scoreResultIcon(changeScopeComponent), changeScopeComponent?.evidence ?? "No public scope metadata found.", changeScopeComponent?.action ?? "No action."] }, + { key: "validationEvidence", cells: ["Validation evidence", scoreResultIcon(validationComponent), validationComponent?.evidence ?? "No validation signal found.", validationComponent?.action ?? "Add validation note."] }, + { key: "openPrQueue", cells: ["Open PR queue", scoreResultIcon(queueComponent), queueComponent?.evidence ?? "Open PR queue unavailable.", queueComponent?.action ?? "No action."] }, + { key: "contributorContext", cells: ["Contributor context", contributorContext.result, contributorContext.evidence, contributorContext.action] }, + { key: "gateResult", cells: ["Gate result", gateStatus(gateEnabled, gateConclusion), gateEnabled ? gateAction(gateConclusion) : "Advisory only.", gateEnabled ? gateNextAction(gateConclusion) : "No action."] }, + ]; + return { rows, readinessTotal: readiness.total }; +} + function isOfficialContributorDetection(detection: ContributorDetection): boolean { return detection.source === "official_gittensor_api"; } diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index f3bbd51d23..69ecb8b63d 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -2724,6 +2724,141 @@ describe("queue processors", () => { expect(skipped.results.map((event) => event.detail)).toEqual(expect.arrayContaining(["not_official_gittensor_miner", "missing_author"])); }); + // #1007 convergence (Stage D): with UNIFIED_REVIEW_COMMENT on AND the gate evaluating, the public PR-panel + // comment is rendered by the UNIFIED renderer (GitHub alert + synthesized "Code review" row) instead of the + // legacy panel — while STILL leading with the same panel marker so the in-place upsert updates the same + // comment. Mirrors the legacy panel-posting setup (confirmed miner + comment_and_label) but flips the flag + // and enables the gate so `maybePublishPrPublicSurface` takes the flag-ON branch. + it("renders the unified PR-review comment when the flag is on and the gate evaluates", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), UNIFIED_REVIEW_COMMENT: "1" }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "detected_contributors_only", + publicAudienceMode: "gittensor_only", + publicSignalLevel: "standard", + publicSurface: "comment_and_label", + autoLabelEnabled: false, + checkRunMode: "off", + checkRunDetailLevel: "minimal", + gateCheckMode: "enabled", + backfillEnabled: true, + privateTrustEnabled: true, + }); + let postedBody = ""; + const calls = { comments: 0, gateChecks: 0 }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") { + return Response.json([ + { + uid: 7, + githubUsername: "oktofeesh1", + githubId: "123", + totalPrs: 4, + totalMergedPrs: 3, + totalOpenPrs: 1, + totalClosedPrs: 0, + totalOpenIssues: 0, + totalClosedIssues: 0, + totalSolvedIssues: 0, + totalValidSolvedIssues: 0, + isEligible: true, + credibility: 1, + eligibleRepoCount: 1, + hotkey: "must-not-leak", + }, + ]); + } + if (url === "https://api.gittensor.io/miners/123") { + return Response.json({ + repositories: [ + { + repositoryFullName: "JSONbored/gittensory", + totalPrs: "4", + totalMergedPrs: "3", + totalOpenPrs: "1", + totalClosedPrs: "0", + totalOpenIssues: "0", + totalClosedIssues: "0", + isEligible: true, + credibility: "1.000000", + }, + ], + }); + } + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + if (url.endsWith("/users/oktofeesh1")) return Response.json({ login: "oktofeesh1", public_repos: 2, followers: 1 }); + if (url.includes("/users/oktofeesh1/repos")) return Response.json([{ language: "TypeScript" }]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + // PR files — the unified branch (re)fetches them to count changed files for the readiness chip. + if (url.includes("/pulls/3/files")) return Response.json([{ filename: "src/cache.ts", additions: 5, deletions: 1, status: "modified" }]); + // Gate check-run — must succeed so `gateEvaluation` is produced and the flag-ON branch runs. + // The pending check is POSTed (in_progress), then PATCHed to its completed conclusion. + if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") { + calls.gateChecks += 1; + return Response.json({ id: 901 }, { status: 201 }); + } + if (url.includes("/check-runs/901") && method === "PATCH") { + calls.gateChecks += 1; + return Response.json({ id: 901 }); + } + if (url.includes("/issues/3/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/3/comments") && method === "POST") { + calls.comments += 1; + postedBody = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 1, html_url: "https://github.com/comment/1" }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "pr-unified-comment", + eventName: "pull_request", + payload: { + action: "synchronize", + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", checks: "write" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 3, + title: "Fix webhook duplicate delivery again", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "unified123" }, + labels: [{ name: "bug" }], + body: "Fixes #1\n\nValidation: npm test", + }, + }, + }); + + expect(calls.comments).toBe(1); + // Still leads with the panel marker → the upsert updates the SAME sticky comment in place (no duplicate). + expect(postedBody).toContain(""); + // The UNIFIED shape, which the legacy body never emits: a full-comment GitHub alert wrapper… + expect(postedBody).toMatch(/> \[!(TIP|NOTE|WARNING|CAUTION)\]/); + // …and the renderer's synthesized "Code review" signal row (bold first table label). + expect(postedBody).toContain("**Code review**"); + // Public-safe by construction — no internal trust/economics fields leak through the unified renderer. + expect(postedBody).not.toMatch(/wallet|hotkey|reward|trust score/i); + }); + it("skips bots and maintainer authors, and keeps explicitly enabled checks minimal", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot( diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 1dff213cac..ba157191aa 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -19,6 +19,7 @@ import { buildPreflightResult, buildPublicCommentSignalBundle, buildPublicPrIntelligenceComment, + buildPublicPrPanelSignalRows, buildPublicReadinessScore, buildQueueHealth, buildRoleContext, @@ -688,6 +689,64 @@ describe("signal coverage edge cases", () => { expect(maintainerComment).not.toMatch(/reward|wallet|hotkey|trust score|farming/i); }); + it("buildPublicPrPanelSignalRows derives the gate conclusion across provided/fallback paths (#1007 unified-panel extraction)", () => { + const directRepo = repo("owner/panel"); + const collisions = buildCollisionReport(directRepo.fullName, [], []); + const baseArgs = { + repo: directRepo, + pr: pr(directRepo.fullName, 70, "Fix cache", { authorLogin: "miner", linkedIssues: [42], body: "Fixes #42" }), + profile: buildContributorProfile("miner", { login: "miner", topLanguages: ["TypeScript"], source: "github" }, [], []), + detection: { detected: true, source: "official_gittensor_api" as const, reason: "Confirmed.", priorPullRequests: 1, priorMergedPullRequests: 0, priorIssues: 0 }, + queueHealth: buildQueueHealth(directRepo, [], [], collisions), + collisions, + preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix cache", body: "Fixes #42", changedFiles: ["src/cache.ts"] }, directRepo, [], []), + }; + const KEYS = ["linkedIssue", "relatedWork", "reviewLoad", "validationEvidence", "openPrQueue", "contributorContext", "gateResult"]; + + // Provided gate is authoritative; gate enabled → a real gate action (not the advisory-only copy). + const provided = buildPublicPrPanelSignalRows({ ...baseArgs, settings: { ...repoSettings(directRepo.fullName), gateCheckMode: "enabled" }, gate: { conclusion: "success", summary: "Passing" } }); + expect(provided.rows.map((r) => r.key)).toEqual(KEYS); + expect(typeof provided.readinessTotal).toBe("number"); + const providedGate = provided.rows.find((r) => r.key === "gateResult")!; + expect(providedGate.cells[2]).not.toBe("Advisory only."); + + // Gate check NOT enabled → fallback success conclusion + the advisory-only action/next-step. + const advisory = buildPublicPrPanelSignalRows({ ...baseArgs, settings: { ...repoSettings(directRepo.fullName), gateCheckMode: "off" } }); + const advisoryGate = advisory.rows.find((r) => r.key === "gateResult")!; + expect(advisoryGate.cells[2]).toBe("Advisory only."); + expect(advisoryGate.cells[3]).toBe("No action."); + + // No gate + enabled + unknown repo → neutral fallback (distinct from the passing cell). + const neutral = buildPublicPrPanelSignalRows({ ...baseArgs, repo: null, settings: { ...repoSettings(directRepo.fullName), gateCheckMode: "enabled" } }); + expect(neutral.rows.find((r) => r.key === "gateResult")!.cells[1]).not.toBe(providedGate.cells[1]); + + // No gate + enabled + a hard linked-issue block (no linked issue, no rationale) → failure fallback. + const blocked = buildPublicPrPanelSignalRows({ + ...baseArgs, + pr: pr(directRepo.fullName, 71, "No issue", { authorLogin: "miner", linkedIssues: [], body: "just a change" }), + settings: { ...repoSettings(directRepo.fullName), gateCheckMode: "enabled", linkedIssueGateMode: "block" }, + }); + expect(blocked.rows).toHaveLength(7); + expect(blocked.rows.find((r) => r.key === "gateResult")!.cells[1]).not.toBe(providedGate.cells[1]); + + // No gate + enabled + a hard duplicate-PR block (another open PR shares the linked issue, and the repo + // configured duplicatePrGateMode: block) → failure fallback via `hardDuplicateBlock`. The current PR (70) + // links #42; a second open PR (88) on the same issue forms the duplicate cluster. + const dupIssue = issue(directRepo.fullName, 42, "Cache invalidation race"); + const dupPr = pr(directRepo.fullName, 88, "Also fixes the cache race", { authorLogin: "other", linkedIssues: [42], body: "Fixes #42" }); + const dupCollisions = buildCollisionReport(directRepo.fullName, [dupIssue], [baseArgs.pr, dupPr]); + const duplicateBlocked = buildPublicPrPanelSignalRows({ + ...baseArgs, + collisions: dupCollisions, + queueHealth: buildQueueHealth(directRepo, [dupIssue], [baseArgs.pr, dupPr], dupCollisions), + settings: { ...repoSettings(directRepo.fullName), gateCheckMode: "enabled", duplicatePrGateMode: "block" }, + }); + expect(duplicateBlocked.rows).toHaveLength(7); + // The duplicate cluster surfaces in the related-work row, and the gate falls back to the failing cell. + expect(duplicateBlocked.rows.find((r) => r.key === "relatedWork")!.cells[1]).toContain("#88"); + expect(duplicateBlocked.rows.find((r) => r.key === "gateResult")!.cells[1]).not.toBe(providedGate.cells[1]); + }); + it("renders opt-in gate panel states for collision and repo evaluation blockers", () => { const directRepo = repo("owner/gate"); const existingIssue = issue(directRepo.fullName, 7, "Cache refresh websocket reconnect failure"); diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts new file mode 100644 index 0000000000..b403b09a32 --- /dev/null +++ b/test/unit/unified-comment-bridge.test.ts @@ -0,0 +1,233 @@ +import { describe, expect, it } from "vitest"; +import { + buildDualReviewNotes, + buildUnifiedCommentBody, + consensusDefectFromFindings, + gateConclusionToVerdict, + isUnifiedReviewCommentEnabled, + panelRowsToSignalRows, + PR_PANEL_COMMENT_MARKER, + verdictToRecommendation, +} from "../../src/review/unified-comment-bridge"; +import type { MergeReadiness, UnifiedCollapsible } from "../../src/review/unified-comment"; +import type { GateCheckEvaluation } from "../../src/rules/advisory"; +import type { AdvisoryFinding } from "../../src/types"; +import type { PublicPrPanelSignalRow } from "../../src/signals/engine"; + +function gate(over: Partial = {}): GateCheckEvaluation { + return { + enabled: true, + conclusion: "success", + title: "Gittensory Gate passed", + summary: "No configured hard blocker was found.", + blockers: [], + warnings: [], + ...over, + }; +} + +// The exact shape the legacy panel emits (icon-prefixed result cells). The bridge derives ok/warn/fail +// from the leading ✅/⚠️/❌ and strips it from the result text. +const panelRows: PublicPrPanelSignalRow[] = [ + { key: "linkedIssue", cells: ["Linked issue", "✅ Linked", "#42", "No action."] }, + { key: "relatedWork", cells: ["Related work", "✅ No active overlap found", "No same-issue overlap.", "No action."] }, + { key: "reviewLoad", cells: ["Review load", "⚠️ 14/20", "Medium review burden.", "Add scope summary."] }, + { key: "validationEvidence", cells: ["Validation evidence", "✅ 25/25", "PR body includes validation.", "No action."] }, + { key: "openPrQueue", cells: ["Open PR queue", "✅ 10/10", "Low queue pressure.", "No action."] }, + { key: "contributorContext", cells: ["Contributor context", "✅ Confirmed Gittensor contributor", "octocat", "No action."] }, + { key: "gateResult", cells: ["Gate result", "✅ Passing", "No configured blocker found.", "No action."] }, +]; + +const footer = "💰 **Earn for open-source contributions like this.** Checked by Gittensory."; + +describe("gateConclusionToVerdict", () => { + it("maps every gate conclusion to its authoritative verdict", () => { + expect(gateConclusionToVerdict("success")).toBe("merge"); + expect(gateConclusionToVerdict("failure")).toBe("close"); + expect(gateConclusionToVerdict("action_required")).toBe("manual"); + expect(gateConclusionToVerdict("neutral")).toBe("manual"); + expect(gateConclusionToVerdict("skipped")).toBe("comment"); + }); +}); + +describe("verdictToRecommendation", () => { + it("maps every verdict (incl. the comment/ignore advisory pair) to a reviewer recommendation", () => { + expect(verdictToRecommendation("merge")).toBe("merge"); + expect(verdictToRecommendation("close")).toBe("close"); + expect(verdictToRecommendation("manual")).toBe("manual_review"); + expect(verdictToRecommendation("comment")).toBe("manual_review"); + expect(verdictToRecommendation("ignore")).toBe("manual_review"); + }); +}); + +describe("panelRowsToSignalRows", () => { + it("derives ok/warn/fail from the leading icon and strips it from the result text", () => { + const rows = panelRowsToSignalRows(panelRows); + const linked = rows.find((row) => row.label === "Linked issue"); + expect(linked).toEqual({ label: "Linked issue", state: "ok", result: "Linked", evidence: "#42" }); + const reviewLoad = rows.find((row) => row.label === "Review load"); + expect(reviewLoad?.state).toBe("warn"); + expect(reviewLoad?.result).toBe("14/20"); + }); + + it("maps a ❌ result cell to fail", () => { + const rows = panelRowsToSignalRows([{ key: "contributorContext", cells: ["Contributor context", "❌ No public Gittensor match", "octocat; not a blocker.", "No action."] }]); + expect(rows[0]?.state).toBe("fail"); + }); +}); + +describe("consensusDefectFromFindings", () => { + it("recovers the ai_consensus_defect finding, ignoring others", () => { + const findings: AdvisoryFinding[] = [ + { code: "missing_linked_issue", severity: "warning", title: "No linked issue", detail: "..." }, + { code: "ai_consensus_defect", severity: "critical", title: "Null deref in handler", detail: "Both models flagged it." }, + ]; + expect(consensusDefectFromFindings(findings)).toEqual({ title: "Null deref in handler", detail: "Both models flagged it." }); + expect(consensusDefectFromFindings([])).toBeUndefined(); + expect(consensusDefectFromFindings(undefined)).toBeUndefined(); + }); +}); + +describe("buildDualReviewNotes", () => { + it("folds the advisory notes (assessment), the consensus defect (blocker), and warnings (nits) into one note", () => { + const reviews = buildDualReviewNotes({ + aiReview: { notes: "The refactor looks correct." }, + consensusDefect: { title: "Off-by-one", detail: "Loop bound is wrong." }, + warnings: [{ code: "w1", severity: "warning", title: "Missing test", detail: "...", action: "Add a test." }], + recommendation: "close", + verdict: "close", + }); + expect(reviews).toHaveLength(1); + expect(reviews[0]?.notes?.assessment).toBe("The refactor looks correct."); + expect(reviews[0]?.notes?.blockers).toEqual(["Off-by-one: Loop bound is wrong."]); + expect(reviews[0]?.notes?.nits).toEqual(["Missing test — Add a test."]); + }); + + it("returns [] when there is nothing reviewer-side to surface", () => { + expect(buildDualReviewNotes({ recommendation: "merge", verdict: "merge" })).toEqual([]); + }); + + it("omits the ': detail' and ' — action' suffixes when the defect has no detail and the warning has no action", () => { + const reviews = buildDualReviewNotes({ + consensusDefect: { title: "Null deref", detail: "" }, + warnings: [{ code: "w1", severity: "warning", title: "No test", detail: "..." }], // no `action` + recommendation: "close", + verdict: "close", + }); + expect(reviews[0]?.notes?.blockers).toEqual(["Null deref"]); // title only, no trailing ": " + expect(reviews[0]?.notes?.nits).toEqual(["No test"]); // title only, no trailing " — " + }); +}); + +describe("buildUnifiedCommentBody", () => { + it("starts with the exact panel marker so the upsert updates in place", () => { + const body = buildUnifiedCommentBody({ + gate: gate(), + aiReview: { notes: "Clean change." }, + panelRows, + readinessTotal: 88, + changedFiles: 3, + footerMarkdown: footer, + }); + expect(body.startsWith(PR_PANEL_COMMENT_MARKER)).toBe(true); + // Same marker the legacy body carries (see comments.ts PR_PANEL_COMMENT_MARKER), so no duplicate comment. + expect(PR_PANEL_COMMENT_MARKER).toBe(""); + }); + + it("renders gittensory's unified shape: a Code review row, the readiness chip, and the gate row", () => { + const body = buildUnifiedCommentBody({ + gate: gate(), + aiReview: { notes: "Clean change." }, + panelRows, + readinessTotal: 88, + changedFiles: 3, + reviewerCount: 2, + footerMarkdown: footer, + }); + expect(body).toContain("Code review"); // the unified renderer's synthesized row + expect(body).toContain("readiness 88/100"); // readinessTotal → chip + expect(body).toContain("Gate result"); // gittensory's signal row is preserved after Code review + expect(body).toContain("> [!TIP]"); // success → ready → TIP alert + }); + + it("the gate conclusion drives the status: a gate failure blocks regardless of reviewer recs", () => { + const failing = buildUnifiedCommentBody({ + gate: gate({ + conclusion: "failure", + title: "Gittensory Gate: blocked", + summary: "A hard blocker was found.", + blockers: [{ code: "ai_consensus_defect", severity: "critical", title: "Real bug", detail: "..." }], + }), + // Even with an upbeat reviewer assessment, the gate failure is authoritative. + aiReview: { notes: "Looks fine to me, recommend merge." }, + advisoryFindings: [{ code: "ai_consensus_defect", severity: "critical", title: "Real bug", detail: "Both models agree." }], + panelRows, + readinessTotal: 40, + changedFiles: 5, + footerMarkdown: footer, + }); + // failure → close verdict → blocked status (CAUTION alert + "Blocked"/"Closed" verdict line). + expect(failing).toContain("> [!CAUTION]"); + expect(failing).toMatch(/Closed|Blocked/); + // The recovered consensus defect surfaces as a blocker. + expect(failing).toContain("Real bug"); + }); + + it("honors review.fields visibility — a hidden row is dropped from the signal table", () => { + const body = buildUnifiedCommentBody({ + gate: gate(), + panelRows, + reviewFields: { contributorContext: false }, + readinessTotal: 88, + changedFiles: 3, + footerMarkdown: footer, + }); + expect(body).not.toContain("Confirmed Gittensor contributor"); + expect(body).toContain("Gate result"); // a visible row is still present + }); + + it("threads the optional merge-readiness, merged, re-run label, and extra collapsibles into the renderer", () => { + const mergeReadiness: MergeReadiness = { ciState: "passed", mergeStateLabel: "clean" }; + const extra: UnifiedCollapsible[] = [{ title: "Signal definitions", body: "Readiness signals describe public-metadata readiness." }]; + const body = buildUnifiedCommentBody({ + gate: gate(), + aiReview: { notes: "Clean change." }, + panelRows, + readinessTotal: 91, + changedFiles: 4, + mergeReadiness, + merged: true, + reRunLabel: "Re-run Gittensory review", + extraCollapsibles: extra, + footerMarkdown: footer, + }); + expect(body).toContain("`CI green`"); // mergeReadiness ciState → chip + expect(body).toContain("`clean`"); // mergeStateLabel → chip + expect(body).toContain("auto-merged"); // merged → ready wording + expect(body).toContain("- [ ] Re-run Gittensory review"); // reRunLabel + expect(body).toContain("
Signal definitions"); // extraCollapsibles + }); + + it("maps a non-merge/non-failure gate conclusion (manual / comment verdicts) through the bridge", () => { + const manual = buildUnifiedCommentBody({ gate: gate({ conclusion: "action_required" }), panelRows, readinessTotal: 60, changedFiles: 2, footerMarkdown: footer }); + expect(manual).toContain("> [!WARNING]"); // action_required → manual → held + const advisory = buildUnifiedCommentBody({ gate: gate({ conclusion: "skipped" }), panelRows, readinessTotal: 50, changedFiles: 2, footerMarkdown: footer }); + expect(advisory).toContain("> [!NOTE]"); // skipped → comment → advisory + }); +}); + +describe("isUnifiedReviewCommentEnabled (flag-OFF selects the legacy path)", () => { + it("is OFF (legacy buildPublicPrIntelligenceComment path) when the flag is unset or falsy", () => { + expect(isUnifiedReviewCommentEnabled({})).toBe(false); + expect(isUnifiedReviewCommentEnabled({ UNIFIED_REVIEW_COMMENT: undefined })).toBe(false); + expect(isUnifiedReviewCommentEnabled({ UNIFIED_REVIEW_COMMENT: "false" })).toBe(false); + expect(isUnifiedReviewCommentEnabled({ UNIFIED_REVIEW_COMMENT: "0" })).toBe(false); + expect(isUnifiedReviewCommentEnabled({ UNIFIED_REVIEW_COMMENT: "" })).toBe(false); + }); + + it("is ON only for an explicit truthy value", () => { + for (const value of ["1", "true", "yes", "on", "TRUE", "On"]) { + expect(isUnifiedReviewCommentEnabled({ UNIFIED_REVIEW_COMMENT: value })).toBe(true); + } + }); +}); diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts new file mode 100644 index 0000000000..1abda8d72f --- /dev/null +++ b/test/unit/unified-comment.test.ts @@ -0,0 +1,268 @@ +import { describe, expect, it } from "vitest"; +import { + buildUnifiedReviewInput, + deriveUnifiedStatus, + type DualReviewNote, + renderUnifiedReviewComment, + type ReviewNotes, + type ReviewRecommendation, + type UnifiedCommentContext, + type UnifiedReviewInput, +} from "../../src/review/unified-comment"; + +const base: UnifiedReviewInput = { + changedFiles: 2, + reviewerCount: 2, + recommendations: ["merge", "merge"], + summary: "Replaces the custom CASE expression with the shared helper and adds a test.", +}; + +describe("deriveUnifiedStatus", () => { + it("ready when the gate decision is merge", () => { + expect(deriveUnifiedStatus({ ...base, decision: "merge" })).toBe("ready"); + }); + + it("ready when every reviewer recommends merge", () => { + expect(deriveUnifiedStatus({ ...base, recommendations: ["merge", "merge"] })).toBe("ready"); + }); + + it("advisory for a comment-only verdict or no actionable recs", () => { + expect(deriveUnifiedStatus({ ...base, decision: "comment", recommendations: [] })).toBe("advisory"); + expect(deriveUnifiedStatus({ ...base, recommendations: [] })).toBe("advisory"); + }); + + it("held for manual / request_changes / failing CI", () => { + expect(deriveUnifiedStatus({ ...base, decision: "manual" })).toBe("held"); + expect(deriveUnifiedStatus({ ...base, recommendations: ["request_changes"] })).toBe("held"); + expect(deriveUnifiedStatus({ ...base, readiness: { ciState: "failed" } })).toBe("held"); + }); + + it("blocked for a close verdict or consensus blockers", () => { + expect(deriveUnifiedStatus({ ...base, decision: "close" })).toBe("blocked"); + expect(deriveUnifiedStatus({ ...base, recommendations: [], blockers: ["leaks a secret"] })).toBe("blocked"); + }); + + it("an explicit merge verdict is authoritative — ready even with a raised concern", () => { + expect(deriveUnifiedStatus({ ...base, decision: "merge", blockers: ["minor"] })).toBe("ready"); + }); + + it("honors an explicit host status override", () => { + expect(deriveUnifiedStatus({ ...base, decision: "close" }, { statusOverride: "ready" })).toBe("ready"); + }); + + it("treats a missing recommendations array as no recs → advisory", () => { + // exercises the `recommendations ?? []` guard for a defensively-shaped input + expect(deriveUnifiedStatus({ changedFiles: 1, reviewerCount: 0, summary: "" } as UnifiedReviewInput)).toBe("advisory"); + }); +}); + +describe("renderUnifiedReviewComment", () => { + const ctx: UnifiedCommentContext = { + readinessScore: 93, + signals: [ + { label: "Linked issue", state: "ok", result: "Linked", evidence: "#1372" }, + { label: "Contributor", state: "ok", result: "Confirmed", evidence: "galuis116 · 168 PRs" }, + ], + extraCollapsibles: [{ title: "Signal definitions", body: "Readiness signals describe public-metadata readiness." }], + reRunLabel: "Re-run Gittensory review", + footerMarkdown: "Checked by Gittensory.", + }; + + it("renders the ready/auto-merged state in the gittensory shape", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "merge", merged: true, readiness: { ciState: "passed" }, nits: ["Document the new property."] }, + ctx, + ); + expect(md).toContain("> [!TIP]"); + expect(md).toContain("🟩"); + expect(md).toContain("Gittensory review — safe to merge · auto-merged"); + expect(md).toContain("Approved & auto-merged"); + expect(md).toContain("`2 files`"); + expect(md).toContain("`2 AI reviewers`"); + expect(md).toContain("`no blockers`"); + expect(md).toContain("`readiness 93/100`"); + expect(md).toContain("`CI green`"); + expect(md).toContain("**Review summary**"); + expect(md).toContain("| **Code review** | ✅ No blockers | 2 reviewers, synthesized |"); + expect(md).toContain("| Linked issue | ✅ Linked | #1372 |"); + expect(md).toContain("
Nits — 1 non-blocking"); + expect(md).toContain("
Signal definitions"); + expect(md).toContain("- [ ] Re-run Gittensory review"); + expect(md).toContain("Checked by Gittensory."); + }); + + it("the entire comment is blockquote-wrapped (the full colored sidebar)", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, ctx); + expect(md.split("\n").every((l) => l.startsWith(">"))).toBe(true); + }); + + it("blocked state uses the caution alert, red bar, and an expanded blockers section", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "close", recommendations: ["close", "close"], blockers: ["Introduces a hardcoded secret."] }, + ctx, + ); + expect(md).toContain("> [!CAUTION]"); + expect(md).toContain("🟥"); + expect(md).toContain("Closed"); + expect(md).toContain("Why this is blocked"); + expect(md).toContain("Introduces a hardcoded secret."); + expect(md).toContain("| **Code review** | ❌ 1 blocker |"); + }); + + it("held state uses the warning alert and amber bar", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "manual", recommendations: ["manual_review"] }, ctx); + expect(md).toContain("> [!WARNING]"); + expect(md).toContain("🟨"); + expect(md).toContain("Held for maintainer review"); + }); + + it("advisory state uses the note alert and blue bar", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "comment", recommendations: [] }, {}); + expect(md).toContain("> [!NOTE]"); + expect(md).toContain("🟦"); + expect(md).toContain("Advisory only"); + }); + + it("dedupes repeated blockers and nits", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "close", blockers: ["Same issue", "same issue", "Same issue"] }, + {}, + ); + expect(md.match(/Same issue/gi)?.length).toBe(1); + }); + + it("omits optional chrome when the host provides none", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, {}); + expect(md).not.toContain("readiness"); + expect(md).not.toContain("- [ ]"); + expect(md.split("\n").some((l) => l.trim() === "> ---")).toBe(false); + }); + + it("only emits provided content (no internal fields leak in)", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, ctx); + expect(md).not.toMatch(/confidenceFloor|scopeCap|hardGuardrailGlobs|rubric/i); + }); + + it("a blocked status from reviewer recs (no close decision) reads 'blocked', not 'closed'", () => { + const md = renderUnifiedReviewComment({ ...base, recommendations: ["close"], blockers: ["Leaks a token."], consensusBlocker: true }, {}); + expect(md).toContain("> [!CAUTION]"); + expect(md).toContain("Gittensory review — blocked"); // verb(): decision !== "close" + expect(md).toContain("**🛑 Blocked**"); // verdictLine(): decision !== "close" + expect(md).not.toContain("Closed"); + }); + + it("renders CI-failing / CI-pending chips and the merge-state label", () => { + const failing = renderUnifiedReviewComment({ ...base, readiness: { ciState: "failed", mergeStateLabel: "behind" } }, {}); + expect(failing).toContain("`CI failing`"); + expect(failing).toContain("`behind`"); + const pending = renderUnifiedReviewComment({ ...base, readiness: { ciState: "unverified" } }, {}); + expect(pending).toContain("`CI pending`"); + }); + + it("appends an explicit verdict reason across ready (merged + unmerged) and advisory states", () => { + // The verdict word is bolded (`**…**`); the reason follows outside the bold, so assert each separately. + const merged = renderUnifiedReviewComment({ ...base, decision: "merge", merged: true, verdictReason: "all checks green" }, {}); + expect(merged).toContain("Approved & auto-merged"); + expect(merged).toContain("all checks green"); // verdictReason appended, not the default " — all checks passed" + const unmerged = renderUnifiedReviewComment({ ...base, decision: "merge", verdictReason: "looks correct" }, {}); + expect(unmerged).not.toContain("auto-merged"); // the unmerged ready variant + expect(unmerged).toContain("looks correct"); + const advisory = renderUnifiedReviewComment({ ...base, decision: "comment", recommendations: [], verdictReason: "for your awareness" }, {}); + expect(advisory).toContain("Advisory only"); + expect(advisory).toContain("for your awareness"); + }); + + it("skips empty blocker lines and caps long nit lists at 12", () => { + const withEmpty = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["", " ", "Real blocker"] }, {}); + expect(withEmpty.match(/Real blocker/g)?.length).toBe(1); + const capped = renderUnifiedReviewComment({ ...base, decision: "merge", nits: Array.from({ length: 13 }, (_, i) => `Distinct nit ${i + 1}`) }, {}); + expect(capped).toContain("Distinct nit 12"); + expect(capped).not.toContain("Distinct nit 13"); + }); + + it("renders a signal row that has neither a result nor evidence", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, { signals: [{ label: "Bare row", state: "warn" }] }); + expect(md).toContain("| Bare row | ⚠️ | |"); + }); + + it("uses the 'Concerns raised' heading (not 'Why this is blocked') for blockers on a non-blocked status", () => { + // a lone request_changes blocker → held, but the concern is still surfaced under the softer heading + const md = renderUnifiedReviewComment({ ...base, recommendations: ["request_changes"], blockers: ["Edge case unhandled."], consensusBlocker: false }, {}); + expect(md).toContain("> [!WARNING]"); + expect(md).toContain("Concerns raised — review before merging"); + expect(md).not.toContain("Why this is blocked"); + expect(md).toContain("Edge case unhandled."); + }); + + it("skips an extra collapsible whose body is empty", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, { extraCollapsibles: [{ title: "Empty section", body: " " }] }); + expect(md).not.toContain("Empty section"); + }); +}); + +function reviewNote(rec: ReviewRecommendation, extra: Partial = {}): DualReviewNote { + return { + model: "test-model", + notes: { verdict: "merge", recommendation: rec, confidence: 0.9, assessment: "Looks fine.", suggestions: [], risks: [], ...extra }, + }; +} + +describe("buildUnifiedReviewInput", () => { + it("maps a clean dual-merge review to a ready input", () => { + const input = buildUnifiedReviewInput({ changedFiles: ["a.ts", "b.ts"], reviews: [reviewNote("merge"), reviewNote("merge")], decision: "merge" }); + expect(input.changedFiles).toBe(2); + expect(input.reviewerCount).toBe(2); + expect(input.summary).toBe("Looks fine."); + expect(deriveUnifiedStatus(input)).toBe("ready"); + }); + + it("a consensus blocker (both reviewers) → blocked even without a gate decision", () => { + const input = buildUnifiedReviewInput({ + changedFiles: 1, + reviews: [reviewNote("request_changes", { blockers: ["secret"] }), reviewNote("request_changes", { blockers: ["secret"] })], + }); + expect(input.consensusBlocker).toBe(true); + expect(deriveUnifiedStatus(input)).toBe("blocked"); + }); + + it("a lone blocker is a split → held, not blocked", () => { + const input = buildUnifiedReviewInput({ + changedFiles: 1, + reviews: [reviewNote("request_changes", { blockers: ["maybe"] }), reviewNote("merge")], + }); + expect(input.consensusBlocker).toBe(false); + expect(deriveUnifiedStatus(input)).toBe("held"); + }); + + it("counts reviewers that produced no verdict (partial review)", () => { + const input = buildUnifiedReviewInput({ changedFiles: 1, reviews: [reviewNote("merge"), { model: "m2", notes: null }] }); + expect(input.failedCount).toBe(1); + expect(input.reviewerCount).toBe(1); + }); + + it("dedupes blockers via the shared extraction", () => { + const input = buildUnifiedReviewInput({ + changedFiles: 1, + reviews: [reviewNote("close", { blockers: ["Same", "same"] }), reviewNote("close", { blockers: ["Same"] })], + }); + expect(input.blockers).toEqual(["Same"]); + }); + + it("drops empty/whitespace blocker lines in the shared extraction", () => { + const input = buildUnifiedReviewInput({ changedFiles: 1, reviews: [reviewNote("close", { blockers: ["", " ", "Real defect"] })] }); + expect(input.blockers).toEqual(["Real defect"]); + }); + + it("threads optional readiness, merged, and verdictReason through to the input", () => { + const input = buildUnifiedReviewInput({ + changedFiles: 1, + reviews: [reviewNote("merge")], + readiness: { ciState: "passed" }, + merged: true, + verdictReason: "auto-merged after green CI", + }); + expect(input.readiness).toEqual({ ciState: "passed" }); + expect(input.merged).toBe(true); + expect(input.verdictReason).toBe("auto-merged after green CI"); + }); +}); diff --git a/wrangler.jsonc b/wrangler.jsonc index d234354c27..84980338a3 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -46,6 +46,9 @@ "AI_MAX_OUTPUT_TOKENS": "256", "AI_GATEWAY_ID": "", "ADMIN_GITHUB_LOGINS": "JSONbored", + // Convergence (Stage D): render the public PR comment via the unified-comment bridge. Default OFF — + // flag-OFF keeps the legacy buildPublicPrIntelligenceComment panel byte-identical. + "UNIFIED_REVIEW_COMMENT": "false", }, "routes": [ {