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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,8 @@ jobs:
# rebrand -- see the script's own header comment (#6786 is the concrete incident this was written
# for). Scoped broadly (src/** plus every workspace package's bin/lib/src/scripts dirs), so it's
# gated broadly to match; same local-only-until-now gap as the drift checks above. Also gated on
# `discoveryIndex`: BRANDING_DRIFT_PATHSPECS includes "packages/*/src/**/*.ts", which matches
# packages/discovery-index/src/**.
# `discoveryIndex`: BRANDING_DRIFT_PATHSPECS includes "packages/*/src/*.ts", which matches
# packages/discovery-index/src/*.ts (git pathspec `*` matches `/`, so this covers every depth).
- name: Branding drift check
if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.discoveryIndex == 'true' }}
run: npm run branding-drift:check
Expand Down
3 changes: 3 additions & 0 deletions scripts/branding-drift-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
"apps/loopover-ui/src/routes/app.index.tsx": 1,
"apps/loopover-ui/src/routes/app.runs.tsx": 1,
"apps/loopover-ui/src/routes/app.workbench.tsx": 1,
"packages/loopover-contract/src/cli-config.ts": 2,
"packages/loopover-engine/src/signals/engine.ts": 2,
"packages/loopover-mcp/bin/loopover-mcp.ts": 2,
"src/api/routes.ts": 2,
"src/db/repositories.ts": 1,
"src/env.d.ts": 3,
"src/github/app.ts": 11,
"src/github/backfill.ts": 7,
"src/github/commands.ts": 2,
Expand All @@ -30,6 +32,7 @@
"src/selfhost/health.ts": 3,
"src/selfhost/monitored-work.ts": 1,
"src/selfhost/orb-collector.ts": 1,
"src/server.ts": 3,
"src/services/ai-review.ts": 4,
"src/services/ai-slop.ts": 1,
"src/services/ai-summaries.ts": 1,
Expand Down
20 changes: 10 additions & 10 deletions scripts/check-branding-drift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ export const BASELINE_RELATIVE_PATH = "scripts/branding-drift-baseline.json";
// top-level src/** scope; docs/README/CHANGELOG/schema/terraform/css and every test dir are deliberately
// excluded (see header comment).
export const BRANDING_DRIFT_PATHSPECS = [
"src/**/*.ts",
"src/**/*.tsx",
"src/*.ts",
"src/*.tsx",
"packages/*/bin/**",
"packages/*/lib/**/*.js",
"packages/*/lib/**/*.ts",
"packages/*/src/**/*.ts",
"packages/*/src/**/*.tsx",
"packages/*/scripts/**/*.mjs",
"apps/*/src/**/*.ts",
"apps/*/src/**/*.tsx",
"apps/*/scripts/**/*.mjs",
"packages/*/lib/*.js",
"packages/*/lib/*.ts",
"packages/*/src/*.ts",
"packages/*/src/*.tsx",
"packages/*/scripts/*.mjs",
"apps/*/src/*.ts",
"apps/*/src/*.tsx",
"apps/*/scripts/*.mjs",
":(exclude)**/*.test.ts",
":(exclude)**/*.test.tsx",
":(exclude)packages/*/test/**",
Expand Down
119 changes: 112 additions & 7 deletions test/unit/check-branding-drift-script.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
import { execFileSync } from "node:child_process";
import { describe, expect, it } from "vitest";
import { diffBrandingBaseline, scanBrandingHits } from "../../scripts/check-branding-drift";
import { BRANDING_DRIFT_PATHSPECS, diffBrandingBaseline, scanBrandingHits } from "../../scripts/check-branding-drift";

// Mirrors git's own pathspec matching for patterns with no `:(glob)` magic: fnmatch(3) with FNM_PATHNAME
// OFF, so `*` (and therefore `**`, which collapses to the same thing under plain fnmatch) matches `/` too.
// This is deliberately NOT a shortcut like `path.startsWith(root)` -- it has to reproduce the exact
// depth-blind-or-not behavior `git grep` applies, so this test fails against the pre-fix `**/`-segmented list.
function globToRegExp(pattern: string): RegExp {
const body = pattern.replace(/[.+^${}()|[\]\\]/g, "\\$&").replace(/\*+/g, ".*").replace(/\?/g, ".");
return new RegExp(`^${body}$`);
}

function matchesPathspecs(pathspecs: readonly string[], file: string): boolean {
const includes = pathspecs.filter((spec) => !spec.startsWith(":(exclude)"));
const excludes = pathspecs.filter((spec) => spec.startsWith(":(exclude)")).map((spec) => spec.slice(":(exclude)".length));
const included = includes.some((spec) => globToRegExp(spec).test(file));
const excluded = excludes.some((spec) => globToRegExp(spec).test(file));
return included && !excluded;
}

describe("scanBrandingHits", () => {
it("parses git grep -c output into a { file: count } map", () => {
Expand Down Expand Up @@ -33,7 +50,7 @@ describe("scanBrandingHits", () => {
scanBrandingHits({ root: "/fake", exec });

expect(capturedArgs[0]).toBe("grep");
expect(capturedArgs).toContain("src/**/*.ts");
expect(capturedArgs).toContain("src/*.ts");
expect(capturedArgs).toContain(":(exclude)**/*.test.ts");
});

Expand All @@ -45,20 +62,20 @@ describe("scanBrandingHits", () => {
};
scanBrandingHits({ root: "/fake", exec });

expect(capturedArgs).toContain("apps/*/src/**/*.ts");
expect(capturedArgs).toContain("apps/*/src/**/*.tsx");
expect(capturedArgs).toContain("apps/*/scripts/**/*.mjs");
expect(capturedArgs).toContain("apps/*/src/*.ts");
expect(capturedArgs).toContain("apps/*/src/*.tsx");
expect(capturedArgs).toContain("apps/*/scripts/*.mjs");
});

it("scans packages/*/src/**/*.tsx, so ui-kit design-system components are covered like apps/* .tsx are", () => {
it("scans packages/*/src/*.tsx, so ui-kit design-system components are covered like apps/* .tsx are", () => {
let capturedArgs: string[] = [];
const exec = (_root: string, args: string[]) => {
capturedArgs = args;
return "";
};
scanBrandingHits({ root: "/fake", exec });

expect(capturedArgs).toContain("packages/*/src/**/*.tsx");
expect(capturedArgs).toContain("packages/*/src/*.tsx");
});

it("includes a packages/*/src/*.tsx hit in the scanned set (a ui-kit component now in scope)", () => {
Expand Down Expand Up @@ -135,6 +152,94 @@ describe("diffBrandingBaseline", () => {
});
});

describe("BRANDING_DRIFT_PATHSPECS depth coverage (regression for #10045)", () => {
// Each row is a root that used to be written `<root>/**/*.<ext>`. A depth-0 file (directly inside the
// root) and a nested file must both match; a file outside the root must not. Against the pre-fix `**/`
// form, `depthZero` fails to match (the exact bug this issue is about) while `nested` and `outside` still
// pass -- so this table only turns fully green once every affected pathspec drops its `**/` segment.
const CASES = [
{ root: "src/*.ts", depthZero: "src/index.ts", nested: "src/api/routes.ts", outside: "docs/index.ts" },
{ root: "src/*.tsx", depthZero: "src/App.tsx", nested: "src/components/App.tsx", outside: "docs/App.tsx" },
{
root: "packages/*/src/*.ts",
depthZero: "packages/discovery-index/src/app.ts",
nested: "packages/discovery-index/src/ingest/app.ts",
outside: "packages/discovery-index/test/app.ts",
},
{
root: "packages/*/src/*.tsx",
depthZero: "packages/loopover-ui-kit/src/card.tsx",
nested: "packages/loopover-ui-kit/src/components/card.tsx",
outside: "packages/loopover-ui-kit/test/card.tsx",
},
{
// outside deliberately avoids bin/ -- packages/*/bin/** matches every file under bin/ regardless
// of extension, so a bin/ path would pass for the wrong reason.
root: "packages/*/lib/*.js",
depthZero: "packages/loopover-mcp/lib/tools.js",
nested: "packages/loopover-mcp/lib/resources/tools.js",
outside: "packages/loopover-mcp/test/tools.js",
},
{
root: "packages/*/lib/*.ts",
depthZero: "packages/loopover-mcp/lib/tools.ts",
nested: "packages/loopover-mcp/lib/resources/tools.ts",
outside: "packages/loopover-mcp/test/tools.ts",
},
{
root: "packages/*/scripts/*.mjs",
depthZero: "packages/loopover-engine/scripts/build.mjs",
nested: "packages/loopover-engine/scripts/codegen/build.mjs",
outside: "packages/loopover-engine/test/build.mjs",
},
{
root: "apps/*/src/*.ts",
depthZero: "apps/loopover-ui/src/main.ts",
nested: "apps/loopover-ui/src/lib/main.ts",
outside: "apps/loopover-ui/scripts/main.ts",
},
{
root: "apps/*/src/*.tsx",
depthZero: "apps/loopover-ui/src/main.tsx",
nested: "apps/loopover-ui/src/routes/main.tsx",
outside: "apps/loopover-ui/scripts/main.tsx",
},
{
root: "apps/*/scripts/*.mjs",
depthZero: "apps/loopover-ui/scripts/build.mjs",
nested: "apps/loopover-ui/scripts/codegen/build.mjs",
outside: "apps/loopover-ui/src/build.mjs",
},
] as const;

it.each(CASES)("$root matches a depth-0 file, still matches a nested file, and rejects an outside file", ({ root, depthZero, nested, outside }) => {
expect(BRANDING_DRIFT_PATHSPECS).toContain(root);
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, depthZero)).toBe(true);
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, nested)).toBe(true);
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, outside)).toBe(false);
});

it("packages/*/bin/** is untouched -- a bare ** tail already matches every depth, including depth 0", () => {
expect(BRANDING_DRIFT_PATHSPECS).toContain("packages/*/bin/**");
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, "packages/loopover-mcp/bin/cli.js")).toBe(true);
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, "packages/loopover-mcp/bin/nested/cli.js")).toBe(true);
});

it("still excludes test files at every depth, including depth 0", () => {
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, "src/index.test.ts")).toBe(false);
expect(matchesPathspecs(BRANDING_DRIFT_PATHSPECS, "packages/loopover-mcp/test/tools.ts")).toBe(false);
});

it("contains no **/ path segment except packages/*/bin/** and the three untouched :(exclude) entries", () => {
const allowedDoubleStarEntries = ["packages/*/bin/**", ":(exclude)**/*.test.ts", ":(exclude)**/*.test.tsx", ":(exclude)packages/*/test/**"];
for (const spec of BRANDING_DRIFT_PATHSPECS) {
if (spec.includes("**")) {
expect(allowedDoubleStarEntries).toContain(spec);
}
}
});
});

describe("check-branding-drift script (real repo state)", () => {
// Most important test in this file: proves the checked-in baseline actually matches the real repo right
// now. If this fails, real drift landed (or a cleanup did) without regenerating the baseline -- either way,
Expand Down