Skip to content

Commit a6f40cb

Browse files
committed
fix(settings): preserve an explicit yml null clearing a contributor cap (#2467)
parseSettingsOverride collapsed an explicit `settings.contributorOpenPrCap: null` in .gittensory.yml to the same outcome as omitting the key, so a maintainer had no way to force a DB-configured cap back to disabled via config-as-code, contradicting the documented yml > DB > null precedence. Distinguish the three states (omitted / explicit null / invalid) before normalizing. Also corrects .gittensory.yml.example, which described these fields as already closing over-cap PRs; enforcement lands in a follow-up PR.
1 parent ad44ba3 commit a6f40cb

3 files changed

Lines changed: 36 additions & 9 deletions

File tree

.gittensory.yml.example

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,10 @@ settings:
263263
# gate-override: [maintainer, collaborator]
264264

265265
# Per-contributor open-PR/open-issue caps (#2270, anti-abuse): the max PRs/issues a single
266-
# non-owner/non-admin/non-bot contributor may have open on this repo at once. Uncomment and set a
267-
# number to opt in — a contributor's newest item(s) above the cap are closed with a clear reason;
268-
# their oldest items up to the cap stay open and reviewable. Positive whole number, or omit/null for
269-
# no cap. Default: null (disabled) for both.
266+
# non-owner/non-admin/non-bot contributor may have open on this repo at once. These fields are
267+
# currently INERT (stored and parsed, not yet enforced) — enforcement (closing the newest item(s)
268+
# above the cap with a clear reason) lands in a follow-up PR; see #2270 for status. Positive whole
269+
# number, or omit/null for no cap. Default: null (disabled) for both. Set explicitly to `null` (not
270+
# just omitted) to force-clear a cap that a dashboard/DB write previously set.
270271
# contributorOpenPrCap: 2
271272
# contributorOpenIssueCap: 5

src/signals/focus-manifest.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -796,11 +796,25 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[])
796796
}
797797
// Per-contributor open PR/issue caps (#2270): discrete counts, not scores — reuse the same positive-integer
798798
// normalizer as contentLane.maxAppendedEntries so a fractional/non-positive typo is dropped with a warning
799-
// instead of configuring a nonsensical cap.
800-
const contributorOpenPrCap = normalizeOptionalPositiveInteger(r.contributorOpenPrCap, "settings.contributorOpenPrCap", warnings);
801-
if (contributorOpenPrCap !== null) out.contributorOpenPrCap = contributorOpenPrCap;
802-
const contributorOpenIssueCap = normalizeOptionalPositiveInteger(r.contributorOpenIssueCap, "settings.contributorOpenIssueCap", warnings);
803-
if (contributorOpenIssueCap !== null) out.contributorOpenIssueCap = contributorOpenIssueCap;
799+
// instead of configuring a nonsensical cap. UNLIKE contributorBlacklist above, an explicit yml `null` here is
800+
// load-bearing (not the same as omitting the key): the documented `yml > DB > null` precedence means a
801+
// maintainer must be able to force a DB-configured cap back to "no cap" via `.gittensory.yml` without deleting
802+
// the DB row. `normalizeOptionalPositiveInteger` collapses "absent" and "null" to the same silent `null`
803+
// return, so that distinction has to be made HERE, before calling it: a literal `null` sets the key to `null`
804+
// (clears); omitted (`undefined`) leaves the key unset (preserves the DB value via the resolver's spread); an
805+
// invalid non-null value (fractional/non-positive/wrong type) warns and also leaves the key unset.
806+
if (r.contributorOpenPrCap === null) {
807+
out.contributorOpenPrCap = null;
808+
} else {
809+
const contributorOpenPrCap = normalizeOptionalPositiveInteger(r.contributorOpenPrCap, "settings.contributorOpenPrCap", warnings);
810+
if (contributorOpenPrCap !== null) out.contributorOpenPrCap = contributorOpenPrCap;
811+
}
812+
if (r.contributorOpenIssueCap === null) {
813+
out.contributorOpenIssueCap = null;
814+
} else {
815+
const contributorOpenIssueCap = normalizeOptionalPositiveInteger(r.contributorOpenIssueCap, "settings.contributorOpenIssueCap", warnings);
816+
if (contributorOpenIssueCap !== null) out.contributorOpenIssueCap = contributorOpenIssueCap;
817+
}
804818
return out;
805819
}
806820

test/unit/focus-manifest.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,6 +1344,18 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
13441344
expect(nonNumber.settings.contributorOpenPrCap).toBeUndefined();
13451345
});
13461346

1347+
it("an EXPLICIT yml null force-clears a DB-configured cap, distinct from an omitted key (regression, gate finding on #2467)", () => {
1348+
// Omitted key preserves the DB value (already covered above); an explicit `null` must ALSO be able to
1349+
// override a DB-configured cap back to "no cap" — the documented `yml > DB > null` precedence otherwise
1350+
// has no way to un-set a cap without a separate dashboard/DB write, which contradicts config-as-code.
1351+
const explicitNull = parseFocusManifest({ settings: { contributorOpenPrCap: null, contributorOpenIssueCap: null } });
1352+
expect(explicitNull.settings.contributorOpenPrCap).toBeNull();
1353+
expect(explicitNull.settings.contributorOpenIssueCap).toBeNull();
1354+
const eff = resolveEffectiveSettings({ contributorOpenPrCap: 4, contributorOpenIssueCap: 4 } as unknown as RepositorySettings, explicitNull);
1355+
expect(eff.contributorOpenPrCap).toBeNull();
1356+
expect(eff.contributorOpenIssueCap).toBeNull();
1357+
});
1358+
13471359
it("resolves contributor blacklist by unioning the shared/global list with effective per-repo settings", () => {
13481360
const manifest = parseFocusManifest({ settings: { contributorBlacklist: [{ login: "repo-only", reason: "manifest" }, { login: "Global-Repo", reason: "manifest-overrides-global" }] } });
13491361
const eff = resolveEffectiveSettings(

0 commit comments

Comments
 (0)