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
7 changes: 0 additions & 7 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -8731,13 +8731,6 @@
"standard"
]
},
"gateCheckMode": {
"type": "string",
"enum": [
"off",
"enabled"
]
},
"regateSweepOrderMode": {
"type": "string",
"enum": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
5 changes: 0 additions & 5 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 0 additions & 3 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
8 changes: 4 additions & 4 deletions src/review/repo-profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 11 additions & 13 deletions test/integration/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand Down
32 changes: 16 additions & 16 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
});
});
});
Expand Down Expand Up @@ -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:
});

Expand Down
Loading
Loading