From f1583056425e241cc5febf7df375fe17527a1799 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 19 Jul 2026 10:56:47 -0700 Subject: [PATCH] ci: narrow ui-preview scope, dedupe fumadocs/ui-kit rebuilds, fix TS buildinfo collision and vacuous whitespace check Five independently-verified fixes from a CI resource-usage audit, each adversarially reviewed before landing (two other proposed fixes from the same audit were rejected after adversarial review found they'd break real CI paths -- see conversation): - ui-preview.yml's path filter matched the whole packages/** workspace, triggering a pointless UI preview build+deploy (~101s) on every miner/mcp/engine/discovery-index-only PR. Narrowed to packages/loopover-ui-kit/**, the only package apps/loopover-ui actually depends on. - ci.yml had two independent, unaware-of-each-other steps regenerating the same fumadocs content collections -- one unconditional, one correctly gated on push||ui==true. Removed the unconditional duplicate. - ci.yml's UI lint/typecheck/test steps each triggered ui:kit:build's full aggregate script, redundantly rebuilding ui-kit a package "Build UI-kit package" already built once earlier in the same job. Scoped to call the underlying workspace subcommands directly instead -- package.json's ui:lint/ui:typecheck/ui:test themselves are left untouched, since ui-deploy.yml and CONTRIBUTING.md's documented local gate sequence both call them directly with no prior ui-kit build and depend on that self-sufficiency. - packages/loopover-engine/tsconfig.json inherited the root config's tsBuildInfoFile unresolved, so its build silently wrote to the same .tsbuildinfo the root Typecheck step caches -- masked today only by step order, a landmine for the next reorder. Given its own path, mirroring the identical fix already applied to packages/loopover-miner/tsconfig.json. - The `changes` job's whitespace check ran bare `git diff --check` against a fresh checkout, comparing the working tree to the index -- always identical right after checkout, so it silently passed regardless of the PR's actual diff. Now diffs against the real comparison SHA. --- .github/workflows/ci.yml | 43 +++++++++++++++------ .github/workflows/ui-preview.yml | 8 +++- packages/loopover-engine/tsconfig.json | 10 ++++- packages/loopover-engine/tsconfig.test.json | 8 +++- 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0839426e9..238eab3da2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,8 +48,22 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} + # `git diff --check` with no args compares the working tree against the index -- on this checkout's + # default shallow, single-commit fetch they're always identical immediately after checkout, so this + # unconditionally passed regardless of what the PR's actual diff contained (a false-green: the step + # promised a whitespace check that never fired). Fetch just the one comparison SHA (still shallow, + # cheap) and diff against it directly instead. On push (github.event.before can be the null SHA for + # a branch's first push -- not reachable for `main` itself, but kept defensive) or when the fetch + # target isn't resolvable, skip rather than fail the whole pipeline over a best-effort check. - name: Check whitespace - run: git diff --check + run: | + BASE_SHA="${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}" + if [ -z "$BASE_SHA" ] || [ "$BASE_SHA" = "0000000000000000000000000000000000000000" ]; then + echo "No comparison SHA available (e.g. branch's first push) -- skipping." + exit 0 + fi + git fetch --depth=1 origin "$BASE_SHA" + git diff --check "$BASE_SHA" HEAD - name: Filter changed paths id: filter uses: dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706 # v4 @@ -244,13 +258,12 @@ jobs: node_modules apps/loopover-ui/node_modules key: ${{ steps.node-modules-cache.outputs.cache-primary-key }} - # apps/loopover-ui/.source (fumadocs-mdx codegen -- the collections/* alias - # apps/loopover-ui/tsconfig.json depends on) lives outside node_modules and is normally produced by - # npm ci's own postinstall. A node_modules cache hit above skips npm ci entirely, so .source/ would - # otherwise never exist on a cache-hit run. Regenerate it explicitly and unconditionally: cheap, - # deterministic, and a harmless no-op re-run of the same postinstall on a cache miss. - - name: Generate fumadocs content-collection sources - run: npm --workspace @loopover/ui run postinstall + # apps/loopover-ui/.source (fumadocs-mdx codegen) is regenerated by "Generate docs content + # collections" below, gated on push||ui==true -- nothing between here and there reads it (docs-drift, + # env-reference, manifest/engine-parity/branding-drift checks all read raw source files directly, and + # the root Typecheck step's tsconfig.json excludes apps/loopover-ui entirely), so it doesn't need to + # be regenerated unconditionally this early too (a prior version of this step duplicated that one, + # added independently and unconditionally before either author noticed the other already existed). - name: Lint workflows if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} run: npm run actionlint @@ -497,15 +510,23 @@ jobs: - name: Generate docs content collections if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' }} run: npm run postinstall --workspace @loopover/ui + # ui:lint/ui:typecheck/ui:test (package.json) each independently re-run ui:kit:build as their own + # first step -- correct as standalone, portable scripts (nothing else in a bare `npm run ui:lint` + # invocation would have built ui-kit first), but redundant here specifically: "Build UI-kit package" + # above already built it once for all four of lint/typecheck/tests/build in this same job. Same + # "call the underlying subcommands directly, skip the redundant rebuild" fix "UI build" below + # already applies -- deliberately NOT changing the package.json scripts themselves, since + # ui-deploy.yml and CONTRIBUTING.md's documented local gate sequence both call ui:lint/ui:typecheck + # directly with no prior ui-kit build step and rely on that self-sufficiency. - name: UI lint if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' }} - run: npm run ui:lint + run: npm --workspace @loopover/ui run lint && npm --workspace @loopover/ui-miner run lint - name: UI typecheck if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' }} - run: npm run ui:typecheck + run: npm --workspace @loopover/ui run typecheck && npm --workspace @loopover/ui-miner run typecheck - name: UI tests if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' }} - run: npm run ui:test + run: npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test && npm --workspace @loopover/miner-extension run test - name: Extension lint if: ${{ github.event_name == 'push' || needs.changes.outputs.ui == 'true' }} run: npm run extension:lint && npm run miner-extension:lint diff --git a/.github/workflows/ui-preview.yml b/.github/workflows/ui-preview.yml index 9934e75caa..f9f32cc8f1 100644 --- a/.github/workflows/ui-preview.yml +++ b/.github/workflows/ui-preview.yml @@ -16,9 +16,15 @@ on: # build job below skips draft PRs, marking a PR ready must itself trigger a real preview build (#6670), # not wait for the next push. Mirrors ci.yml's pull_request.types comment/list exactly. types: [opened, synchronize, reopened, ready_for_review] + # Scoped to exactly what apps/loopover-ui actually depends on (packages/loopover-ui-kit only) -- + # NOT the whole "packages/**" workspace, which also matches loopover-miner/loopover-mcp/loopover-engine/ + # discovery-index and previously built+deployed a pointless preview for every PR touching those (#ci-scope). + # This check isn't required (see branch protection), so a future package apps/loopover-ui starts + # depending on that isn't added here will silently stop getting previews rather than failing loud -- + # keep this list in sync with what apps/loopover-ui/package.json actually imports. paths: - "apps/loopover-ui/**" - - "packages/**" + - "packages/loopover-ui-kit/**" permissions: contents: read diff --git a/packages/loopover-engine/tsconfig.json b/packages/loopover-engine/tsconfig.json index 36e5711aa1..46ad09e759 100644 --- a/packages/loopover-engine/tsconfig.json +++ b/packages/loopover-engine/tsconfig.json @@ -7,7 +7,15 @@ "declaration": true, "rootDir": "src", "outDir": "dist", - "noEmit": false + "noEmit": false, + // Without this, the inherited root value (tsconfig.json:24, "./.tsbuildinfo") resolves relative to + // the ROOT config's location, not this one -- this package's build and the root Typecheck step would + // then read/write the exact same cache file at the repo root and corrupt each other's incremental + // state (the same collision packages/loopover-miner/tsconfig.json already documents and fixes the + // same way). Currently masked in CI by step order (the root cache restore always runs after this + // package's build and overwrites whatever it just wrote, before Typecheck reads it) -- but that + // makes the inherited value a landmine for the next reordering, not a working fix. + "tsBuildInfoFile": "./.tsbuildinfo" }, "include": ["src"], "exclude": ["src/signals/engine.ts"] diff --git a/packages/loopover-engine/tsconfig.test.json b/packages/loopover-engine/tsconfig.test.json index 45f2856087..daaf30d0cc 100644 --- a/packages/loopover-engine/tsconfig.test.json +++ b/packages/loopover-engine/tsconfig.test.json @@ -5,7 +5,13 @@ "rootDir": "test", "outDir": "dist-test", "declaration": false, - "noEmit": false + "noEmit": false, + // Distinct from tsconfig.json's own "./.tsbuildinfo" (this config extends that file, so without an + // override here it would inherit the SAME explicit path -- two different rootDir/outDir/include sets + // sharing one incremental-state file, corrupting each other exactly like the collision this fixes). + // Named to END in ".tsbuildinfo" (not start with it) so it's still matched by .gitignore's existing + // "*.tsbuildinfo" glob -- ".tsbuildinfo-test" would NOT match that pattern and would leak as untracked. + "tsBuildInfoFile": "./.test.tsbuildinfo" }, "include": ["test"] }