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: 11 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -878,10 +878,20 @@ export async function fanOutAgentRegateSweepJobs(
// outcome for just this repo (it gets picked up again next tick) instead of rejecting.
try {
const settings = await resolveRepositorySettings(env, repoFullName);
// #sweep-requires-installation: isAgentConfigured resolves the OPERATOR'S global-default autonomy
// (e.g. a self-host `.gittensory.yml` settings.autonomy block meant for the repos this instance
// actually operates on) for ANY repoFullName, regardless of whether the GitHub App is installed
// there. A repo that merely has a local `repositories` row (a stray subnet-registry row, say) with
// no real `installationId` would otherwise inherit that global default and look "agent-configured"
// purely by existing — even though no installation token exists to act on it, and it was never
// intentionally onboarded. Require a real installation before the autonomy-based path can make a
// repo eligible; the explicit allowlist path is untouched (GITTENSORY_REVIEW_REPOS is a deliberate,
// operator-typed signal independent of installation state, e.g. reviewing ahead of a pending install).
const hasInstallation = typeof repo.installationId === "number";
if (
!(
isConvergenceRepoAllowed(env, repoFullName) ||
isAgentConfigured(settings.autonomy)
(hasInstallation && isAgentConfigured(settings.autonomy))
)
)
return { kind: "ineligible" };
Expand Down
4 changes: 4 additions & 0 deletions src/review/maintainer-recap-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ async function recapScanRepos(env: Env): Promise<string[]> {
const configured: string[] = [];
for (const repo of repos) {
try {
// #sweep-requires-installation: a repo with no real GitHub App installation must never be treated as
// agent-configured purely because it resolves the operator's global-default autonomy by merely having
// a local row -- mirrors fanOutAgentRegateSweepJobs's own guard.
if (typeof repo.installationId !== "number") continue;
const settings = await resolveRepositorySettings(env, repo.fullName);
if (isAgentConfigured(settings.autonomy)) configured.push(repo.fullName);
} catch {
Expand Down
4 changes: 4 additions & 0 deletions src/review/ops-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ async function opsScanRepos(env: Env): Promise<string[]> {
const configured: string[] = [];
for (const repo of repos) {
try {
// #sweep-requires-installation: a repo with no real GitHub App installation must never be treated as
// agent-configured purely because it resolves the operator's global-default autonomy by merely having
// a local row -- mirrors fanOutAgentRegateSweepJobs's own guard.
if (typeof repo.installationId !== "number") continue;
const settings = await resolveRepositorySettings(env, repo.fullName);
if (isAgentConfigured(settings.autonomy)) configured.push(repo.fullName);
} catch {
Expand Down
7 changes: 6 additions & 1 deletion src/review/pr-reconciliation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ async function watchedRepos(env: Env): Promise<Array<{ fullName: string; install
for (const repo of byKey.values()) {
try {
const settings = await resolveRepositorySettings(env, repo.fullName);
if (isConvergenceRepoAllowed(env, repo.fullName) || isAgentConfigured(settings.autonomy)) configured.push(repo);
// #sweep-requires-installation: isAgentConfigured resolves the operator's global-default autonomy for
// ANY repoFullName -- a repo that merely has a local row (no real GitHub App installation) would
// otherwise inherit that default and look "agent-configured" purely by existing. Require a real
// installation before the autonomy-based path counts; the explicit allowlist stays untouched.
const hasInstallation = typeof repo.installationId === "number";
if (isConvergenceRepoAllowed(env, repo.fullName) || (hasInstallation && isAgentConfigured(settings.autonomy))) configured.push(repo);
} catch {
/* a settings blip on one repo must not abort the whole reconciliation scan */
}
Expand Down
4 changes: 4 additions & 0 deletions src/review/selftune-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ async function selfTuneRepos(env: Env): Promise<string[]> {
const configured: string[] = [];
for (const repo of repos) {
try {
// #sweep-requires-installation: a repo with no real GitHub App installation must never be treated as
// agent-configured purely because it resolves the operator's global-default autonomy by merely having
// a local row -- mirrors fanOutAgentRegateSweepJobs's own guard.
if (typeof repo.installationId !== "number") continue;
const settings = await resolveRepositorySettings(env, repo.fullName);
if (!isAgentConfigured(settings.autonomy)) continue;
const manifest = await loadRepoFocusManifest(env, repo.fullName).catch(() => null);
Expand Down
6 changes: 5 additions & 1 deletion src/review/sweep-watchdog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ async function watchedRepos(env: Env): Promise<Array<{ fullName: string; install
for (const repo of byKey.values()) {
try {
const settings = await resolveRepositorySettings(env, repo.fullName);
if (isConvergenceRepoAllowed(env, repo.fullName) || isAgentConfigured(settings.autonomy)) configured.push(repo);
// #sweep-requires-installation: mirrors fanOutAgentRegateSweepJobs's own guard -- a repo with no real
// GitHub App installation must never be treated as agent-configured purely because it resolves the
// operator's global-default autonomy by merely having a local row.
const hasInstallation = typeof repo.installationId === "number";
if (isConvergenceRepoAllowed(env, repo.fullName) || (hasInstallation && isAgentConfigured(settings.autonomy))) configured.push(repo);
} catch {
/* a settings blip on one repo must not abort the whole watchdog scan */
}
Expand Down
11 changes: 7 additions & 4 deletions test/unit/maintainer-recap-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ function poisonDbPrepare(env: Env, pattern: RegExp): void {
}

// Mark a repo registered so recapScanRepos picks it up (mirrors ops-wire.test.ts's seedRegisteredRepo).
async function seedRegisteredRepo(env: Env, fullName: string): Promise<void> {
async function seedRegisteredRepo(env: Env, fullName: string, installationId?: number): Promise<void> {
const [owner, name] = fullName.split("/");
// installationId is only needed by tests that rely on the acting-autonomy selection path
// (#sweep-requires-installation): a repo with no real installation is never treated as agent-configured,
// regardless of its resolved autonomy settings.
await (env.DB as unknown as { prepare: (s: string) => { bind: (...v: unknown[]) => { run: () => Promise<unknown> } } })
.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, ?, ?, 1, 1)")
.bind(fullName, owner, name)
.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered, installation_id) VALUES (?, ?, ?, 1, 1, ?)")
.bind(fullName, owner, name, installationId ?? null)
.run();
}

Expand Down Expand Up @@ -231,7 +234,7 @@ describe("runMaintainerRecapJob — cross-repo digest (#1963, #2248)", () => {

it("prefers agent-configured repos over the full registered set when at least one is configured", async () => {
const env = createTestEnv({ DISCORD_WEBHOOK_URL: HOOK });
await seedRegisteredRepo(env, "owner/configured");
await seedRegisteredRepo(env, "owner/configured", 9402);
await seedMergedPr(env, "owner/configured", 1);
await upsertRepositorySettings(env, { repoFullName: "owner/configured", autonomy: { merge: "auto" } });
await seedRegisteredRepo(env, "owner/unconfigured");
Expand Down
28 changes: 28 additions & 0 deletions test/unit/ops-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,19 @@ async function seedRegisteredRepo(env: Env, fullName: string): Promise<void> {
.run();
}

// A registered repo with a REAL installation + acting-autonomy settings, so opsScanRepos's "prefer
// agent-configured repos" branch picks it up (#sweep-requires-installation: installation_id is required
// alongside the autonomy row, matching the real upsertRepositoryFromGitHub invariant).
async function seedAgentConfiguredRepo(env: Env, fullName: string, installationId: number): Promise<void> {
const [owner, name] = fullName.split("/");
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered, installation_id) VALUES (?, ?, ?, 1, 1, ?)")
.bind(fullName, owner, name, installationId)
.run();
await env.DB.prepare("INSERT INTO repository_settings (repo_full_name, autonomy_json) VALUES (?, ?)")
.bind(fullName, JSON.stringify({ review: "auto" }))
.run();
}

// Seed a gate-block ledger anomaly: blocked PRs that later MERGED (false positives) over the min sample.
async function seedGateFalsePositiveAnomaly(env: Env, repoFullName: string): Promise<void> {
for (let i = 1; i <= 6; i += 1) {
Expand Down Expand Up @@ -228,6 +241,21 @@ describe("runOpsAlerts — cron path over gittensory's outcome data", () => {
expect(errors.mock.calls.map((c) => String(c[0])).some((line) => line.includes("ops_anomaly\""))).toBe(false);
});

it("REGRESSION (#sweep-requires-installation): prefers the agent-configured repo and never scans an uninstalled registered repo when a configured one exists", async () => {
const env = createTestEnv();
await seedAgentConfiguredRepo(env, "owner/configured", 9501);
await seedGateFalsePositiveAnomaly(env, "owner/configured");
// Registered, but no real installation -- must not count as "agent-configured" merely by resolving the
// operator's global-default autonomy, and must be excluded from the scan once a configured repo exists.
await seedRegisteredRepo(env, "owner/no-install");
await seedGateFalsePositiveAnomaly(env, "owner/no-install");

const found = await runOpsAlerts(env);

expect(found["owner/configured"]?.some((a) => /gate false-positive spike/.test(a))).toBe(true);
expect(found["owner/no-install"]).toBeUndefined();
});

it("detects and reports a review burst end-to-end (a PR published far more review surfaces than normal in the window)", async () => {
setSelfHostedMetricsMode(true); // keep the repo label so the counter assertion can target the exact series
const env = createTestEnv();
Expand Down
41 changes: 32 additions & 9 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -888,9 +888,11 @@ describe("queue processors", () => {
},
} as unknown as Queue,
});
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "plain-repo", full_name: "owner/plain-repo", private: false, owner: { login: "owner" } });
// #sweep-requires-installation: acting-autonomy eligibility also requires a real installation now — give
// the repos that SHOULD be swept a real installationId, mirroring an actually-installed repo.
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } }, 9201);
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } }, 9202);
await upsertRepositoryFromGitHub(env, { name: "plain-repo", full_name: "owner/plain-repo", private: false, owner: { login: "owner" } }, 9203);
await upsertRepositorySettings(env, { repoFullName: "owner/agent-a", autonomy: { label: "auto" } });
await upsertRepositorySettings(env, { repoFullName: "owner/agent-b", autonomy: { merge: "auto_with_approval" } });
await upsertRepositorySettings(env, { repoFullName: "owner/plain-repo", autonomy: { review: "observe" } }); // non-acting → not configured
Expand All @@ -908,6 +910,27 @@ describe("queue processors", () => {
expect(JSON.parse(fanout?.metadata_json ?? "{}")).toMatchObject({ repoCount: 2, requestedBy: "schedule" });
});

it("REGRESSION (#sweep-requires-installation): an acting-autonomy repo with NO real installation is excluded, even though it resolves the operator's global-default autonomy", async () => {
const sent: import("../../src/types").JobMessage[] = [];
const env = createTestEnv({
GITTENSORY_REVIEW_REPOS: "",
JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue,
});
// A repo that merely EXISTS locally (e.g. a stray row from an unrelated sync) with a real installation and
// acting autonomy — sweep-eligible.
await upsertRepositoryFromGitHub(env, { name: "installed-repo", full_name: "owner/installed-repo", private: false, owner: { login: "owner" } }, 9301);
await upsertRepositorySettings(env, { repoFullName: "owner/installed-repo", autonomy: { merge: "auto" } });
// Same acting autonomy, but NO installationId (never upserted with one) — this is exactly the shape a
// stray repositories row takes: it can still resolve a permissive global-default autonomy policy, but
// there is no GitHub App installation on it at all, so it must never be treated as agent-configured.
await upsertRepositoryFromGitHub(env, { name: "uninstalled-repo", full_name: "owner/uninstalled-repo", private: false, owner: { login: "owner" } });
await upsertRepositorySettings(env, { repoFullName: "owner/uninstalled-repo", autonomy: { merge: "auto" } });

await processJob(env, { type: "agent-regate-sweep", requestedBy: "schedule" });

expect(sent.map((m) => (m.type === "agent-regate-sweep" ? m.repoFullName : null))).toEqual(["owner/installed-repo"]);
});

it("agent re-gate sweep ALSO fans out to allowlisted repos regardless of autonomy mode (#sweep-all-modes)", async () => {
const sent: import("../../src/types").JobMessage[] = [];
const env = createTestEnv({ GITTENSORY_REVIEW_REPOS: "owner/advisory-repo", JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue });
Expand All @@ -931,8 +954,8 @@ describe("queue processors", () => {
GITTENSORY_REVIEW_REPOS: "",
JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue,
});
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } }, 9204);
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } }, 9205);
await upsertRepositorySettings(env, { repoFullName: "owner/agent-a", autonomy: { label: "auto" } });
await upsertRepositorySettings(env, { repoFullName: "owner/agent-b", autonomy: { label: "auto" } });
const realResolve = repositorySettingsModule.resolveRepositorySettings;
Expand Down Expand Up @@ -964,8 +987,8 @@ describe("queue processors", () => {
},
} as unknown as Queue,
});
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } });
await upsertRepositoryFromGitHub(env, { name: "agent-a", full_name: "owner/agent-a", private: false, owner: { login: "owner" } }, 9206);
await upsertRepositoryFromGitHub(env, { name: "agent-b", full_name: "owner/agent-b", private: false, owner: { login: "owner" } }, 9207);
await upsertRepositorySettings(env, { repoFullName: "owner/agent-a", autonomy: { label: "auto" } });
await upsertRepositorySettings(env, { repoFullName: "owner/agent-b", autonomy: { label: "auto" } });
const errors = vi.spyOn(console, "error").mockImplementation(() => undefined);
Expand All @@ -988,8 +1011,8 @@ describe("queue processors", () => {
JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue,
});
const repoNames = ["r1", "r2", "r3", "r4", "r5", "r6"];
for (const name of repoNames) {
await upsertRepositoryFromGitHub(env, { name, full_name: `owner/${name}`, private: false, owner: { login: "owner" } });
for (const [index, name] of repoNames.entries()) {
await upsertRepositoryFromGitHub(env, { name, full_name: `owner/${name}`, private: false, owner: { login: "owner" } }, 9300 + index);
await upsertRepositorySettings(env, { repoFullName: `owner/${name}`, autonomy: { label: "auto" } });
}
const { mapWithConcurrencyLimit: realMapWithConcurrencyLimit } =
Expand Down
24 changes: 22 additions & 2 deletions test/unit/selftune-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ function poisonDbPrepare(env: Env, pattern: RegExp): void {

async function seedRegisteredRepo(env: Env, fullName: string, autonomyJson: string): Promise<void> {
const [owner, name] = fullName.split("/");
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, ?, ?, 1, 1)")
.bind(fullName, owner, name)
// installation_id is required alongside is_installed=1 (#sweep-requires-installation): selfTuneRepos now
// requires a real installation before isAgentConfigured counts, matching the real upsertRepositoryFromGitHub
// invariant (isInstalled is only ever true alongside a real installationId).
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered, installation_id) VALUES (?, ?, ?, 1, 1, ?)")
.bind(fullName, owner, name, 9401)
.run();
// Opt the repo into the acting-autonomy surface so isAgentConfigured(settings.autonomy) is true (selfTuneRepos filter).
await env.DB.prepare("INSERT INTO repository_settings (repo_full_name, autonomy_json) VALUES (?, ?)")
Expand Down Expand Up @@ -273,6 +276,23 @@ describe("selfTuneRepos — per-repo review.selftune FORCE-OFF (#4104)", () => {
expect((await listOverrideAudit(env as never, "owner/opted-out")).length).toBe(0);
});

it("REGRESSION (#sweep-requires-installation): an acting-autonomy repo with NO real installation is excluded from the tuning pass, even though it resolves the operator's global-default autonomy", async () => {
const owner = "owner";
const name = "no-install";
const env = createTestEnv({ GITTENSORY_REVIEW_SELFTUNE: "true" });
await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, ?, ?, 0, 1)")
.bind(`${owner}/${name}`, owner, name)
.run();
await env.DB.prepare("INSERT INTO repository_settings (repo_full_name, autonomy_json) VALUES (?, ?)")
.bind(`${owner}/${name}`, ACTING_AUTONOMY)
.run();
await seedRecommendationOutcomes(env, `${owner}/${name}`, 5, 10); // would otherwise be a clear tightening signal

await runSelfTune(env);

expect(await loadShadowOverride(env as never, `${owner}/${name}`)).toBeNull();
});

it("unset review.selftune (the default) does not change today's behavior — an agent-configured repo still tunes normally", async () => {
const env = createTestEnv({ GITTENSORY_REVIEW_SELFTUNE: "true" });
await seedRegisteredRepo(env, "owner/repo", ACTING_AUTONOMY);
Expand Down