From 538f59a84ee1ea0ff1620f838ddec132e0670eba Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 13:24:02 -0700 Subject: [PATCH] feat!: remove the gateCheckMode field from RepositorySettings entirely (#5373) Final stage of the #5373 staged removal: gateCheckMode is gone from RepositorySettings (src/types.ts), its canonical derivation site (applyGateConfigOverrides in src/signals/focus-manifest.ts), the DB layer's default/read population (src/db/repositories.ts), and the last OpenAPI schema occurrence (RepositorySettingsSchema) -- the field no longer exists anywhere in the main app's type system. All prior stages (internal passthroughs, the DB column, registration- readiness/settings-preview responses, the maintainer-activation surface) already merged, so this lands the field's last remaining uses: it can now only ever exist as raw JSON request-body noise (silently ignored, same as any other unknown key) or as the gittensory-engine package's own separate .gittensory.yml back-compat parser (packages/gittensory-engine/**, untouched -- a distinct, published npm package with its own removal timeline). Updates every test that read/wrote the field directly: deletes the now-compile-time-impossible "gateCheckMode is a no-op write input" assertions (repository-settings-review-check-mode.test.ts, gate-check-policy.test.ts), replaces "unrelated settings preserved" checks with the real reviewCheckMode value (routes-ai-byok.test.ts, integration/api.test.ts), and updates stale comments referencing a field that no longer exists. This is an atomic, must-land-together change (removing the field breaks every remaining reference at once) -- cannot be split further without a temporary compatibility shim. --- apps/gittensory-ui/public/openapi.json | 7 ---- .../site/app-panels/maintainer-settings.tsx | 4 +- src/api/routes.ts | 5 ++- src/db/repositories.ts | 5 --- src/openapi/schemas.ts | 3 -- src/review/repo-profile.ts | 8 ++-- src/signals/focus-manifest.ts | 4 -- src/types.ts | 7 ---- test/integration/api.test.ts | 24 +++++------ test/unit/focus-manifest.test.ts | 32 +++++++-------- test/unit/gate-check-policy.test.ts | 12 +++--- test/unit/maintainer-activation.test.ts | 1 - test/unit/queue-4.test.ts | 2 +- ...ository-settings-review-check-mode.test.ts | 41 +++++-------------- test/unit/routes-ai-byok.test.ts | 6 +-- 15 files changed, 55 insertions(+), 106 deletions(-) diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index e6515b011d..9c82c108b0 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8731,13 +8731,6 @@ "standard" ] }, - "gateCheckMode": { - "type": "string", - "enum": [ - "off", - "enabled" - ] - }, "regateSweepOrderMode": { "type": "string", "enum": [ diff --git a/apps/gittensory-ui/src/components/site/app-panels/maintainer-settings.tsx b/apps/gittensory-ui/src/components/site/app-panels/maintainer-settings.tsx index f3e03663b4..9d712ee98c 100644 --- a/apps/gittensory-ui/src/components/site/app-panels/maintainer-settings.tsx +++ b/apps/gittensory-ui/src/components/site/app-panels/maintainer-settings.tsx @@ -21,8 +21,8 @@ type MaintainerSettings = { publicSurface: "off" | "comment_and_label" | "comment_only" | "label_only"; checkRunMode: "off" | "enabled"; checkRunDetailLevel: "minimal" | "standard"; - // #4618: gateCheckMode is deprecated (a computed read-back value only) -- reviewCheckMode is the real, - // writable authority for whether the review-agent check-run publishes. + // #4618/#5373: a prior gateCheckMode field was a deprecated computed read-back, since removed entirely -- + // reviewCheckMode is the real, writable authority for whether the review-agent check-run publishes. reviewCheckMode: "required" | "visible" | "disabled"; gatePack: "gittensor" | "oss-anti-slop"; linkedIssueGateMode: GateMode; diff --git a/src/api/routes.ts b/src/api/routes.ts index e06f88a040..ae4e673cf8 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -673,8 +673,9 @@ const repositorySettingsSchema = z.object({ // this full-replace route that omits this field must land on the same safe default as a never-configured row. checkRunDetailLevel: z.enum(["minimal", "standard"]).default("minimal"), regateSweepOrderMode: z.enum(["staleness", "oldest-first"]).default("staleness"), - // #4618: gateCheckMode dropped from this write schema -- it is a computed read-back value only (see its - // doc comment on RepositorySettings). Set reviewCheckMode directly. + // #4618/#5373: this write schema never accepted a gateCheckMode field -- it was a deprecated computed + // read-back of reviewCheckMode, removed from RepositorySettings entirely in #5373. Set reviewCheckMode + // directly. reviewCheckMode: z.enum(["required", "visible", "disabled"]).default("disabled"), gatePack: z.enum(["gittensor", "oss-anti-slop"]).default("gittensor"), linkedIssueGateMode: z.enum(["off", "advisory", "block"]).default("advisory"), diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 2b83d5daad..b70d8f474f 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -522,7 +522,6 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise publicSignalLevel: "standard", checkRunMode: "off", checkRunDetailLevel: "minimal", - gateCheckMode: "off", regateSweepOrderMode: "staleness", reviewCheckMode: "disabled", autoProjectMilestoneMatch: "off", @@ -604,10 +603,6 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise publicSignalLevel: row.publicSignalLevel === "minimal" ? "minimal" : "standard", checkRunMode: parseCheckRunMode(row.checkRunMode), checkRunDetailLevel: parseCheckRunDetailLevel(row.checkRunDetailLevel), - // #4618/#5373: gateCheckMode is a computed field, not its own stored source of truth -- always derive it - // from the real authority (reviewCheckMode) rather than a stored value. The gate_check_mode column itself - // was dropped (#5373, migrations/0146) since it never carried any information a fresh derivation didn't. - gateCheckMode: parseReviewCheckMode(row.reviewCheckMode) === "disabled" ? "off" : "enabled", regateSweepOrderMode: parseRegateSweepOrderMode(row.regateSweepOrderMode), reviewCheckMode: parseReviewCheckMode(row.reviewCheckMode), autoProjectMilestoneMatch: parseProjectMilestoneMatchMode(row.projectMilestoneMatchMode), diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index ea90a74d2f..8ac3f7cb7a 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -670,9 +670,6 @@ export const RepositorySettingsSchema = z publicSignalLevel: z.enum(["minimal", "standard"]), checkRunMode: z.enum(["off", "enabled"]), checkRunDetailLevel: z.enum(["minimal", "standard"]), - // @deprecated (#4618, tracked for removal in #5373): computed read-back of reviewCheckMode kept only - // for API/dashboard back-compat display -- read reviewCheckMode instead. - gateCheckMode: z.enum(["off", "enabled"]).optional(), regateSweepOrderMode: z.enum(["staleness", "oldest-first"]), reviewCheckMode: z.enum(["required", "visible", "disabled"]), autoProjectMilestoneMatch: z.enum(["off", "suggest", "auto"]).optional(), diff --git a/src/review/repo-profile.ts b/src/review/repo-profile.ts index f44273be13..7e798da098 100644 --- a/src/review/repo-profile.ts +++ b/src/review/repo-profile.ts @@ -58,10 +58,10 @@ export type RepoProfileCommands = { export type RepoProfileContributionWorkflow = { /** Whether the review gate (the "Gittensory Orb Review Agent" check) publishes a check at all, derived * from `settings.reviewCheckMode` -- the actual runtime authority for that check's publication (#2852). - * `gateCheckMode` is a genuinely legacy, deprecated read-back field (#4618) kept only for API/back-compat - * display and no longer drives anything. `checkRunMode` is NOT legacy -- it's a live, independent field - * that governs the SEPARATE "Gittensory Context" check, unrelated to this one. Reuses the EXISTING - * settings resolver rather than re-deriving gate presence from raw repo files. */ + * A prior `gateCheckMode` field was a deprecated read-back of this same value; it was removed entirely + * (#5373). `checkRunMode` is NOT related -- it's a live, independent field that governs the SEPARATE + * "Gittensory Context" check, unrelated to this one. Reuses the EXISTING settings resolver rather than + * re-deriving gate presence from raw repo files. */ gatePublishesCheck: boolean; linkedIssuePolicy: "required" | "preferred" | "optional"; requireLinkedIssue: boolean; diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 704acda01c..b270480486 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -464,10 +464,6 @@ function applyGateConfigOverrides(effective: RepositorySettings, gate: FocusMani // override, else the DB value) from the caller's spread. if (gate.checkMode !== null) effective.reviewCheckMode = gate.checkMode; else if (gate.enabled !== null) effective.reviewCheckMode = gate.enabled ? "required" : "disabled"; - // #4618: gateCheckMode is a computed read-back value only -- always re-derive it from the reviewCheckMode - // just resolved above (not from gate.enabled alone), so it stays correct even when only gate.checkMode was - // the field actually set in the manifest. - effective.gateCheckMode = effective.reviewCheckMode === "disabled" ? "off" : "enabled"; if (gate.pack !== null) effective.gatePack = gate.pack; if (gate.linkedIssue !== null) effective.linkedIssueGateMode = gate.linkedIssue; if (gate.duplicates !== null) effective.duplicatePrGateMode = gate.duplicates; diff --git a/src/types.ts b/src/types.ts index 8a807f350d..3160267582 100644 --- a/src/types.ts +++ b/src/types.ts @@ -715,13 +715,6 @@ export type RepositorySettings = { // #4620: "deep" removed -- it was never wired to any different behavior than "standard" (formatCheckRunOutput // and buildCheckRunAnnotations in rules/advisory.ts both branch only on `=== "minimal"` vs not). checkRunDetailLevel: "minimal" | "standard"; - /** @deprecated (#4618, being removed per #5373) Legacy shadow of {@link reviewCheckMode} (#2852): a - * computed read-back value only, for API/dashboard back-compat display. `"enabled"` when - * `reviewCheckMode !== "disabled"`, else `"off"` -- see getRepositorySettings/upsertRepositorySettings in - * db/repositories.ts. No write path accepts this field anymore; set {@link reviewCheckMode} directly - * instead. Optional (widened ahead of full removal) so callers building a partial RepositorySettings no - * longer need to supply it; production code (repositories.ts) still always populates it on every read. */ - gateCheckMode?: "off" | "enabled" | undefined; /** Scheduled re-gate sweep candidate ordering (#3815). `staleness` (default) picks whichever open PR the * sweep has gone longest WITHOUT re-gating (see selectRegateCandidates), which is what gives the sweep its * documented full-coverage-in-ceil(open/max)-ticks convergence guarantee even under dry-run/pause (when diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 4cdd344442..c13268cd5f 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -432,11 +432,10 @@ describe("api routes", () => { await expect(response.json()).resolves.toMatchObject({ repoFullName: "acme/readiness-block", qualityGateMode: "advisory" }); }); - it("ignores a gateCheckMode-only body through the internal settings write endpoint -- it is not a write field (#4618)", async () => { - // gateCheckMode is deprecated (#4618): a computed read-back value only, derived from reviewCheckMode. - // The internal full-replace route's schema no longer accepts it as input, so a caller sending ONLY - // gateCheckMode gets the schema's plain reviewCheckMode default ("disabled"), not the old legacy-write - // derivation. gateCheckMode in the response reflects that default, not the caller's (ignored) input. + it("ignores an unknown gateCheckMode key in the request body through the internal settings write endpoint (#4618/#5373)", async () => { + // gateCheckMode was removed entirely from RepositorySettings (#5373); the internal full-replace route's + // schema never accepted it as input even before removal (#4618). A caller sending it gets the schema's + // plain reviewCheckMode default ("disabled") -- the unknown key is silently ignored, not rejected. const app = createApp(); const env = createTestEnv(); const enabled = await app.request( @@ -445,7 +444,7 @@ describe("api routes", () => { env, ); expect(enabled.status).toBe(200); - await expect(enabled.json()).resolves.toMatchObject({ gateCheckMode: "off", reviewCheckMode: "disabled" }); + await expect(enabled.json()).resolves.toMatchObject({ reviewCheckMode: "disabled" }); // reviewCheckMode set directly is the real, honored write path. const explicit = await app.request( @@ -454,7 +453,7 @@ describe("api routes", () => { env, ); expect(explicit.status).toBe(200); - await expect(explicit.json()).resolves.toMatchObject({ gateCheckMode: "enabled", reviewCheckMode: "visible" }); + await expect(explicit.json()).resolves.toMatchObject({ reviewCheckMode: "visible" }); }); it("rejects invalid public GitHub repo stats paths before calling GitHub", async () => { @@ -2555,15 +2554,14 @@ describe("api routes", () => { // #2267: qualityGateMode: "block" is downgraded to "advisory" on write — readiness/quality can never // hard-block a PR, so the dashboard/API save path can't persist a value implying enforcement it doesn't // have. slopGateMode: "block" is a DIFFERENT, legitimately-blockable dimension and is left untouched. - // #4618: gateCheckMode is a legacy key with no effect here (dropped from the write schema) -- included - // to confirm it is silently ignored, not to drive reviewCheckMode. + // #4618/#5373: gateCheckMode is an unknown key with no effect here (removed from RepositorySettings + // entirely) -- included to confirm it is silently ignored, not to drive reviewCheckMode. body: JSON.stringify({ gateCheckMode: "enabled", reviewCheckMode: "required", slopGateMode: "block", slopGateMinScore: 55, qualityGateMode: "block", mergeTrainMode: "enforce", autonomy: { merge: "auto_with_approval", deploy: "auto" }, autoMaintain: { requireApprovals: 2, mergeMethod: "rebase" }, agentPaused: true, agentDryRun: true }), }, ownerEnv, ); expect(settingsUpdate.status).toBe(200); await expect(settingsUpdate.json()).resolves.toMatchObject({ - gateCheckMode: "enabled", // #4618: derived read-back from reviewCheckMode below, not the request's own gateCheckMode key reviewCheckMode: "required", slopGateMode: "block", slopGateMinScore: 55, @@ -2574,15 +2572,15 @@ describe("api routes", () => { agentPaused: true, // #776 kill-switch agentDryRun: true, }); - // #4618: gateCheckMode alone has NO effect -- it is dropped from the write schema, so reviewCheckMode - // stays whatever it already was (still "required" from the write immediately above), not derived "disabled". + // #4618/#5373: gateCheckMode alone has NO effect -- it is an unknown key, so reviewCheckMode stays + // whatever it already was (still "required" from the write immediately above), not derived "disabled". const settingsUpdateOff = await app.request( "/v1/repos/repo-owner/owned-repo/settings", { method: "PUT", headers: ownerHeaders, body: JSON.stringify({ gateCheckMode: "off" }) }, ownerEnv, ); expect(settingsUpdateOff.status).toBe(200); - await expect(settingsUpdateOff.json()).resolves.toMatchObject({ gateCheckMode: "enabled", reviewCheckMode: "required" }); + await expect(settingsUpdateOff.json()).resolves.toMatchObject({ reviewCheckMode: "required" }); // requireApprovals is bounded at the API boundary — an out-of-range value is rejected, not silently clamped. const settingsBadApprovals = await app.request( "/v1/repos/repo-owner/owned-repo/settings", diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 8278be810d..e3cbe4251c 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -2518,11 +2518,11 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(eff.reviewCheckMode).toBe("visible"); }); - // #4618: gateCheckMode is deprecated -- a computed read-back value only. It is no longer independently - // settable via any DB/API write path, but the yml settings.gateCheckMode key still parses (back-compat) - // and effective.gateCheckMode is always re-derived from the resolved reviewCheckMode, never trusted as - // its own source of truth. - describe("gateCheckMode deprecation (#4618)", () => { + // #4618/#5373: the RepositorySettings.gateCheckMode field (a computed read-back of reviewCheckMode) was + // removed entirely in #5373 -- resolveEffectiveSettings no longer derives or exposes it. The yml + // settings.gateCheckMode key still parses at the gittensory-engine layer (back-compat, tracked separately + // for removal), always resolving to reviewCheckMode rather than being trusted as its own source of truth. + describe("settings.gateCheckMode back-compat parsing (#4618)", () => { it("settings.gateCheckMode alone (no reviewCheckMode) derives reviewCheckMode, keeping its historical effect", () => { const enabled = parseFocusManifest({ settings: { gateCheckMode: "enabled" } }); expect(enabled.settings.reviewCheckMode).toBe("required"); @@ -2535,18 +2535,18 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(m.settings.reviewCheckMode).toBe("visible"); }); - it("resolveEffectiveSettings re-derives gateCheckMode from gate.checkMode alone, not just gate.enabled (regression)", () => { - // Before #4618, gateCheckMode was only mutated in the gate.enabled branch, so a manifest setting ONLY - // gate.checkMode left effective.gateCheckMode stale (still the DB's, potentially "off" while the check - // actually publishes) -- a latent lie in the back-compat display field. - const eff = resolveEffectiveSettings({ reviewCheckMode: "disabled", gateCheckMode: "off" } as unknown as RepositorySettings, parseFocusManifest({ gate: { checkMode: "visible" } })); + it("resolveEffectiveSettings resolves reviewCheckMode from gate.checkMode alone, not just gate.enabled (regression)", () => { + // Before #4618, the (since-removed) gateCheckMode display field was only mutated in the gate.enabled + // branch, so a manifest setting ONLY gate.checkMode left it stale. That field is gone now (#5373), but + // the underlying regression -- gate.checkMode alone must still resolve reviewCheckMode correctly, + // independent of whatever the DB settings held -- remains real and worth guarding. + const eff = resolveEffectiveSettings({ reviewCheckMode: "disabled" } as unknown as RepositorySettings, parseFocusManifest({ gate: { checkMode: "visible" } })); expect(eff.reviewCheckMode).toBe("visible"); - expect(eff.gateCheckMode).toBe("enabled"); }); - it("resolveEffectiveSettings re-derives gateCheckMode to off when reviewCheckMode resolves to disabled", () => { - const eff = resolveEffectiveSettings({ reviewCheckMode: "required", gateCheckMode: "enabled" } as unknown as RepositorySettings, parseFocusManifest({ gate: { checkMode: "disabled" } })); - expect(eff.gateCheckMode).toBe("off"); + it("resolveEffectiveSettings resolves reviewCheckMode to disabled via gate.checkMode, overriding an initially-required DB value", () => { + const eff = resolveEffectiveSettings({ reviewCheckMode: "required" } as unknown as RepositorySettings, parseFocusManifest({ gate: { checkMode: "disabled" } })); + expect(eff.reviewCheckMode).toBe("disabled"); }); }); }); @@ -2627,14 +2627,14 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = }); it("resolveEffectiveSettings overlays settings: over DB and lets gate: win for gate fields", () => { - const db = { commentMode: "off", gateCheckMode: "off", linkedIssueGateMode: "off", duplicatePrGateMode: "off", autoLabelEnabled: true } as unknown as RepositorySettings; + const db = { commentMode: "off", reviewCheckMode: "disabled", linkedIssueGateMode: "off", duplicatePrGateMode: "off", autoLabelEnabled: true } as unknown as RepositorySettings; const eff = resolveEffectiveSettings( db, parseFocusManifest({ settings: { commentMode: "all_prs", linkedIssueGateMode: "advisory", autoLabelEnabled: false }, gate: { enabled: true, linkedIssue: "block" } }), ); expect(eff.commentMode).toBe("all_prs"); // settings: override expect(eff.autoLabelEnabled).toBe(false); // settings: override (boolean) - expect(eff.gateCheckMode).toBe("enabled"); // gate.enabled + expect(eff.reviewCheckMode).toBe("required"); // gate.enabled: true expect(eff.linkedIssueGateMode).toBe("block"); // gate: wins over settings: }); diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 91ad56bf2f..893c30b82a 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -13,7 +13,6 @@ import type { Advisory, PullRequestRecord, RepositorySettings } from "../../src/ function settings(over: Partial = {}): RepositorySettings { return { commentMode: "detected_contributors_only", - gateCheckMode: "enabled", linkedIssueGateMode: "advisory", duplicatePrGateMode: "block", qualityGateMode: "advisory", @@ -44,17 +43,16 @@ describe(".gittensory.yml settings override (resolveEffectiveSettings)", () => { const eff = resolveEffectiveSettings(settings({ linkedIssueGateMode: "block" }), parseFocusManifest(null)); expect(eff.linkedIssueGateMode).toBe("block"); expect(eff.duplicatePrGateMode).toBe("block"); - expect(eff.gateCheckMode).toBe("enabled"); }); - it("overlays the friendly gate: alias over DB settings (incl. gate.enabled -> gateCheckMode)", () => { + it("overlays the friendly gate: alias over DB settings (incl. gate.enabled -> reviewCheckMode)", () => { const eff = resolveEffectiveSettings( - settings({ gateCheckMode: "enabled", linkedIssueGateMode: "advisory", duplicatePrGateMode: "block", qualityGateMode: "off", qualityGateMinScore: 10 }), + settings({ reviewCheckMode: "required", linkedIssueGateMode: "advisory", duplicatePrGateMode: "block", qualityGateMode: "off", qualityGateMinScore: 10 }), // readiness.mode: "block" is downgraded to "advisory" at parse time (#2267) — readiness/quality can // never hard-block, so this exercises the SAME downgrade flowing through resolveEffectiveSettings. parseFocusManifest({ gate: { enabled: false, linkedIssue: "block", duplicates: "off", readiness: { mode: "block", minScore: 70 } } }), ); - expect(eff.gateCheckMode).toBe("off"); // gate.enabled: false disables from config + expect(eff.reviewCheckMode).toBe("disabled"); // gate.enabled: false disables from config expect(eff.linkedIssueGateMode).toBe("block"); expect(eff.duplicatePrGateMode).toBe("off"); expect(eff.qualityGateMode).toBe("advisory"); @@ -63,12 +61,12 @@ describe(".gittensory.yml settings override (resolveEffectiveSettings)", () => { it("overlays the generic settings: block over DB, and gate: wins for gate fields", () => { const eff = resolveEffectiveSettings( - settings({ commentMode: "off", publicSurface: "off", gateCheckMode: "off", linkedIssueGateMode: "off" }), + settings({ commentMode: "off", publicSurface: "off", reviewCheckMode: "disabled", linkedIssueGateMode: "off" }), parseFocusManifest({ settings: { commentMode: "all_prs", publicSurface: "comment_only", gateCheckMode: "enabled", linkedIssueGateMode: "advisory" }, gate: { linkedIssue: "block" } }), ); expect(eff.commentMode).toBe("all_prs"); // settings: override expect(eff.publicSurface).toBe("comment_only"); // settings: override - expect(eff.gateCheckMode).toBe("enabled"); // settings: override (config enables the gate) + expect(eff.reviewCheckMode).toBe("required"); // settings.gateCheckMode: enabled -> reviewCheckMode: required (engine-level back-compat parse, #5373 stage 2.10) expect(eff.linkedIssueGateMode).toBe("block"); // gate: wins over settings: }); diff --git a/test/unit/maintainer-activation.test.ts b/test/unit/maintainer-activation.test.ts index b202bde85e..13d328b4e4 100644 --- a/test/unit/maintainer-activation.test.ts +++ b/test/unit/maintainer-activation.test.ts @@ -27,7 +27,6 @@ function settings(overrides: Partial = {}): RepositorySettin publicSignalLevel: "standard", checkRunMode: "off", checkRunDetailLevel: "standard", - gateCheckMode: "off", regateSweepOrderMode: "staleness", reviewCheckMode: "disabled", gatePack: "gittensor", diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index 2288d77a74..379f461de5 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -1527,7 +1527,7 @@ describe("queue processors", () => { linkedIssueGateMode: "block", requireLinkedIssue: true, }); - // Config turns the gate OFF even though repo settings have gateCheckMode: enabled. + // Config turns the gate OFF even though repo settings have reviewCheckMode: required (the gate check-run publishing). await upsertRepoFocusManifest(env, "JSONbored/gittensory", { gate: { enabled: false } }); const calls = { gateChecks: 0 }; vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { diff --git a/test/unit/repository-settings-review-check-mode.test.ts b/test/unit/repository-settings-review-check-mode.test.ts index 829736198f..ebc2e6ff01 100644 --- a/test/unit/repository-settings-review-check-mode.test.ts +++ b/test/unit/repository-settings-review-check-mode.test.ts @@ -2,49 +2,28 @@ import { describe, expect, it } from "vitest"; import { getRepositorySettings, upsertRepositorySettings } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; -// #2852/#4618: reviewCheckMode is the sole runtime authority for the "Gittensory Orb Review Agent" check-run -// publish decision (required/visible/disabled). gateCheckMode (off/enabled) is deprecated: a computed -// read-back value only, derived from reviewCheckMode on every read, and it has NO effect as a write input to -// upsertRepositorySettings -- the legacy dual-write sync now lives only at the yml settings.gateCheckMode -// parse step (packages/gittensory-engine/src/focus-manifest.ts), not in the DB/API layer. -describe("repository_settings: reviewCheckMode default + gateCheckMode read-only derivation (#2852, #4618)", () => { +// #2852/#5373: reviewCheckMode is the sole runtime authority for the "Gittensory Orb Review Agent" check-run +// publish decision (required/visible/disabled). A prior gateCheckMode (off/enabled) field was a deprecated +// computed read-back of reviewCheckMode with no effect as a write input; it has since been removed from +// RepositorySettings entirely (#5373) -- passing it to upsertRepositorySettings is now a compile-time error, +// not just a runtime no-op, so the tests that used to prove "gateCheckMode is ignored as a write input" no +// longer apply (the type system enforces it more strongly than a runtime assertion ever could). The legacy +// yml settings.gateCheckMode -> reviewCheckMode dual-write sync still exists one layer up, at +// packages/gittensory-engine/src/focus-manifest.ts's parse step (tracked separately for removal). +describe("repository_settings: reviewCheckMode default (#2852)", () => { it("getRepositorySettings returns disabled for a repo with no DB row at all (conservative, opt-in default)", async () => { const env = createTestEnv(); const settings = await getRepositorySettings(env, "acme/brand-new-repo"); expect(settings.reviewCheckMode).toBe("disabled"); - expect(settings.gateCheckMode).toBe("off"); }); - it("upsertRepositorySettings persists disabled when the caller omits reviewCheckMode AND gateCheckMode entirely", async () => { + it("upsertRepositorySettings persists disabled when the caller omits reviewCheckMode entirely", async () => { const env = createTestEnv(); await upsertRepositorySettings(env, { repoFullName: "acme/omits-both" }); const settings = await getRepositorySettings(env, "acme/omits-both"); expect(settings.reviewCheckMode).toBe("disabled"); }); - it("a caller that sets ONLY gateCheckMode: enabled (never touching reviewCheckMode) is ignored -- gateCheckMode is not a write input", async () => { - const env = createTestEnv(); - await upsertRepositorySettings(env, { repoFullName: "acme/legacy-enable", gateCheckMode: "enabled" }); - const settings = await getRepositorySettings(env, "acme/legacy-enable"); - expect(settings.reviewCheckMode).toBe("disabled"); - expect(settings.gateCheckMode).toBe("off"); // re-derived from reviewCheckMode, not the caller's stale input - }); - - it("a caller that sets ONLY gateCheckMode: off (never touching reviewCheckMode) stays disabled", async () => { - const env = createTestEnv(); - await upsertRepositorySettings(env, { repoFullName: "acme/legacy-disable", gateCheckMode: "off" }); - const settings = await getRepositorySettings(env, "acme/legacy-disable"); - expect(settings.reviewCheckMode).toBe("disabled"); - }); - - it("reviewCheckMode is honored regardless of a gateCheckMode also passed in the same call (gateCheckMode is a no-op input)", async () => { - const env = createTestEnv(); - await upsertRepositorySettings(env, { repoFullName: "acme/explicit-wins", gateCheckMode: "off", reviewCheckMode: "visible" }); - const settings = await getRepositorySettings(env, "acme/explicit-wins"); - expect(settings.reviewCheckMode).toBe("visible"); - expect(settings.gateCheckMode).toBe("enabled"); // derived from reviewCheckMode ("visible" !== "disabled"), not the "off" input - }); - it("an explicit required/visible/disabled opt-in round-trips through a re-upsert that carries it forward explicitly", async () => { const env = createTestEnv(); await upsertRepositorySettings(env, { repoFullName: "acme/round-trip", reviewCheckMode: "visible" }); diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index 6bb6f38e6e..9ee35f7bf8 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/test/unit/routes-ai-byok.test.ts @@ -44,7 +44,7 @@ describe("maintainer AI-review config route", () => { expect(settings.aiReviewMode).toBe("block"); expect(settings.aiReviewAllAuthors).toBe(true); // persisted + read back (DB column round-trip) expect(settings.closeOwnerAuthors).toBe(true); // persisted + read back (DB column round-trip) - expect(settings.gateCheckMode).toBe("enabled"); // preserved + expect(settings.reviewCheckMode).toBe("required"); // preserved expect(settings.gittensorLabel).toBe("custom-label"); // preserved expect(settings.blacklistLabel).toBe("abuse"); // #1425 round-trips through the DB }); @@ -106,7 +106,7 @@ describe("maintainer AI-review config route", () => { expect(await res.json()).toMatchObject({ aiReviewMode: "block", aiReviewLowConfidenceDisposition: "advisory_only" }); const settings = await getRepositorySettings(env, REPO); expect(settings.aiReviewLowConfidenceDisposition).toBe("advisory_only"); // persisted + read back (DB column round-trip) - expect(settings.gateCheckMode).toBe("enabled"); // preserved + expect(settings.reviewCheckMode).toBe("required"); // preserved expect(settings.gittensorLabel).toBe("custom-label"); // preserved }); @@ -155,7 +155,7 @@ describe("maintainer AI-review config route", () => { expect(await res.json()).toMatchObject({ closeOwnerAuthors: true, reviewCheckMode: "required", gittensorLabel: "custom-label" }); const settings = await getRepositorySettings(env, REPO); expect(settings.closeOwnerAuthors).toBe(true); - expect(settings.gateCheckMode).toBe("enabled"); + expect(settings.reviewCheckMode).toBe("required"); }); it("round-trips requireFreshRebaseWindowMinutes through the maintainer settings PUT route (#2552 gate finding)", async () => {