From a19e6e4f36fff16882fb761a39a25c58873df79e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:10:14 -0700 Subject: [PATCH] fix(config): route repository-settings reads through the resolver, not the raw DB accessor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backfillRegisteredRepositories, enqueueRepositoryOpenDataBackfill, buildInstallationRepairDiagnostics, refreshInstallationHealthRecords, and the MCP getAutomationState tool all read settings via the raw getRepositorySettings DB accessor instead of resolveRepositorySettings, so a .gittensory.yml override for backfillEnabled/checkRunMode/gateCheckMode/reviewCheckMode/autonomy/ agentPaused/agentDryRun was silently ignored at these call sites even though the real gate/action pipeline already honors it. loadPublicRepoBadge, buildRegistrationReadinessResponse, buildGittensorConfigRecommendationResponse, and loadContributorIssueDraftContext keep the raw DB read intentionally (a high-frequency public route, and advisory tools that need the raw settings/manifest layers unmerged for comparison) — each now has a comment explaining why. --- src/api/routes.ts | 13 +++++++ src/github/backfill.ts | 10 ++--- src/mcp/server.ts | 4 +- src/services/contributor-issue-draft.ts | 4 ++ test/unit/backfill.test.ts | 49 +++++++++++++++++++++++++ test/unit/mcp-automation-state.test.ts | 27 +++++++++++++- 6 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/api/routes.ts b/src/api/routes.ts index b9f454ef7a..c080b7220f 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -282,6 +282,10 @@ type AppContext = Context; async function loadPublicRepoBadge(env: Env, owner: string, repo: string): Promise { const repository = await getRepository(env, `${owner}/${repo}`); if (!repository || repository.isPrivate || !repository.isInstalled) return null; + // Intentionally the raw DB row, not resolveRepositorySettings: this is an unauthenticated, high-frequency + // public route (a README-embedded badge image), so it deliberately trades honoring a yml-only `badgeEnabled` + // override for avoiding a manifest-cache lookup (and a possible cold-cache GitHub fetch) on every image load. + // `badgeEnabled` is normally set via the dashboard/API, which persists straight to this same DB row (#2912). const settings = await getRepositorySettings(env, repository.fullName); if (!settings.badgeEnabled) return null; const pullRequests = await listPullRequests(env, repository.fullName); @@ -4332,6 +4336,11 @@ async function buildRepoOutcomePatternsResponse(env: Env, fullName: string) { async function buildRegistrationReadinessResponse(env: Env, fullName: string) { /* v8 ignore start -- Registration readiness route-level shaping over covered signal helpers. */ + // Intentionally the raw DB `settings` alongside the raw (cache-only, never live-fetched) `focusManifest`, + // not resolveRepositorySettings's merged view: this endpoint's whole purpose is to advise on the + // relationship between the two config layers (e.g. "your yml sets X but the currently active settings say + // Y"), which requires seeing them unmerged (#2912). See buildRegistrationReadiness's use of `focusManifest` + // for the yml-compiled policy section, separate from `settings` for the currently-active-behavior section. const [intelligence, settings, upstreamReports, focusManifest] = await Promise.all([ buildRepoIntelligenceResponse(env, fullName), getRepositorySettings(env, fullName), @@ -4385,6 +4394,10 @@ async function buildSelfDogfoodRegistrationPackResponse(env: Env) { async function buildGittensorConfigRecommendationResponse(env: Env, fullName: string) { /* v8 ignore start -- Config recommendation route-level shaping over covered signal helpers. */ + // Intentionally the raw DB settings, not resolveRepositorySettings's merged view: this tool recommends what + // to ADD to .gittensory.yml based on the repo's currently-active (dashboard/API-configured) behavior — using + // the yml-merged view here would be comparing the recommendation against itself once a yml override exists + // (#2912). const intelligence = await buildRepoIntelligenceResponse(env, fullName); const settings = await getRepositorySettings(env, fullName); const repo = intelligence.repo; diff --git a/src/github/backfill.ts b/src/github/backfill.ts index fa3190ca87..9cc0bbf1f7 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -1,5 +1,4 @@ import { - getRepositorySettings, getRepository, getPullRequest, countOpenIssues, @@ -43,6 +42,7 @@ import { extractLinkedIssueNumbers, } from "../db/repositories"; import { agentRequiresContentsWrite, agentRequiresPrWrite } from "../settings/agent-execution"; +import { resolveRepositorySettings } from "../settings/repository-settings"; import type { ContributorRepoStatRecord, GitHubRateLimitObservationRecord, @@ -373,7 +373,7 @@ export async function backfillRegisteredRepositories( const mode = options.mode ?? "light"; const limits = { ...DEFAULT_LIMITS, ...MODE_LIMITS[mode], ...(options.limits ?? {}) }; const repoResults = await mapWithConcurrency(repositories, limits.repoConcurrency, async (repo): Promise => { - const settings = await getRepositorySettings(env, repo.fullName); + const settings = await resolveRepositorySettings(env, repo.fullName); if (!settings.backfillEnabled) { const completedAt = nowIso(); await upsertSkippedSegments(env, repo, mode, completedAt, ["Backfill is disabled for this repository."]); @@ -447,7 +447,7 @@ export async function enqueueRepositoryOpenDataBackfill( const repo = await getRepository(env, options.repoFullName); if (!repo?.isRegistered) return { ok: true, repoFullName: options.repoFullName, status: "skipped", warnings: ["Repository is not registered for Gittensory backfill."] }; const mode = options.mode ?? "light"; - const settings = await getRepositorySettings(env, repo.fullName); + const settings = await resolveRepositorySettings(env, repo.fullName); if (!settings.backfillEnabled) return { ok: true, repoFullName: repo.fullName, status: "skipped", warnings: ["Backfill is disabled for this repository."] }; const token = await tokenForRepo(env, repo); const sourceKind: RepoSyncSegmentRecord["sourceKind"] = repo.installationId && token !== env.GITHUB_PUBLIC_TOKEN ? "installation" : "github"; @@ -950,7 +950,7 @@ export function enrichInstallationHealth(health: InstallationHealthRecord) { export async function buildInstallationRepairDiagnostics(env: Env, health: InstallationHealthRecord) { const installedRepos = (await listRepositories(env)).filter((repo) => repo.installationId === health.installationId && repo.isInstalled); - const installedSettings = await Promise.all(installedRepos.map((repo) => getRepositorySettings(env, repo.fullName))); + const installedSettings = await Promise.all(installedRepos.map((repo) => resolveRepositorySettings(env, repo.fullName))); const commentRepoCount = installedSettings.filter(usesCommentMode).length; const labelRepoCount = installedSettings.filter(usesLabelMode).length; const checkRunRepoCount = installedSettings.filter((settings) => settings.checkRunMode === "enabled").length; @@ -1149,7 +1149,7 @@ async function refreshInstallationHealthRecords(env: Env, installations: Install const { installation: currentInstallation, errorSummary, authMode } = await refreshStoredInstallation(env, installation); const installedRepos = repositories.filter((repo) => repo.installationId === currentInstallation.id && repo.isInstalled); const registeredInstalled = installedRepos.filter((repo) => repo.isRegistered); - const installedSettings = await Promise.all(installedRepos.map((repo) => getRepositorySettings(env, repo.fullName))); + const installedSettings = await Promise.all(installedRepos.map((repo) => resolveRepositorySettings(env, repo.fullName))); const requiresChecks = installedSettings.some((settings) => settings.checkRunMode === "enabled"); const requiresPrWrite = installedSettings.some((settings) => agentRequiresPrWrite(settings.autonomy)); const requiresContentsWrite = installedSettings.some((settings) => agentRequiresContentsWrite(settings.autonomy)); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index aedaa6cbf6..1da05b400d 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -20,7 +20,6 @@ import { getPendingAgentAction, getPullRequest, getRepository, - getRepositorySettings, isGlobalAgentFrozen, getRepoQueueTrendSnapshot, listAgentAuditEvents, @@ -117,6 +116,7 @@ import { import { applyStepResult, buildPlanDag, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "../services/plan-dag"; import { isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness } from "../settings/agent-execution"; import { AGENT_ACTION_CLASSES, isActingAutonomyLevel, resolveAutonomy } from "../settings/autonomy"; +import { resolveRepositorySettings } from "../settings/repository-settings"; import { MAX_FOCUS_MANIFEST_BYTES } from "../signals/focus-manifest"; import { loadPublicRepoFocusManifest, loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { buildPredictedGateVerdict } from "../rules/predicted-gate"; @@ -2450,7 +2450,7 @@ export class GittensoryMcp { await this.requireRepoAccess(fullName); const [repo, settings, pendingActionCount] = await Promise.all([ getRepository(this.env, fullName), - getRepositorySettings(this.env, fullName), + resolveRepositorySettings(this.env, fullName), countPendingAgentActions(this.env, { repoFullName: fullName, status: "pending" }), ]); const autonomy = settings.autonomy; diff --git a/src/services/contributor-issue-draft.ts b/src/services/contributor-issue-draft.ts index 425b79289c..8f9f687cfa 100644 --- a/src/services/contributor-issue-draft.ts +++ b/src/services/contributor-issue-draft.ts @@ -500,6 +500,10 @@ function pathSlug(path: string): string { } async function loadContributorIssueDraftContext(env: Env, repoFullName: string): Promise { + // Intentionally the raw DB `settings` alongside the raw (cache-only, never live-fetched) `focusManifest`, + // not resolveRepositorySettings's merged view: downstream consumers (e.g. buildContributorIssueDraftTestingRequirements) + // read `focusManifest` on its own for the yml-authored policy (wantedPaths/testExpectations/etc.), separate + // from `settings` for the currently-active dashboard/API behavior (#2912). const [repo, settings, openIssues, declinedIssues, focusManifest, upstreamReports, issues, pullRequests, recentMergedPullRequests, labels, queueCounts] = await Promise.all([ getRepository(env, repoFullName), getRepositorySettings(env, repoFullName), diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 73f57c0375..5cf3824405 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -939,6 +939,39 @@ describe("GitHub backfill", () => { ); }); + it("REGRESSION (#2912): repair diagnostics honor a .gittensory.yml-only checkRunMode: enabled override (DB row left at off)", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 123); + // DB row explicitly says checkRunMode: off; only the yml manifest turns it on, so this only passes if the + // resolver (not the raw DB accessor) is consulted for the installed-repo settings scan. + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", checkRunMode: "off" }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.endsWith("/.gittensory.yml")) return new Response("settings:\n checkRunMode: enabled\n", { status: 200 }); + return new Response("Not Found", { status: 404 }); + }); + + const repair = await buildInstallationRepairDiagnostics(env, { + installationId: 123, + accountLogin: "JSONbored", + repositorySelection: "selected", + installedReposCount: 1, + registeredInstalledCount: 0, + status: "healthy", + missingPermissions: [], + missingEvents: [], + permissions: { metadata: "read", pull_requests: "read", issues: "write" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + checkedAt: "2026-05-28T00:00:00.000Z", + authMode: "local", + }); + + expect(repair.requiredPermissions).toHaveProperty("checks"); + expect(repair.modeImpacts).toEqual( + expect.arrayContaining([expect.objectContaining({ mode: "check_run", enabled: true, affectedRepoCount: 1 })]), + ); + }); + it("repair diagnostics require contents:write for merge autonomy (#audit-install-health display)", async () => { const env = createTestEnv(); await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: true, owner: { login: "JSONbored" } }, 123); @@ -1122,6 +1155,22 @@ describe("GitHub backfill", () => { expect(result.repos[0]).toMatchObject({ status: "skipped", warnings: ["Backfill is disabled for this repository."] }); }); + it("REGRESSION (#2912): honors a .gittensory.yml-only backfillEnabled: false override (DB row left at its true default)", async () => { + const env = createTestEnv(); + await seedRegisteredRepo(env); + // No upsertRepositorySettings call: the DB row stays at its default (backfillEnabled: true). Only the + // yml manifest disables it, so this only passes if the resolver (not the raw DB accessor) is consulted. + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.endsWith("/.gittensory.yml")) return new Response("settings:\n backfillEnabled: false\n", { status: 200 }); + return new Response("Not Found", { status: 404 }); + }); + + const result = await backfillRegisteredRepositories(env); + + expect(result.repos[0]).toMatchObject({ status: "skipped", warnings: ["Backfill is disabled for this repository."] }); + }); + it("skips public repo backfill without a service token and backs off fresh sync states", async () => { const missingTokenEnv = createTestEnv(); await seedRegisteredRepo(missingTokenEnv); diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 0f216b463a..84d5a48f38 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -1,6 +1,6 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { GittensoryMcp } from "../../src/mcp/server"; import { getRepositoryCollaboratorPermission } from "../../src/github/app"; import { mergePullRequest } from "../../src/github/pr-actions"; @@ -52,6 +52,10 @@ beforeEach(() => { mockedPermission.mockResolvedValue("write"); }); +afterEach(() => { + vi.unstubAllGlobals(); +}); + async function connect(env: Env, identity?: AuthIdentity) { const server = (identity ? new GittensoryMcp(env, identity) : new GittensoryMcp(env)).createServer(); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); @@ -112,6 +116,27 @@ describe("MCP gittensory_get_automation_state (#784)", () => { expect(data.pendingActionCount).toBe(201); }); + it("REGRESSION (#2912): honors a .gittensory.yml-only agentPaused: true override (DB row left at its false default)", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 5); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto" } }); + // No agentPaused in the DB row above (stays at its false default): only the yml manifest pauses the repo, + // so this only passes if the resolver (not the raw DB accessor) is consulted. + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.endsWith("/.gittensory.yml")) return new Response("settings:\n agentPaused: true\n", { status: 200 }); + return new Response("Not Found", { status: 404 }); + }); + + const client = await connect(env); + const result = await client.callTool({ name: "gittensory_get_automation_state", arguments: { owner: "owner", repo: "repo" } }); + + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as State; + expect(data.agentPaused).toBe(true); + expect(data.mode).toBe("paused"); + }); + it("reports unconfigured + not_required readiness for an unknown / un-onboarded repo (no repo record)", async () => { const env = createTestEnv(); // no repo seeded → getRepository returns null (exercises the no-installation path) + default settings.