diff --git a/config/examples/README.md b/config/examples/README.md index be75528955..76ffeaaeab 100644 --- a/config/examples/README.md +++ b/config/examples/README.md @@ -23,6 +23,7 @@ ${GITTENSORY_REPO_CONFIG_DIR}/owner__repo/.gittensory.yml # 1. owner-qualified ${GITTENSORY_REPO_CONFIG_DIR}/repo/.gittensory.yml # 2. bare repo-name folder ${GITTENSORY_REPO_CONFIG_DIR}/owner__repo.yml # 3. flat file (back-compat) ${GITTENSORY_REPO_CONFIG_DIR}/.gittensory.yml # 4. global default, shared by every repo +${GITTENSORY_REPO_CONFIG_DIR}/_shared/.gittensory.yml # 5. shared base (#1959), lowest priority ``` `.yaml` and `.json` are accepted everywhere `.yml` is. Every one of these files uses the **exact @@ -36,19 +37,27 @@ or into your private mount and customize from there. From highest to lowest priority: -1. **Private per-repo file**, deep-merged over **2** when both exist (see below) — or used alone - when only a per-repo file exists. -2. **Private global default** (`${GITTENSORY_REPO_CONFIG_DIR}/.gittensory.yml`) — used alone when - a repo has no per-repo file of its own. -3. When **neither** a private per-repo nor a private global file exists, the loader falls back to - the **public repo `.gittensory.yml`** (or `.github/gittensory.yml`) fetched from GitHub. -4. **Dashboard/API-stored settings** for the repo. -5. **Built-in safe defaults.** - -Layers 1-2 are evaluated together as one private-config layer: if *either* a per-repo or a global -file exists privately, the public file in layer 3 is **never consulted** for that repo. This is -unchanged from the original private-config behavior (#1390) — only the interaction *between* the -private per-repo and private global layers is new. +1. **Private per-repo file**, deep-merged over **2** and **3** when more than one exists (see + below) — or used alone when it is the only private layer present. +2. **Private global default** (`${GITTENSORY_REPO_CONFIG_DIR}/.gittensory.yml`) — deep-merged + under **1** when both exist; used alone when a repo has no per-repo file of its own and no + shared base is mounted. +3. **Private shared base** (`${GITTENSORY_REPO_CONFIG_DIR}/_shared/.gittensory.yml`, #1959) — the + lowest-priority private layer, deep-merged under both **1** and **2**. An operator running many + repos writes a house review policy (e.g. a default `review.tone`, `path_filters`, or + `labeling_rules`) here **once** instead of copy-pasting it into every repo's per-repo file or + the global default. `.yaml`/`.json` are accepted, same as every other candidate. Absent (the + default, common case) ⇒ byte-identical behavior to the pre-#1959 2-layer chain. +4. When **none** of the three private layers above exists, the loader falls back to the **public + repo `.gittensory.yml`** (or `.github/gittensory.yml`) fetched from GitHub. +5. **Dashboard/API-stored settings** for the repo. +6. **Built-in safe defaults.** + +Layers 1-3 are evaluated together as one private-config layer: if *any* of a per-repo file, a +global default, or a shared base exists privately, the public file in layer 4 is **never +consulted** for that repo. This is unchanged from the original private-config behavior (#1390) — +only the interaction *among* the three private layers is new (the per-repo/global interaction +shipped first; the shared base is the newest, lowest layer, #1959). This chain governs *per-repo review policy* only. A separate, lower-level set of **deployment environment variables** (`GITTENSORY_REVIEW_*` flags, AI provider keys/models, self-host runtime @@ -60,24 +69,28 @@ narrows what's already permitted. See the generated, always-current ## Overlay (deep-merge) semantics -When **both** a per-repo file and a global default exist for a repo, they are merged — the -per-repo file overlaid onto the global default: +When **two or more** of {a per-repo file, a global default, a shared base} exist for a repo, they +are merged in ascending priority — shared base first, global default overlaid on top of that, then +the per-repo file overlaid on top of that (see [Shared base layer](#shared-base-layer-multi-repo-operators-1959) +below for the shared base specifically): - **Nested mappings** (`gate`, `settings`, `review`, `features`, `contentLane`, and their own - nested blocks like `gate.readiness` or `gate.aiReview`) merge **key by key**. A per-repo file - only needs to mention the keys it wants to change; everything else is inherited from global. + nested blocks like `gate.readiness` or `gate.aiReview`) merge **key by key**. A higher-priority + file only needs to mention the keys it wants to change; everything else is inherited from the + next layer down. - **Arrays** (`wantedPaths`, `preferredLabels`, `testExpectations`, `review.pathInstructions`, `review.excludePaths`, `contentLane.duplicateKeyFields`, etc.) - **replace wholesale** — a per-repo array is never concatenated with the global one. -- An **explicit `null`** at a key in the per-repo file always overrides the global value there. - This clears a setting wherever the manifest parser already treats an explicit `null` as + **replace wholesale** — a higher-priority array is never concatenated with a lower layer's. +- An **explicit `null`** at a key in a higher-priority file always overrides a lower layer's value + there. This clears a setting wherever the manifest parser already treats an explicit `null` as "off"/"clear" — e.g. `settings.contributorOpenPrCap`, `settings.contributorOpenIssueCap`, `settings.accountAgeThresholdDays`, and the enforcement label names (`settings.blacklistLabel`/`contributorCapLabel`/`reviewNagLabel`, see below) — and is a harmless no-op (equivalent to omitting the key) everywhere else. -- If either file fails to parse (or is malformed/oversized), the merge is skipped and the - still-valid file is used alone; a still-good sibling's policy is never silently discarded just - because the other file is broken. +- If any layer fails to parse (or is malformed/oversized), it is dropped from the merge and the + remaining, still-valid layers merge as if it were never mounted; a still-good layer's policy is + never silently discarded just because another layer is broken, and a broken layer never blocks a + review. ### Example 1 — global defaults + a per-repo override @@ -127,6 +140,62 @@ settings: - your-trusted-regular ``` +## Shared base layer (multi-repo operators, #1959) + +An operator running **many** repos through the same self-host instance can express one house +review policy — e.g. a default `review.tone`, a baseline `path_filters`/`wantedPaths` set, or +common `labeling_rules` — **once**, instead of copy-pasting it into every repo's per-repo file or +even the global default. That policy lives at: + +``` +${GITTENSORY_REPO_CONFIG_DIR}/_shared/.gittensory.yml +``` + +(`.yaml`/`.json` also accepted, same lookup order as every other candidate — see +[`shared.gittensory.yml`](./shared.gittensory.yml) for a starter). It sits at the **lowest** +priority of the three private layers: a per-repo file overlays a global default, which overlays +the shared base — the shared base fills in only the fields a higher layer is silent on. This is +the exact same deep-merge helper and array-replace/explicit-null-clear semantics described above, +folded across one more layer; it is not a new merge algorithm. + +**Absent shared base is the default, common case** — with no `_shared/.gittensory.yml` mounted, +behavior is byte-identical to the pre-#1959 2-layer chain. A malformed or unreadable shared file +fails safe exactly like a malformed per-repo or global file always has: it is dropped from the +merge and the remaining, still-valid layers combine as if it were never mounted — a broken shared +base never blocks a review. + +### Example 4 — shared base + global default + a per-repo override, all three present + +`_shared/.gittensory.yml` (shared base — one house policy for every repo on this instance): + +```yaml +review: + tone: friendly-terse +gate: + duplicates: block +``` + +`.gittensory.yml` (global default — this instance's own baseline, silent on `review.tone`): + +```yaml +gate: + enabled: true +``` + +`owner__repo/.gittensory.yml` (per-repo override — only touches what's different for this repo): + +```yaml +gate: + enabled: true + # duplicates is inherited from the shared base (still "block") — neither this file nor the + # global default repeats it. +``` + +The effective config for `owner/repo` has `review.tone: friendly-terse` (from the shared base; +neither global nor the per-repo file mentions it), `gate.duplicates: block` (from the shared base, +passed through untouched by global), and `gate.enabled: true` (set the same way by both global and +the per-repo file). + ## Label autonomy scoping for one-shot review mode Two `autonomy` classes govern every label the bot can apply, and they are **independent**: diff --git a/config/examples/shared.gittensory.yml b/config/examples/shared.gittensory.yml new file mode 100644 index 0000000000..53aba5c8c9 --- /dev/null +++ b/config/examples/shared.gittensory.yml @@ -0,0 +1,30 @@ +# ============================================================================ +# Self-host PRIVATE shared base — GENERIC EXAMPLE, safe to publish (#1959) +# ============================================================================ +# +# Copy this file to `${GITTENSORY_REPO_CONFIG_DIR}/_shared/.gittensory.yml` and edit your copy — +# never this one. This is the LOWEST-priority layer: a house review policy an operator running +# MANY repos writes ONCE here instead of copy-pasting it into every repo's private config. It is +# deep-merged UNDER both the global default (global.gittensory.yml) and any per-repo override +# (repo-override.gittensory.yml) — any key it doesn't mention is inherited from whichever of those +# two files is present; see ../README.md for the full precedence chain. +# +# Absent shared base (the default, common case) => byte-identical behavior to the pre-#1959 +# 2-layer chain — this file only matters once you actually mount one. +# ============================================================================ + +# House-wide review tone and defaults every repo starts from unless a higher layer overrides it. +review: + tone: friendly-terse + +# A default wanted-paths guidance for repos that don't set their own — a per-repo or global +# wantedPaths list REPLACES this wholesale (array-replace overlay semantics), it never merges with +# it. +wantedPaths: + - src/** + - test/** + +gate: + # House baseline: duplicates block by default fleet-wide. A repo needing a different policy sets + # its own `gate.duplicates` in its per-repo file (or the global default) to override just this key. + duplicates: block diff --git a/docker-compose.yml b/docker-compose.yml index abc21083b1..354aba49bf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -79,8 +79,9 @@ services: PORT: "8787" DATABASE_PATH: /data/gittensory.sqlite # Container-private per-repo config (#1390): point at a mounted dir holding {owner}__{repo}/.gittensory.yml - # files layered over an optional root .gittensory.yml global default (a per-repo file overrides only the - # keys it sets; anything it doesn't mention is inherited from the global default — see + # files layered over an optional root .gittensory.yml global default, itself layered over an optional + # _shared/.gittensory.yml cross-repo base for multi-repo operators (#1959) — a higher layer overrides only the + # keys it sets; anything it doesn't mention is inherited from the next layer down — see # config/examples/README.md). Keeps each repo's policy OUT of the public GitHub repo so contributors can't # game the rules. Empty/unmounted ⇒ repos use the public file / defaults. GITTENSORY_REPO_CONFIG_DIR: "${GITTENSORY_REPO_CONFIG_DIR:-/config}" diff --git a/src/selfhost/private-config.ts b/src/selfhost/private-config.ts index 75c9669e4e..93847af053 100644 --- a/src/selfhost/private-config.ts +++ b/src/selfhost/private-config.ts @@ -5,25 +5,35 @@ // into the Workers-safe loader via setLocalManifestReader at boot (server.ts), so this module's fs import never // reaches the Cloudflare bundle. // -// Layout (CodeRabbit-style: per-repo override, layered over a global default). For a repo `JSONbored/gittensory` -// the reader tries, in priority order: +// Layout (CodeRabbit-style: per-repo override, layered over a global default, layered over a cross-repo shared +// base — #1959). For a repo `JSONbored/gittensory` the reader tries, in priority order: // 1. `jsonbored__gittensory/.gittensory.yml` — owner-qualified folder (robust to repo-name collisions across owners) // 2. `gittensory/.gittensory.yml` — bare repo-name folder (the clean, human-readable layout) // 3. `jsonbored__gittensory.yml` — flat owner__repo file (the original #1390 layout; back-compat) // 4. `.gittensory.yml` — GLOBAL default at the dir root, shared by every repo. -// `.yaml` / `.json` are accepted everywhere `.yml` is. With only ONE of {a per-repo candidate, the global default} -// present, its raw text is returned unchanged — byte-identical to the original #1390 behavior. With BOTH present, -// they are DEEP-MERGED (the per-repo file overlaid onto the global default): nested mappings (`gate`, `settings`, -// `review`, `features`, `contentLane`, and their own nested blocks) merge key by key, arrays replace wholesale -// (never concatenated), and an explicit YAML/JSON `null` at a key always overrides the global value there — which -// clears a setting wherever the manifest parser already treats an explicit null as "off"/"clear" (e.g. -// `settings.contributorOpenPrCap`, `settings.accountAgeThresholdDays`), and is otherwise equivalent to omitting the -// key. A per-repo key that is simply absent leaves the corresponding global value untouched. If ONE file fails to -// parse as a YAML/JSON mapping (or is oversized), the merge is skipped and the OTHER file's raw text is used alone -// — a still-valid sibling's policy is never silently discarded just because its counterpart is broken. If BOTH -// fail, the per-repo file's raw text is returned (matching the original single-candidate priority), so a doubly -// broken pair degrades exactly like a single malformed manifest always has. The slug is lowercased (GitHub repo -// full-names are case-insensitive; #1390 already lowercased). +// 5. `_shared/.gittensory.yml` — SHARED BASE (#1959), the lowest-priority layer: one house policy +// an operator running many repos writes once instead of copy-pasting into every repo's private config. +// `.yaml` / `.json` are accepted everywhere `.yml` is. With only ONE of {a per-repo candidate, the global default, +// the shared base} present, its raw text is returned unchanged — byte-identical to the original #1390 behavior +// (and to the pre-#1959 2-layer behavior when no shared base is mounted, the common case). With more than one +// present, they are DEEP-MERGED in ascending priority (shared base → global default → per-repo file): nested +// mappings (`gate`, `settings`, `review`, `features`, `contentLane`, and their own nested blocks) merge key by key, +// arrays replace wholesale (never concatenated), and an explicit YAML/JSON `null` at a key always overrides a +// lower layer's value there — which clears a setting wherever the manifest parser already treats an explicit null +// as "off"/"clear" (e.g. `settings.contributorOpenPrCap`, `settings.accountAgeThresholdDays`), and is otherwise +// equivalent to omitting the key. A key that is simply absent from a higher layer leaves the lower layer's value at +// that key untouched. If a layer fails to parse as a YAML/JSON mapping (or is oversized), it is dropped from the +// fold and the remaining, still-valid layers merge as if it were never mounted — a broken layer never discards a +// still-good sibling's policy, and never blocks a review. If NONE of the present layers parse, the highest-priority +// present layer's raw text is returned (matching the original single-candidate priority), so a doubly/triply broken +// set degrades exactly like a single malformed manifest always has. The slug is lowercased (GitHub repo full-names +// are case-insensitive; #1390 already lowercased). +// +// Known limitation (#1959): the shared-base folder name `_shared` could collide with a bare repo-name folder +// (layer 2) for a real GitHub repo literally named `_shared` — `isSafeRepoSegment` permits `_` as an interior +// character, so that repo name is technically valid. This is a deliberately accepted edge case; an operator +// hosting a repo named exactly `_shared` should rename this folder via a private, documented convention rather +// than relying on any automatic disambiguation. import { readFile, readdir } from "node:fs/promises"; import { join, resolve } from "node:path"; import { parse as parseYaml } from "yaml"; @@ -50,6 +60,12 @@ function isSafeRepoSegment(segment: string): boolean { * deep-merged under any per-repo file (or applied alone, when a repo has no per-repo file of its own). */ export const GLOBAL_CONFIG_CANDIDATES: string[] = [...CONFIG_BASENAMES]; +/** Shared-base candidates (#1959, relative to GITTENSORY_REPO_CONFIG_DIR): `_shared/.gittensory.{yml,yaml,json}`, + * sibling to the per-repo folders inside the SAME container-private directory — no new env var. This is the + * lowest-priority layer: a cross-repo "house policy" an operator running many repos writes once, deep-merged + * UNDER both the global default and any per-repo file (or applied alone, when neither of those exists). */ +export const SHARED_BASE_CONFIG_CANDIDATES: string[] = CONFIG_BASENAMES.map((base) => join("_shared", base)); + /** Per-repo private-config candidate paths (relative to GITTENSORY_REPO_CONFIG_DIR), in priority order: * owner-qualified folder → bare repo-name folder → flat `owner__repo` file (the #1390 back-compat form). The slug * is the lowercased GitHub `owner__repo` (double underscore because `/` is not filename-safe); the bare folder is @@ -121,39 +137,56 @@ export function mergeConfigOverlay(base: unknown, override: unknown): unknown { return merged; } -/** Combine a global-default and a per-repo config's raw text, given that BOTH files exist. Both parse as mappings - * → deep-merged (the per-repo file overlaid onto the global default via {@link mergeConfigOverlay}) into one JSON - * document — a lossless round trip for the downstream `parseFocusManifestContent` (it starts with `{`, so that - * parser `JSON.parse`s it) since both inputs are already JSON-safe values. Only one parses → that file's raw text - * alone (best-effort: a broken sibling never discards a still-valid file's policy). Neither parses → the per-repo - * text (matching the original single-candidate-wins priority, so it flows downstream to the same "malformed, - * ignoring" warning a lone broken manifest always got). */ -function combineConfigText(globalText: string, repoText: string): string { - const globalRaw = parseConfigMapping(globalText); - const repoRaw = parseConfigMapping(repoText); - if (globalRaw && repoRaw) return JSON.stringify(mergeConfigOverlay(globalRaw, repoRaw)); - if (repoRaw) return repoText; - if (globalRaw) return globalText; - return repoText; +/** Combine any number of config-text layers, given in ASCENDING priority order (lowest first, e.g. + * `[sharedText, globalText, repoText]` — #1959), into the raw text `parseFocusManifestContent` should parse. + * `null` entries (a layer whose file simply doesn't exist) are ignored outright. Every remaining, present layer is + * tolerantly parsed via {@link parseConfigMapping}; layers that parse as a mapping are folded left-to-right with + * {@link mergeConfigOverlay} (each later — higher-priority — layer overlays every earlier one), so a 3-way fold + * reduces to exactly the existing 2-way "per-repo overlays global" semantics when only 2 layers are present, and to + * a single file's raw text when only 1 is. A present-but-unparseable layer (malformed, oversized, or a parsed + * non-mapping value) is DROPPED from the fold — never blocks, never discards a still-valid sibling's policy — + * which is why fewer than 2 layers may end up parsed even when more than 2 are present on disk. With 0 parsed + * layers, the highest-priority PRESENT layer's raw text is returned unchanged (matching the original + * single-candidate priority, so a fully-broken set degrades exactly like a single malformed manifest always has). + * With exactly 1 parsed layer, that layer's own raw text is returned unchanged — never re-serialized — so a lone + * valid file's formatting/comments survive untouched, identical to the pre-#1959 "only one side present" case. + * Only 2+ successfully parsed layers are actually re-serialized as merged JSON. */ +function combineConfigLayers(layersAscendingPriority: (string | null)[]): string | null { + const present = layersAscendingPriority.filter((text): text is string => text !== null); + if (present.length === 0) return null; + const parsedLayers: { text: string; mapping: Record }[] = []; + for (const text of present) { + const mapping = parseConfigMapping(text); + if (mapping) parsedLayers.push({ text, mapping }); + } + if (parsedLayers.length === 0) return present[present.length - 1]!; // none parsed → highest-priority present layer, raw + if (parsedLayers.length === 1) return parsedLayers[0]!.text; // exactly one parsed → its raw text, unchanged + let merged: unknown = parsedLayers[0]!.mapping; + for (const layer of parsedLayers.slice(1)) merged = mergeConfigOverlay(merged, layer.mapping); + return JSON.stringify(merged); } /** Build the container-local manifest reader over GITTENSORY_REPO_CONFIG_DIR, or null when the dir is unset/blank - * (⇒ the loader keeps fetching the public `.gittensory.yml`). Looks up the first existing per-repo candidate and - * the global-default candidate independently: with only one present, its raw text is returned unchanged; with - * both present, they are deep-merged (see the module header) and returned as one JSON document; with neither - * present, null (⇒ the loader falls through to the public file). An invalid repo full name yields no per-repo - * candidates and is NOT served the global default (it is never a real webhook repo). */ + * (⇒ the loader keeps fetching the public `.gittensory.yml`). Looks up the first existing per-repo candidate, the + * global-default candidate, and the shared-base candidate (#1959) independently and folds whichever are present + * in ascending priority (shared → global → per-repo) via {@link combineConfigLayers}: with only one present, its + * raw text is returned unchanged; with two or more, they are deep-merged (see the module header) and returned as + * one JSON document; with none present, null (⇒ the loader falls through to the public file). An invalid repo + * full name yields no per-repo candidates and is NOT served the global default or the shared base either (it is + * never a real webhook repo). */ export function makeLocalManifestReader(dir: string | undefined): RepoFocusManifestFetcher | null { const trimmed = (dir ?? "").trim(); if (!trimmed) return null; const base = resolve(trimmed); return async (repoFullName: string): Promise => { const perRepo = localConfigCandidates(repoFullName); - if (perRepo.length === 0) return null; // invalid repo name → no per-repo file AND no global default - const [repoText, globalText] = await Promise.all([readFirstExisting(base, perRepo), readFirstExisting(base, GLOBAL_CONFIG_CANDIDATES)]); - if (repoText === null) return globalText; - if (globalText === null) return repoText; - return combineConfigText(globalText, repoText); + if (perRepo.length === 0) return null; // invalid repo name → no per-repo file, global default, or shared base + const [sharedText, globalText, repoText] = await Promise.all([ + readFirstExisting(base, SHARED_BASE_CONFIG_CANDIDATES), + readFirstExisting(base, GLOBAL_CONFIG_CANDIDATES), + readFirstExisting(base, perRepo), + ]); + return combineConfigLayers([sharedText, globalText, repoText]); }; } diff --git a/test/unit/private-config.test.ts b/test/unit/private-config.test.ts index 92f14eb082..d5640f9f37 100644 --- a/test/unit/private-config.test.ts +++ b/test/unit/private-config.test.ts @@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { describe, expect, it } from "vitest"; -import { GLOBAL_CONFIG_CANDIDATES, isReviewSkillEnabled, localConfigCandidates, makeLocalManifestReader, makeLocalReviewContextReader, mergeConfigOverlay, parseReviewSkill } from "../../src/selfhost/private-config"; +import { GLOBAL_CONFIG_CANDIDATES, isReviewSkillEnabled, localConfigCandidates, makeLocalManifestReader, makeLocalReviewContextReader, mergeConfigOverlay, parseReviewSkill, SHARED_BASE_CONFIG_CANDIDATES } from "../../src/selfhost/private-config"; import { loadRepoReviewContext, setLocalReviewContextReader } from "../../src/signals/focus-manifest-loader"; import { MAX_FOCUS_MANIFEST_BYTES, parseFocusManifestContent } from "../../src/signals/focus-manifest"; @@ -37,6 +37,13 @@ describe("localConfigCandidates (container-private config paths)", () => { it("exposes the dir-root global-fallback candidates", () => { expect(GLOBAL_CONFIG_CANDIDATES).toEqual([".gittensory.yml", ".gittensory.yaml", ".gittensory.json"]); }); + it("exposes the shared-base candidates, sibling to the per-repo folders (#1959)", () => { + expect(SHARED_BASE_CONFIG_CANDIDATES).toEqual([ + join("_shared", ".gittensory.yml"), + join("_shared", ".gittensory.yaml"), + join("_shared", ".gittensory.json"), + ]); + }); }); describe("mergeConfigOverlay (generic recursive deep-merge, no manifest-field-specific code)", () => { @@ -209,6 +216,126 @@ describe("makeLocalManifestReader (GITTENSORY_REPO_CONFIG_DIR)", () => { }); }); +describe("makeLocalManifestReader — shared base layer (#1959)", () => { + it("falls back to the shared base alone when neither a per-repo file nor a global default exists", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "gate:\n enabled: false\n"); + const reader = makeLocalManifestReader(dir); + expect(await reader!("owner/repo")).toBe("gate:\n enabled: false\n"); // byte-identical raw text, no merge attempted + }); + + it("byte-identical to pre-#1959 behavior when no shared base file is mounted at all (repo-only)", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "gate:\n enabled: true\n"); + const reader = makeLocalManifestReader(dir); + expect(await reader!("owner/repo")).toBe("gate:\n enabled: true\n"); // no _shared/ present → same as the existing repo-only test + }); + + it("deep-merges a per-repo file over a shared base with no global default present: per-repo wins, shared fills the rest", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "gate:\n enabled: false\n duplicates: block\n"); // shared house policy + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "gate:\n enabled: true\n"); // repo overrides only `enabled` + const reader = makeLocalManifestReader(dir); + const manifest = parseFocusManifestContent(await reader!("owner/repo")); + expect(manifest.gate.enabled).toBe(true); // per-repo wins on the shared key + expect(manifest.gate.duplicates).toBe("block"); // inherited from the shared base, untouched + }); + + it("folds all three layers: per-repo wins over global, which wins over the shared base, per-field", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "gate:\n enabled: false\n duplicates: block\n linkedIssue: advisory\n"); // shared house policy + writeFileSync(join(dir, ".gittensory.yml"), "gate:\n duplicates: off\n"); // global overrides duplicates only + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "gate:\n enabled: true\n"); // per-repo overrides enabled only + const reader = makeLocalManifestReader(dir); + const manifest = parseFocusManifestContent(await reader!("owner/repo")); + expect(manifest.gate.enabled).toBe(true); // from per-repo (highest priority) + expect(manifest.gate.duplicates).toBe("off"); // from global, overlaying the shared base's "block" + expect(manifest.gate.linkedIssue).toBe("advisory"); // inherited from the shared base, untouched by either override + }); + + it("replaces an array wholesale across the 3-layer fold instead of concatenating", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "wantedPaths:\n - shared/**\n"); + writeFileSync(join(dir, ".gittensory.yml"), "wantedPaths:\n - src/**\n - test/**\n"); + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "wantedPaths:\n - docs/**\n"); + const reader = makeLocalManifestReader(dir); + const manifest = parseFocusManifestContent(await reader!("owner/repo")); + expect(manifest.wantedPaths).toEqual(["docs/**"]); // per-repo array wins wholesale, shared/global arrays discarded + }); + + it("lets an explicit per-repo null clear a shared-base-configured value even when global doesn't mention the key", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "settings:\n contributorOpenPrCap: 5\n"); + writeFileSync(join(dir, ".gittensory.yml"), "gate:\n enabled: true\n"); // global present, but silent on contributorOpenPrCap + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "settings:\n contributorOpenPrCap: null\n"); + const reader = makeLocalManifestReader(dir); + const manifest = parseFocusManifestContent(await reader!("owner/repo")); + expect(manifest.settings.contributorOpenPrCap).toBeNull(); // explicit null clears the shared 5, not "unset" + }); + + it("fails safe to repo+global (as if unmounted) when the shared base is malformed, and never blocks a review", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "{ not valid json"); // starts with `{` → JSON.parse throws + writeFileSync(join(dir, ".gittensory.yml"), "gate:\n duplicates: block\n"); + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "gate:\n enabled: true\n"); + const reader = makeLocalManifestReader(dir); + const manifest = parseFocusManifestContent(await reader!("owner/repo")); + expect(manifest.gate.enabled).toBe(true); // still merged from the two still-valid layers + expect(manifest.gate.duplicates).toBe("block"); // global's value survives; broken shared base dropped, not blocking + }); + + it("fails safe to repo-only (shared base oversized) when it is the ONLY other layer present", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + const oversized = `# ${"x".repeat(MAX_FOCUS_MANIFEST_BYTES + 10)}\ngate:\n enabled: false\n`; + writeFileSync(join(dir, "_shared", ".gittensory.yml"), oversized); // too large to attempt a merge against + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "gate:\n enabled: true\n"); + const reader = makeLocalManifestReader(dir); + expect(await reader!("owner/repo")).toBe("gate:\n enabled: true\n"); // oversized shared base dropped; per-repo raw text unchanged + }); + + it("falls back to the highest-priority present layer's raw text when ALL THREE fail to parse as mappings", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "{ shared also broken"); + writeFileSync(join(dir, ".gittensory.yml"), "{ global also broken"); + mkdirSync(join(dir, "repo")); + writeFileSync(join(dir, "repo", ".gittensory.yml"), "{ broken json"); + const reader = makeLocalManifestReader(dir); + expect(await reader!("owner/repo")).toBe("{ broken json"); // per-repo (highest priority) raw text, same downstream "malformed" handling + }); + + it("tries _shared/.gittensory.yml before .yaml before .json", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yaml"), "gate:\n enabled: true\n"); + writeFileSync(join(dir, "_shared", ".gittensory.json"), '{"gate":{"enabled":false}}'); + const reader = makeLocalManifestReader(dir); + expect(await reader!("owner/repo")).toBe("gate:\n enabled: true\n"); // .yaml found before .json is tried + }); + + it("does NOT serve the shared base to an invalid repo full name (no per-repo candidates)", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-repo-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), "gate:\n enabled: false\n"); + const reader = makeLocalManifestReader(dir); + expect(await reader!("no-slash")).toBeNull(); // perRepo.length === 0 early return, before the shared base is even read + }); +}); + describe("parseReviewSkill (#review-skills)", () => { it("parses frontmatter name + when (quotes stripped); body is the remainder", () => { expect(parseReviewSkill("sql.md", '---\nname: sql-rubric\nwhen: "**/*.sql"\n---\nCheck the index.\n')).toEqual({ name: "sql-rubric", when: "**/*.sql", body: "Check the index." }); diff --git a/test/unit/selfhost-config-examples.test.ts b/test/unit/selfhost-config-examples.test.ts index 4ddd4a1d30..cbf0368600 100644 --- a/test/unit/selfhost-config-examples.test.ts +++ b/test/unit/selfhost-config-examples.test.ts @@ -34,6 +34,17 @@ describe("config/examples/global.gittensory.yml", () => { }); }); +describe("config/examples/shared.gittensory.yml (#1959)", () => { + it("parses cleanly with no warnings and sets the documented fields", () => { + const manifest = parseFocusManifestContent(readExample("shared.gittensory.yml")); + expect(manifest.warnings).toEqual([]); + expect(manifest.present).toBe(true); + expect(manifest.review.tone).toBe("friendly-terse"); + expect(manifest.wantedPaths).toEqual(["src/**", "test/**"]); + expect(manifest.gate.duplicates).toBe("block"); + }); +}); + describe("config/examples/repo-override.gittensory.yml", () => { it("parses cleanly with no warnings and sets the documented fields", () => { const manifest = parseFocusManifestContent(readExample("repo-override.gittensory.yml")); @@ -64,3 +75,22 @@ describe("the two examples together demonstrate the documented overlay behavior" expect(manifest.settings.autonomy).toEqual({ close: "auto" }); // inherited from global untouched (repo-override never mentions it) }); }); + +describe("all three examples together demonstrate the documented shared-base overlay (#1959)", () => { + it("merges shared → global → per-repo exactly as config/examples/README.md describes", async () => { + const dir = mkdtempSync(join(tmpdir(), "gt-example-config-")); + mkdirSync(join(dir, "_shared")); + writeFileSync(join(dir, "_shared", ".gittensory.yml"), readExample("shared.gittensory.yml")); + writeFileSync(join(dir, ".gittensory.yml"), readExample("global.gittensory.yml")); + mkdirSync(join(dir, "owner__repo")); + writeFileSync(join(dir, "owner__repo", ".gittensory.yml"), readExample("repo-override.gittensory.yml")); + const reader = makeLocalManifestReader(dir)!; + const manifest = parseFocusManifestContent(await reader("owner/repo")); + + expect(manifest.review.tone).toBe("friendly-terse"); // inherited from the shared base; neither global nor repo-override mentions it + expect(manifest.gate.duplicates).toBe("block"); // shared base and global agree; still inherited, not overridden + expect(manifest.gate.enabled).toBe(true); // set by both global and repo-override, shared base is silent on it + expect(manifest.wantedPaths).toEqual(["src/**"]); // repo-override's array wins wholesale over shared's AND global's + expect(manifest.settings.contributorOpenPrCap).toBeNull(); // repo-override's explicit null still clears global's 3 + }); +});