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
13 changes: 7 additions & 6 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ import {
buildPullRequestAdvisory,
evaluateGateCheck,
} from "../rules/advisory";
import { isTestPath } from "../signals/test-evidence";
import { hasValidationNote, isTestPath } from "../signals/test-evidence";
import { detectNotificationEvents } from "../notifications/events";
import {
deliverNotification,
Expand Down Expand Up @@ -832,13 +832,14 @@ export async function runRetentionPrune(
const PUBLIC_MANIFEST_POLICY_FINDING_OVERRIDES: Partial<
Record<
FocusManifestFinding["code"],
Pick<AdvisoryFinding, "detail" | "action">
Pick<AdvisoryFinding, "title" | "detail" | "action">
>
> = {
manifest_missing_tests: {
detail: "Maintainer test expectations are not satisfied by this PR.",
title: "Configured validation evidence missing",
detail: "No changed test files or passing validation evidence were detected for this PR.",
action:
"Add or update tests, or attach passing validation output that satisfies the maintainer's test expectations.",
"Add regression/invariant coverage, update relevant tests, or attach passing validation output that satisfies the repo's configured expectations.",
},
};

Expand All @@ -852,7 +853,7 @@ export function publicSafeManifestPolicyFinding(
detail: finding.detail,
/* v8 ignore next -- the three manifest policy findings always carry an action; the no-action arm is unreachable. */
...(finding.action !== undefined ? { action: finding.action } : {}),
// Override the leaky detail/action with a static, public-safe version for the codes whose raw text would echo
// Override the leaky title/detail/action with static, public-safe text for codes whose raw text would echo
// private blocked-path globs / test expectations; codes absent from the table keep their already-generic text.
...PUBLIC_MANIFEST_POLICY_FINDING_OVERRIDES[finding.code],
};
Expand Down Expand Up @@ -7610,7 +7611,7 @@ async function maybePublishPrPublicSurface(
linkedIssueCount: pr.linkedIssues.length,
testFileCount: manifestFiles.filter((file) => isTestPath(file.path))
.length,
passedValidationCount: 0,
passedValidationCount: hasValidationNote(pr.body ?? "") ? 1 : 0,
});
const policyCodes = new Set([
"manifest_blocked_path",
Expand Down
7 changes: 5 additions & 2 deletions src/rules/predicted-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const OSS_ANTI_SLOP_FUNNEL = {
registerUrl: GITTENSOR_HOME_URL,
} as const;
import { buildPullRequestAdvisory, evaluateGateCheck, type GateCheckConclusion } from "./advisory";
import { isTestPath } from "../signals/test-evidence";
import { hasValidationNote, isTestPath } from "../signals/test-evidence";
import { evaluateClaCheck } from "../review/cla-check";
import { evaluatePreMergeChecks } from "../review/pre-merge-checks";

Expand Down Expand Up @@ -236,7 +236,10 @@ export function buildPredictedGateVerdict(args: {
labels: syntheticPr.labels,
linkedIssueCount: syntheticPr.linkedIssues.length,
testFileCount: changedPaths.filter((path) => isTestPath(path)).length,
passedValidationCount: 0,
// Parity with the live gate (queue/processors.ts's manifestPolicyGateMode block): the predictor
// already has the same PR body available via input.body, so a manifest_missing_tests prediction must
// not stay stuck at "no validation evidence" when the real gate would already treat the body as evidence.
passedValidationCount: hasValidationNote(input.body ?? "") ? 1 : 0,
});
const policyCodes = new Set(["manifest_linked_issue_required", "manifest_missing_tests"]);
for (const finding of guidance.findings) {
Expand Down
19 changes: 15 additions & 4 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { AgentActionClass, AutoMaintainPolicy, AutoMergeMethod, AutonomyPolicy } from "../types";
import { AI_JUDGMENT_BLOCKER_CODES, type GateCheckConclusion } from "../rules/advisory";
import { DEFAULT_AUTO_MAINTAIN_POLICY, autonomyRequiresApproval, isActingAutonomyLevel, resolveAutonomy } from "./autonomy";
import { isGuardrailHit } from "../signals/change-guardrail";
import { changedPathsHittingGuardrail, isGuardrailHit } from "../signals/change-guardrail";
import { AGENT_LABEL_PENDING_CLOSURE } from "../review/linked-issue-hard-rules";
import { sanitizePublicComment } from "../github/commands";

Expand Down Expand Up @@ -357,6 +357,13 @@ function resolveAgentDispositionLabels(settings: AgentDispositionLabelSettings):
};
}

function guardrailHoldReason(changedPaths: string[], hardGuardrailGlobs: string[]): string {
const matches = changedPathsHittingGuardrail(changedPaths, hardGuardrailGlobs);
if (matches.length === 0) return "guarded path -> manual review (changed-file list unavailable)";
const visible = matches.slice(0, 3).map((path) => `\`${path}\``).join(", ");
return `guarded path -> manual review (${visible}${matches.length > 3 ? `, and ${matches.length - 3} more` : ""})`;
}

/**
* Accuracy circuit-breaker (#self-improve / GAP-4): when auto-merge is DISABLED for a repo (the auto-tuner
* engaged the holdonly flag after merge precision dropped, or a human set it), DOWNGRADE a would-MERGE into a
Expand Down Expand Up @@ -620,6 +627,10 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
// never auto-merge, auto-approve, or auto-close a PR whose diff we don't know. Repos with no guardrails
// configured stay permissive.
const guardrailHit = isGuardrailHit(input.changedPaths, input.hardGuardrailGlobs);
// Every read site below is itself gated on guardrailHit being true, so this default is never actually
// read -- it exists only so guardrailReason stays a plain string instead of forcing a `?? fallback` at
// every call site (each of which would be an untestable, permanently-unreachable branch).
const guardrailReason = guardrailHit ? guardrailHoldReason(input.changedPaths, input.hardGuardrailGlobs) : "guarded path -> manual review";
// Manual review is the RARE exception (the operator's minimize-manual goal): the ONLY things that hold a PR
// for a human instead of merge/close are an auto-merge-ready PR that touches a hard-guardrail path, or a
// live migration-number collision detected against the CURRENT tip of the base branch (#2550 — a sibling PR
Expand Down Expand Up @@ -702,7 +713,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
actionClass: "label",
autonomyClass: "merge",
requiresApproval: false,
reason: `verdict=${conclusion}; guarded path → manual review`,
reason: `verdict=${conclusion}; ${guardrailReason}`,
label: labels.manualReview,
labelOp: "add",
});
Expand All @@ -727,7 +738,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
: input.migrationCollisionHold !== undefined
? `verdict=${conclusion}; ${input.migrationCollisionHold.reason}`
: heldForManualReview
? `verdict=${conclusion}; guarded path → manual review`
? `verdict=${conclusion}; ${guardrailReason}`
: `verdict=${conclusion}; CI green`;
if (label !== null && !hasLabelOrPlanned(input.pr.labels, actions, label)) {
actions.push({
Expand Down Expand Up @@ -913,7 +924,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
// review-good-but-not-yet-mergeable → held briefly (rebase/approve resolves it next pass).
const manualHoldReason =
guardrailHit
? `verdict=${conclusion}; guarded path → manual review`
? `verdict=${conclusion}; ${guardrailReason}`
: ciUnverified
? "CI could not be verified"
: conclusion === "action_required"
Expand Down
6 changes: 1 addition & 5 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import type { GittensorContributorSnapshot } from "../gittensor/api";
import { nowIso } from "../utils/json";
import { sanitizePublicComment } from "../queue-intelligence";
import { labelMatchesPattern, projectLinkedIssueMultiplierForPlannedSolve, type LinkedIssueMultiplierStatus } from "../scoring/preview";
import { hasLocalTestEvidence, isTestPath } from "./test-evidence";
import { hasLocalTestEvidence, hasValidationNote, isTestPath } from "./test-evidence";
import { isFailingCheckSummary } from "./local-branch";
import { isDuplicateClusterWinnerByClaim } from "./duplicate-winner";
import { PREFLIGHT_LIMITS } from "./preflight-limits";
Expand Down Expand Up @@ -5012,10 +5012,6 @@ export function hasClearNoIssueRationale(pr: Pick<PullRequestRecord, "title" | "
return /\b(?:no issue\s*(?:because\b|:)|no linked issue\s*(?:because\b|:)|no ticket\s*(?:because\b|:)|(?:maintenance|docs?[\s-]+only|tests?[\s-]+only|ci[\s-]+only|refactor[\s-]+only|typo|chore|cleanup)\b)/i.test([pr.title, pr.body ?? ""].join(" "));
}

function hasValidationNote(value: string): boolean {
return /\b(test(?:ed|s|ing)?|validation|validated|verified|manual check|smoke|pytest|vitest|npm test|pnpm test|cargo test|go test)\b/i.test(value);
}

function gateStatus(gateEnabled: boolean, conclusion: PublicPrPanelGateEvaluation["conclusion"]): string {
if (!gateEnabled) return "⚠️ Advisory only";
if (conclusion === "success") return "✅ Passing";
Expand Down
10 changes: 6 additions & 4 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2132,14 +2132,16 @@ export function buildFocusManifestGuidance(args: {
}

if (manifest.testExpectations.length > 0 && testFileCount === 0 && passedValidationCount === 0) {
const safeExpectations = manifest.testExpectations.filter(isFocusManifestPublicSafe).slice(0, 3);
const expectationDetail = safeExpectations.length > 0 ? ` Expected evidence: ${safeExpectations.join("; ")}.` : "";
findings.push({
code: "manifest_missing_tests",
severity: "warning",
title: "Maintainer test expectations unmet",
detail: `Maintainer expects test evidence: ${manifest.testExpectations.slice(0, 3).join("; ")}.`,
action: "Add or update tests, or attach passing validation output that satisfies the maintainer's test expectations.",
title: "Configured validation evidence missing",
detail: `No changed test files or passing validation evidence were detected for this PR.${expectationDetail}`,
action: "Add regression/invariant coverage, update relevant tests, or attach passing validation output that satisfies the repo's configured expectations.",
});
publicNextSteps.push("Add tests or attach passing validation that meets the maintainer's test expectations.");
publicNextSteps.push("Add relevant tests or passing validation evidence that matches the repo's configured expectations.");
}

if (manifest.issueDiscoveryPolicy === "discouraged") {
Expand Down
41 changes: 41 additions & 0 deletions src/signals/test-evidence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,47 @@ export function hasLocalTestEvidence(input: { tests?: string[] | undefined; test
return (input.tests ?? []).length > 0 || (input.testFiles ?? []).some((file) => isTestPath(file));
}

// A body can mention testing without having actually done it ("No tests run", "Tests not run", "Not
// tested locally", "did not run any tests") -- the affirmative keyword match below would otherwise treat
// that as passing evidence and let a configured manifest test expectation silently disappear. Rather than
// enumerate ever more literal phrase templates (which a previous version of this function tried, and which
// still missed "Not tested" because its test-noun list didn't include the verb form "tested"), detect
// negation by PROXIMITY: a negation word within a few words of a test/validation stem, in either order,
// with a shared stem definition so the "is this a test/validation mention at all" question is answered
// exactly once. The filler between the negation word and the stem may not cross a clause/sentence boundary
// (a comma/period/exclamation/question mark/semicolon), so an unrelated "not" earlier in the body (e.g.
// "This is not a breaking change. Tested with npm run test:ci.") cannot suppress a later, unrelated
// affirmative note.
const TEST_STEM = "(?:test(?:ed|s|ing)?|validat(?:ion|ed)|verif(?:y|ied|ying)|manual check|smoke(?:\\s+tests?)?)";
const NEGATION_WORD = "(?:no|not|never|without|skip(?:ped)?|didn't|doesn't|isn't|wasn't|weren't|haven't|hasn't)";
const NEGATION_CONTINUATION = "(?:not|never|failed|failing|skipped|incomplete)";
const SAME_SENTENCE_FILLER_WORD = "[^\\s.,!?;]+";

const NEGATES_BEFORE_TEST_STEM = new RegExp(`\\b${NEGATION_WORD}\\b(?:\\s+${SAME_SENTENCE_FILLER_WORD}){0,3}\\s+${TEST_STEM}\\b`, "i");
const NEGATES_AFTER_TEST_STEM = new RegExp(`\\b${TEST_STEM}\\b(?:\\s+${SAME_SENTENCE_FILLER_WORD}){0,2}\\s+${NEGATION_CONTINUATION}\\b`, "i");
// A compound negated adjective with no separating whitespace at all ("untested", "unvalidated", "unverified").
const NEGATES_TEST_STEM_PREFIX = /\bun(?:tested|validated|verified)\b/i;

const AFFIRMATIVE_TEST_MENTION = /\b(test(?:ed|s|ing)?|validation|validated|verified|manual check|smoke|pytest|vitest|npm test|pnpm test|cargo test|go test)\b/i;

// A body can contain BOTH a genuine negated clause ("No tests run locally.") and a separate, later clause
// with real affirmative evidence ("Validated with npm run test:ci.") -- evaluating the negation checks
// against the WHOLE body would let the first clause veto the second, discarding real evidence the manifest
// gate is specifically trying to detect (#3304, round 3). Split on the same clause-boundary punctuation the
// proximity checks already treat as a hard stop, and require at least one clause to be an affirmative,
// non-negated mention -- so an earlier honest "no tests" disclosure can no longer suppress later evidence.
export function hasValidationNote(value: string): boolean {
return value
.split(/[.,!?;]+/)
.some(
(clause) =>
!NEGATES_TEST_STEM_PREFIX.test(clause) &&
!NEGATES_BEFORE_TEST_STEM.test(clause) &&
!NEGATES_AFTER_TEST_STEM.test(clause) &&
AFFIRMATIVE_TEST_MENTION.test(clause),
);
}

/**
* Coarse classification of how much test coverage accompanies a set of changed paths.
* Used by slop signals to weight diffs that touch source but include no tests differently
Expand Down
27 changes: 27 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,36 @@ describe("planAgentMaintenanceActions (#778)", () => {
expect(label?.label).toBe(AGENT_LABEL_NEEDS_REVIEW);
expect(label?.label).not.toBe(AGENT_LABEL_READY);
expect(label?.reason).toContain("guarded path");
expect(label?.reason).toContain("src/scoring/model.ts");
expect(classes(plan)).not.toContain("merge");
});

it("truncates the guardrail hold reason to 3 visible paths and counts the rest (#3304)", () => {
const label = planAgentMaintenanceActions(input({
conclusion: "success",
autonomy: { merge: "auto" },
changedPaths: ["src/scoring/a.ts", "src/scoring/b.ts", "src/scoring/c.ts", "src/scoring/d.ts", "src/scoring/e.ts"],
hardGuardrailGlobs: ["src/scoring/**"],
pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" },
})).find((a) => a.actionClass === "label");
expect(label?.reason).toContain("src/scoring/a.ts");
expect(label?.reason).toContain("src/scoring/c.ts");
expect(label?.reason).not.toContain("src/scoring/d.ts");
expect(label?.reason).toContain("and 2 more");
});

it("explains when guardrail hold is fail-closed because changed files are unavailable", () => {
const label = planAgentMaintenanceActions(input({
conclusion: "success",
autonomy: { merge: "auto" },
changedPaths: [],
hardGuardrailGlobs: ["src/scoring/**"],
pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" },
})).find((a) => a.actionClass === "label");
expect(label?.label).toBe(AGENT_LABEL_NEEDS_REVIEW);
expect(label?.reason).toContain("changed-file list unavailable");
});

it("does not re-plan the manual-review label when the guarded PR already carries it (idempotent)", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { review_state_label: "auto" }, ...guarded, pr: { labels: [AGENT_LABEL_NEEDS_REVIEW] } })));
expect(plan).not.toContain("label");
Expand Down
19 changes: 18 additions & 1 deletion test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,24 @@ describe("buildFocusManifestGuidance", () => {
it("surfaces missing preferred labels and test expectations", () => {
const guidance = buildFocusManifestGuidance({ manifest: wanted, changedPaths: ["src/x.ts"], labels: [], linkedIssueCount: 1, testFileCount: 0, passedValidationCount: 0 });
expect(guidance.findings.some((finding) => finding.code === "manifest_missing_preferred_label")).toBe(true);
expect(guidance.findings.some((finding) => finding.code === "manifest_missing_tests")).toBe(true);
const missingTests = guidance.findings.find((finding) => finding.code === "manifest_missing_tests");
expect(missingTests).toMatchObject({
title: "Configured validation evidence missing",
detail: expect.stringContaining("No changed test files or passing validation evidence were detected"),
action: "Add regression/invariant coverage, update relevant tests, or attach passing validation output that satisfies the repo's configured expectations.",
});
expect(missingTests?.detail).toContain("unit tests for new branches.");
});

it("omits the 'Expected evidence' detail when every test expectation is public-unsafe (#3304)", () => {
// testExpectations.length > 0 still trips the finding, but the public-safe filter drops the only entry,
// so the detail must fall back to the base sentence with no "Expected evidence: ..." suffix appended.
const unsafeManifest = parseFocusManifest({ testExpectations: ["Submit your wallet seed phrase"] });
const guidance = buildFocusManifestGuidance({ manifest: unsafeManifest, changedPaths: ["src/x.ts"], linkedIssueCount: 1, testFileCount: 0, passedValidationCount: 0 });
const missingTests = guidance.findings.find((finding) => finding.code === "manifest_missing_tests");
expect(missingTests?.detail).toBe("No changed test files or passing validation evidence were detected for this PR.");
expect(missingTests?.detail).not.toContain("Expected evidence");
expect(missingTests?.detail).not.toContain("wallet");
});

it("treats passing validation as satisfying test expectations", () => {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/gate-check-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ describe("focus-manifest policy gate (#555)", () => {
// Path-based manual review lives in settings.hardGuardrailGlobs, not manifest policy.
const POLICY_FINDINGS = {
manifest_linked_issue_required: { code: "manifest_linked_issue_required", title: "Maintainer requires a linked issue", severity: "warning" as const, detail: "Manifest requires a linked issue.", action: "Link the issue." },
manifest_missing_tests: { code: "manifest_missing_tests", title: "Maintainer test expectations unmet", severity: "warning" as const, detail: "Manifest expects test evidence.", action: "Add tests." },
manifest_missing_tests: { code: "manifest_missing_tests", title: "Configured validation evidence missing", severity: "warning" as const, detail: "No changed test files or passing validation evidence were detected.", action: "Add regression/invariant coverage, update relevant tests, or attach passing validation output." },
};

function manifestAdvisory(code: keyof typeof POLICY_FINDINGS): Advisory {
Expand Down
Loading
Loading