Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/github/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@
repo,
issue_number: pullNumber,
labels: [labelName],
});

Check notice on line 55 in src/github/labels.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 55 in src/github/labels.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 55 in src/github/labels.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
return { applied: true, created };
}

/** Remove a single label from a PR if present. Best-effort — a 404 (label not on the PR) is ignored. Used to
* keep the mutually-exclusive managed TYPE labels (gittensor:bug/feature/priority) down to exactly one. */
export async function removePullRequestLabel(env: Env, installationId: number, repoFullName: string, pullNumber: number, labelName: string): Promise<void> {
const [owner, repo] = repoFullName.split("/");
if (!owner || !repo) return;
const token = await createInstallationToken(env, installationId);
const octokit = new Octokit({ auth: token });
await octokit
.request("DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}", { owner, repo, issue_number: pullNumber, name: labelName })
.catch(() => undefined);
}
47 changes: 43 additions & 4 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@
isMaintainerAssociation,
isMaintainerQueueDigestCommand,
parseAgentCommandFeedbackContext,
parseGittensoryMentionCommand,

Check notice on line 85 in src/queue/processors.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 85 in src/queue/processors.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 85 in src/queue/processors.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
sanitizePublicComment,
} from "../github/commands";
import { ensurePullRequestLabel } from "../github/labels";
import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels";
import { ALL_TYPE_LABELS, resolvePrTypeLabel } from "../settings/pr-type-label";
import { fetchPublicContributorProfile } from "../github/public";
import { refreshRegistry } from "../registry/sync";
import { buildIssueAdvisory, buildPullRequestAdvisory, evaluateGateCheck, isTestPath } from "../rules/advisory";
Expand Down Expand Up @@ -348,6 +349,10 @@
// so flag-OFF does zero work here too. indexRepo / reindexChangedPaths are fully fail-safe (never throw).
if (isRagEnabled(env)) await runRagIndexJob(env, message.requestedBy, message.repoFullName, message.paths);
return;
case "recapture-preview":
// Delayed visual self-poll: re-review the PR to re-capture the AFTER preview shot once its deploy is live.
await reReviewStoredPullRequest(env, message.deliveryId, message.installationId, message.repoFullName, message.prNumber, message.attempt);
break;
case "github-webhook":
await processGitHubWebhook(env, message.deliveryId, message.eventName, message.payload);
return;
Expand Down Expand Up @@ -667,7 +672,7 @@
* "the CI event WAKES the existing row and re-runs the full review". The PR's persisted head SHA is used as-is
* (never overwritten from the CI payload — reviewbot scope parity). Best-effort throughout.
*/
async function reReviewStoredPullRequest(env: Env, deliveryId: string, installationId: number, repoFullName: string, prNumber: number): Promise<void> {
async function reReviewStoredPullRequest(env: Env, deliveryId: string, installationId: number, repoFullName: string, prNumber: number, previewPollAttempt?: number): Promise<void> {
const [repo, settings] = await Promise.all([getRepository(env, repoFullName), resolveRepositorySettings(env, repoFullName)]);
const pr = await getPullRequest(env, repoFullName, prNumber);
if (!pr || pr.state !== "open") return;
Expand All @@ -677,7 +682,7 @@
if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
await refreshPullRequestDetails(env, repoFullName, prNumber).catch(() => undefined);
}
const gate = await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, { deliveryId }).catch((error) => {
const gate = await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, { deliveryId, ...(previewPollAttempt !== undefined ? { previewPollAttempt } : {}) }).catch((error) => {
console.error(JSON.stringify({ level: "warn", event: "pr_public_surface_failed", deliveryId, repository: repoFullName, pullNumber: prNumber, error: errorMessage(error) }));
return undefined;
});
Expand All @@ -693,6 +698,12 @@
// re-check still catch the settled state.
const CI_COALESCE_WINDOW_SECONDS = 60;

// Visual preview self-poll (reviewbot PREVIEW_POLL_SECONDS parity): when a PR's preview deploy isn't live at
// review time, re-review after this delay to re-capture the AFTER shot, up to MAX_PREVIEW_POLLS times (so a
// never-resolving preview can't poll forever ~ 5×90s = 7.5min).
const PREVIEW_POLL_SECONDS = 90;
const MAX_PREVIEW_POLLS = 5;

/**
* Coalesce CI-completion re-reviews: claims a per-PR window and returns true if this PR was already re-reviewed
* within CI_COALESCE_WINDOW_SECONDS (caller skips). KV-backed (REVIEW_CONFIG); a missing KV or a KV hiccup
Expand Down Expand Up @@ -1730,7 +1741,7 @@
repo: Awaited<ReturnType<typeof getRepository>>,
settings: Awaited<ReturnType<typeof getRepositorySettings>>,
advisory: Awaited<ReturnType<typeof buildPullRequestAdvisory>>,
webhook: { deliveryId: string; authorType?: string | undefined; action?: string | undefined },
webhook: { deliveryId: string; authorType?: string | undefined; action?: string | undefined; previewPollAttempt?: number | undefined },
): Promise<ReturnType<typeof evaluateGateCheck> | undefined> {
const author = pr.authorLogin ?? null;
// Per-repo cutover gate (GITTENSORY_REVIEW_REPOS): the unified converged comment renders for THIS repo
Expand Down Expand Up @@ -2164,6 +2175,17 @@
previewFromChecks: true,
}, visualFiles);
beforeAfter = capture.routes;
// Visual self-poll: the FIRST capture returns a "loading" placeholder for the AFTER shot when the
// preview deploy isn't live yet (capture.previewPending). Schedule a delayed re-review to re-capture
// the now-ready shot — bounded by `attempt` so a never-resolving preview can't loop (the deployment_status
// webhook also refills it; this is the backstop when that event is missed/late).
const previewPollAttempt = webhook.previewPollAttempt ?? 0;
if (capture.previewPending && previewPollAttempt < MAX_PREVIEW_POLLS) {
await env.JOBS.send(
{ type: "recapture-preview", deliveryId: webhook.deliveryId, repoFullName, prNumber: pr.number, installationId, attempt: previewPollAttempt + 1 },
{ delaySeconds: PREVIEW_POLL_SECONDS },
).catch((error) => console.log(JSON.stringify({ ev: "recapture_enqueue_failed", repoFullName, pull: pr.number, message: errorMessage(error).slice(0, 120) })));
}
} catch (error) {
console.log(JSON.stringify({ ev: "visual_capture_error", repoFullName, pull: pr.number, message: errorMessage(error).slice(0, 200) }));
}
Expand Down Expand Up @@ -2220,6 +2242,23 @@
failedOutputs.push({ output: "label", error: message });
await recordPublicSurfaceOutputFailure(env, "label", author, repoFullName, pr.number, webhook.deliveryId, message);
}
// Per-PR TYPE label (reviewbot auto-label parity): exactly ONE of gittensor:bug/feature/priority by the PR
// title + changed paths. Review-time + neutral, BEST-EFFORT + independent of the context label above so a
// type-label hiccup never drops the "label" output. Files are only fetched when content globs are configured
// (otherwise the label is title-derived). The status labels (ready-to-merge etc.) remain the autonomy layer's.
if (settings.autoLabelEnabled) {
try {
const contentGlobs = (settings as { contentGlobs?: string[] }).contentGlobs ?? [];
const typeFiles = contentGlobs.length > 0 ? await getReviewFiles().catch(() => [] as Awaited<ReturnType<typeof getReviewFiles>>) : [];
const chosenType = resolvePrTypeLabel({ title: pr.title, changedPaths: typeFiles.map((file) => file.path), contentGlobs });
await ensurePullRequestLabel(env, installationId, repoFullName, pr.number, chosenType, { createMissingLabel: true });
for (const other of ALL_TYPE_LABELS.filter((label) => label !== chosenType)) {
await removePullRequestLabel(env, installationId, repoFullName, pr.number, other);
}
} catch (error) {
console.log(JSON.stringify({ ev: "type_label_error", repoFullName, pull: pr.number, message: errorMessage(error).slice(0, 150) }));
}
}
}
if (publishedOutputs.length === 0) {
if (failedOutputs.length > 0) {
Expand Down
14 changes: 9 additions & 5 deletions src/review/unified-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,18 @@

const SIGNAL_ICON: Record<UnifiedSignalRow["state"], string> = { ok: "✅", warn: "⚠️", fail: "❌" };

/** Derive the single unified status from reviewbot's decision/recs/CI + the host override. */

Check notice on line 217 in src/review/unified-comment.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 217 in src/review/unified-comment.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 217 in src/review/unified-comment.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
export function deriveUnifiedStatus(input: UnifiedReviewInput, ctx: UnifiedCommentContext = {}): UnifiedCommentStatus {
if (ctx.statusOverride) return ctx.statusOverride;
// A failing CI is NEVER "safe to merge": a red CI downgrades any otherwise-ready/merge verdict to blocked (the
// disposition layer then closes it for a non-owner author / holds it open for the owner). This runs BEFORE the
// explicit-verdict switch so an optimistic gate "merge" can't render a green "safe to merge" headline over a
// red CI — the exact bug where a PR with a failing codecov/patch showed "Approved — safe to merge".
if (input.readiness?.ciState === "failed") return "blocked";
// CI gate — a PR is "safe to merge" ONLY when CI is GREEN. This runs BEFORE the explicit-verdict switch so an
// optimistic gate "merge" can never render a "safe to merge" headline over a CI that hasn't passed:
// • failed → BLOCKED (red CI; the disposition layer closes non-owner / holds owner)
// • unverified / pending (chip "CI pending") → HELD (still running / not yet reported — NOT safe to merge)
// Only ciState === "passed" falls through to honor the gate verdict. (Bug this fixes: a PR with a failing
// codecov OR with CI still in progress showed "Approved — safe to merge".)
if (input.readiness && input.readiness.ciState !== "passed") {
return input.readiness.ciState === "failed" ? "blocked" : "held";
}
// An explicit gate verdict is authoritative — it already weighed the reviewers + guardrails.
switch (input.decision) {
case "merge":
Expand Down
44 changes: 44 additions & 0 deletions src/settings/pr-type-label.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Neutral per-PR TYPE label (reviewbot src/core/auto-label.ts parity). Applies EXACTLY ONE of:

Check notice on line 1 in src/settings/pr-type-label.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 1 in src/settings/pr-type-label.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 1 in src/settings/pr-type-label.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
// gittensor:priority — a content submission (changed paths match contentGlobs) — truly valuable.
// gittensor:feature — genuine NEW functionality only (conventional-commit `feat`/`feature`).
// gittensor:bug — EVERYTHING ELSE: fix, test, docs, chore, refactor, perf, ci, build, style, revert.
// Public + neutral categorization (NOT the reputation signal). Review-time + independent of the gate /
// autonomy / dry-run (matches reviewbot, where auto-label runs at review start). Fail-safe.
import { matchesAny } from "../signals/change-guardrail";

export interface PrTypeLabelSet {
bug: string;
feature: string;
priority: string;
}

/** The gittensor: namespace the maintainer uses. The three are mutually exclusive (the other two are dropped). */
export const DEFAULT_TYPE_LABELS: PrTypeLabelSet = {
bug: "gittensor:bug",
feature: "gittensor:feature",
priority: "gittensor:priority",
};

export const ALL_TYPE_LABELS: readonly string[] = [DEFAULT_TYPE_LABELS.bug, DEFAULT_TYPE_LABELS.feature, DEFAULT_TYPE_LABELS.priority];

/** feature ONLY for genuine new functionality (feat); EVERYTHING else — fix, test, docs, chore, refactor,
* perf, ci, build, style, revert — is bug (a test PR is a test, not a feature). (reviewbot auto-label.ts:27) */
export function deriveKindFromTitle(title: string | undefined): "bug" | "feature" {
const match = /^([a-zA-Z]+)/.exec((title ?? "").trim());
const type = match?.[1]?.toLowerCase();
return type === "feat" || type === "feature" ? "feature" : "bug";
}

/**
* Resolve the single TYPE label for a PR (priority order):
* 1. CONTENT submission — any changed path matches a contentGlob → priority.
* 2. else feature (feat) / bug (everything else) by the conventional-commit title prefix.
* Pure + total. Returns the chosen label name.
*/
export function resolvePrTypeLabel(input: { title: string | undefined; changedPaths: string[]; contentGlobs: string[]; labels?: PrTypeLabelSet }): string {
const labels = input.labels ?? DEFAULT_TYPE_LABELS;
if (input.contentGlobs.length > 0 && input.changedPaths.some((path) => matchesAny(path, input.contentGlobs))) {
return labels.priority;
}
return labels[deriveKindFromTitle(input.title)];
}
11 changes: 11 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,20 @@
| {
type: "github-webhook";
deliveryId: string;
eventName: string;

Check notice on line 8 in src/types.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 8 in src/types.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 8 in src/types.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
payload: GitHubWebhookPayload;
}
| {
// Delayed self-poll to re-capture a PR's before/after preview once its preview deploy is live — the first
// review captures a "loading" placeholder when the deploy isn't ready yet (capture.previewPending). Each
// recapture re-reviews the PR; bounded by `attempt` so a never-resolving preview can't loop forever.
type: "recapture-preview";
deliveryId: string;
repoFullName: string;
prNumber: number;
installationId: number;
attempt: number;
}
| {
type: "refresh-registry";
requestedBy: "schedule" | "api" | "test";
Expand Down
29 changes: 29 additions & 0 deletions test/unit/pr-type-label.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { describe, expect, it } from "vitest";

Check notice on line 1 in test/unit/pr-type-label.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 1 in test/unit/pr-type-label.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 1 in test/unit/pr-type-label.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
import { DEFAULT_TYPE_LABELS, deriveKindFromTitle, resolvePrTypeLabel } from "../../src/settings/pr-type-label";

describe("deriveKindFromTitle", () => {
it("maps feat/feature → feature; everything else → bug", () => {
expect(deriveKindFromTitle("feat: add X")).toBe("feature");
expect(deriveKindFromTitle("feature(api): boards")).toBe("feature");
expect(deriveKindFromTitle("fix: bug")).toBe("bug");
expect(deriveKindFromTitle("test: add coverage")).toBe("bug");
expect(deriveKindFromTitle("docs: readme")).toBe("bug");
expect(deriveKindFromTitle("chore: deps")).toBe("bug");
expect(deriveKindFromTitle("refactor: cleanup")).toBe("bug");
expect(deriveKindFromTitle(undefined)).toBe("bug");
expect(deriveKindFromTitle("")).toBe("bug");
});
});

describe("resolvePrTypeLabel", () => {
it("returns the feature/bug label by title when no content globs match", () => {
expect(resolvePrTypeLabel({ title: "feat: x", changedPaths: ["src/a.ts"], contentGlobs: [] })).toBe(DEFAULT_TYPE_LABELS.feature);
expect(resolvePrTypeLabel({ title: "fix: y", changedPaths: ["src/a.ts"], contentGlobs: [] })).toBe(DEFAULT_TYPE_LABELS.bug);
});

it("returns priority when a changed path matches a content glob (content submission)", () => {
expect(resolvePrTypeLabel({ title: "feat: add entry", changedPaths: ["content/posts/x.md"], contentGlobs: ["content/**"] })).toBe(DEFAULT_TYPE_LABELS.priority);
// a non-content feat with content globs configured but no match → feature
expect(resolvePrTypeLabel({ title: "feat: code", changedPaths: ["src/a.ts"], contentGlobs: ["content/**"] })).toBe(DEFAULT_TYPE_LABELS.feature);
});
});
10 changes: 7 additions & 3 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3484,10 +3484,11 @@
installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } },
pull_request: { number: 46, title: "Miner label-only follow-up", state: "open", user: { login: "oktofeesh1" }, labels: [], body: "Fixes #1" },
},

Check notice on line 3487 in test/unit/queue.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 3487 in test/unit/queue.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 3487 in test/unit/queue.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
});

expect(calls).toEqual({ comments: 0, labels: 2, minerList: 1 });
// 2 PRs × 3 label POSTs each: the gittensor context label (apply) + the per-PR TYPE label (create + apply).
expect(calls).toEqual({ comments: 0, labels: 6, minerList: 1 });
const cacheAudit = await env.DB.prepare("select event_type, detail from audit_events where actor = ? order by created_at")
.bind("oktofeesh1")
.all<{ event_type: string; detail: string | null }>();
Expand Down Expand Up @@ -3558,7 +3559,9 @@
}),
).resolves.toBeUndefined();

expect(calls).toEqual({ comments: 0, labels: 1 });
// gittensor context-label apply (fails 503, recorded) + the best-effort type-label create attempt (also 503,
// swallowed). The context-label failure is still recorded below; the type label never drops the recording.
expect(calls).toEqual({ comments: 0, labels: 2 });
const outputFailure = await env.DB.prepare("select event_type, detail from audit_events where event_type = ?")
.bind("github_app.pr_label_publish_failed")
.first<{ event_type: string; detail: string }>();
Expand Down Expand Up @@ -3851,7 +3854,8 @@
},
});

expect(calls).toEqual({ minerList: 2, labels: 1 });
// 1 labeled PR × 3 label POSTs: the gittensor context label (apply) + the per-PR TYPE label (create + apply).
expect(calls).toEqual({ minerList: 2, labels: 3 });
const cached = await env.DB.prepare("select status, snapshot_json from official_miner_detections where login = ?")
.bind("oktofeesh1")
.first<{ status: string; snapshot_json: string }>();
Expand Down
6 changes: 4 additions & 2 deletions test/unit/unified-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,16 @@

it("held for manual / request_changes", () => {
expect(deriveUnifiedStatus({ ...base, decision: "manual" })).toBe("held");
expect(deriveUnifiedStatus({ ...base, recommendations: ["request_changes"] })).toBe("held");

Check notice on line 36 in test/unit/unified-comment.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Issue discovery is disabled for this repo

This repo is configured for direct contribution review rather than issue-discovery flow.

Check notice on line 36 in test/unit/unified-comment.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

Open PR queue is busy

This repo has a busy open PR queue in the local Gittensory cache.

Check notice on line 36 in test/unit/unified-comment.test.ts

View check run for this annotation

Deleted GitHub App / Gittensory Context

PR author has maintainer association

This PR appears to come from a maintainer-associated account.
});

it("a failing CI is BLOCKED (never safe-to-merge) and overrides an optimistic merge verdict", () => {
it("CI that hasn't passed is NEVER safe-to-merge — failed→blocked, pending/unverified→held, even over a merge verdict", () => {
// A red CI must never render "safe to merge". It downgrades even an explicit `merge` verdict to blocked.
expect(deriveUnifiedStatus({ ...base, readiness: { ciState: "failed" } })).toBe("blocked");
expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "failed" } })).toBe("blocked");
// green CI + merge verdict still renders ready.
// CI still running / not yet reported (chip "CI pending") → HELD, never "safe to merge".
expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "unverified" } })).toBe("held");
// ONLY green CI + a merge verdict renders ready.
expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } })).toBe("ready");
});

Expand Down
Loading