From 6d45f98665004d314468d98e50084996f4046cdf Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 18 Aug 2026 07:36:25 -0600 Subject: [PATCH 1/2] ci: make file-size policy a first-class gate (#6187) ## Summary - make the differential file-size ratchet a first-class repository gate - run the same unfiltered gate from pre-push, `just check`, and CI - remove hidden file-size coupling from Desktop, Web, and Mobile lint commands - isolate ratchet Git subprocesses from hook-exported repository state ## Why The Desktop ratchet grew to govern `desktop/src-tauri/crates/**`, but the pre-push `desktop-check` command remained path-filtered to non-Tauri files. That contract drift allowed a Tauri Rust file-size regression through local validation. The ratchet already computes its own merge-base diff, so duplicating governed paths in Lefthook and CI adds drift risk without meaningful runtime savings. One root gate owns the policy now. ## Testing - `just file-size-check` - oversized untracked probe under `desktop/src-tauri/crates/**` fails with the 1,000-line ceiling - file-size core tests with hook-style `GIT_DIR` / `GIT_WORK_TREE` environment - `lefthook dump` confirms the unfiltered pre-push command - mandatory pre-push suite passed on `3217003db10c84d8a0c5f636ec4c92936777a002` - `git diff --check` Signed-off-by: Wes Co-authored-by: Carl --- .github/workflows/ci.yml | 13 +++---------- AGENTS.md | 24 ++++++++++++------------ Justfile | 19 ++++++++++++++----- desktop/package.json | 2 +- lefthook.yml | 7 +++++++ scripts/check-file-sizes-core.mjs | 6 ++++++ scripts/check-file-sizes-core.test.mjs | 12 +++++++++++- web/package.json | 2 +- 8 files changed, 55 insertions(+), 30 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13d1424ea7..a832c0a0af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,7 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: fetch-depth: 2 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: @@ -50,8 +51,6 @@ jobs: - 'scripts/normative-corpus.json' - 'justfile' desktop: - - 'scripts/check-file-sizes-core.mjs' - - 'scripts/check-file-sizes-core.test.mjs' - 'scripts/model-capabilities.json' - 'scripts/normative-corpus.json' - 'desktop/**' @@ -60,13 +59,9 @@ jobs: desktop-rust: - 'desktop/src-tauri/**' web: - - 'scripts/check-file-sizes-core.mjs' - - 'scripts/check-file-sizes-core.test.mjs' - 'web/**' - 'pnpm-lock.yaml' mobile: - - 'scripts/check-file-sizes-core.mjs' - - 'scripts/check-file-sizes-core.test.mjs' - 'mobile/**' - 'scripts/mobile-release.sh' - 'scripts/mobile-worktree-overrides.sh' @@ -92,8 +87,8 @@ jobs: scripts/test-mobile-release-candidate-publisher.sh - name: Mobile worktree identity contract run: scripts/test-mobile-worktree-overrides.sh - - name: File size ratchet unit tests - run: node --test scripts/check-file-sizes-core.test.mjs + - name: File size policy + run: just file-size-check rust-lint: name: Rust Lint @@ -896,8 +891,6 @@ jobs: with: path: ~/.pub-cache key: pub-${{ runner.os }}-${{ hashFiles('mobile/pubspec.lock') }} - - name: File size ratchet - run: node mobile/scripts/check-file-sizes.mjs - name: Format check run: cd mobile && dart format --output=none --set-exit-if-changed . - name: Analyze diff --git a/AGENTS.md b/AGENTS.md index d66ecbdc74..3a79c4b38d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,14 +120,14 @@ Run `just test` for integration tests if you touched `buzz-relay`, formatting via `stage_fixed`. Pre-commit runs fix variants in parallel (Rust fmt, Tauri Rust fmt, desktop biome fix, web biome fix, mobile dart format). Auto-fixable issues are fixed and re-staged; unfixable lint issues block the -commit. **Pre-push hooks** run clippy (workspace + Tauri), desktop TypeScript -typechecking (`tsc --noEmit`), and fast unit tests in parallel (Rust, desktop -JS, Tauri Rust, mobile Flutter) — no overlap with pre-commit. Builds are -CI-only. Run `just fix-all` to auto-fix all formatting in one shot. Run -`just ci` for the full local gate. Run `just hooks` to -re-install hooks after env changes. Before agents run Git or hooks, activate the -repo's Hermit environment (`. ./bin/activate-hermit`); do not rewrite hook -commands to compensate for an unconfigured shell `PATH`. +commit. **Pre-push hooks** run the repository-wide differential file-size gate, +clippy (workspace + Tauri), desktop TypeScript typechecking (`tsc --noEmit`), +and fast unit tests in parallel (Rust, desktop JS, Tauri Rust, mobile Flutter) +— no overlap with pre-commit. Builds are CI-only. Run `just fix-all` to auto-fix +all formatting in one shot. Run `just ci` for the full local gate. Run `just +hooks` to re-install hooks after env changes. Before agents run Git or hooks, +activate the repo's Hermit environment (`. ./bin/activate-hermit`); do not +rewrite hook commands to compensate for an unconfigured shell `PATH`. **Commit with `git commit -s`.** The required **DCO Check** fails any PR with a commit missing a `Signed-off-by` trailer, and `just hooks` installs a `commit-msg` hook that adds it to commits you create locally (`git rebase` and `git cherry-pick` still need `--signoff`) — if you build commit commands programmatically, include `-s` every time. To repair a branch that already has unsigned commits: `git rebase --signoff main`, then force-push. @@ -566,10 +566,10 @@ The mobile app lives in `mobile/` — a Flutter app using Riverpod + Hooks. - **Keep widgets small and composable.** One public widget per file; push private sub-widgets (`_Foo`) into sibling `part` files under a `/` folder rather than growing the page file. Hard ceiling: - **1000 lines/file**, enforced by `mobile/scripts/check-file-sizes.mjs` via - `just mobile-check` (runs in `just check` + pre-push, mirroring desktop/web). - If the guard trips, **split the file — never bump the limit or add an - override to slip under it.** + **1000 lines/file**, enforced across Desktop, Web, and Mobile by the + repository-level `just file-size-check` gate (`just check`, CI, and every + pre-push). If the guard trips, **split the file — never bump the limit or add + an override to slip under it.** - Feature modules must not import from other feature modules — only from `shared/`. - Use `Grid` tokens for spacing, `Radii` for border radius. diff --git a/Justfile b/Justfile index 5b2ed88c95..ce8647cf77 100644 --- a/Justfile +++ b/Justfile @@ -91,8 +91,17 @@ build: build-release: cargo build --workspace --release -# Run repo lint and formatting checks -check: fmt-check clippy desktop-check desktop-tauri-fmt-check desktop-tauri-clippy web-check mobile-check +# Run repo lint, formatting, and repository policy checks +check: fmt-check clippy desktop-check desktop-tauri-fmt-check desktop-tauri-clippy web-check mobile-check file-size-check + +# Run the repository-wide differential file-size ratchet and its policy tests. +# The ratchet inspects only files changed from the merge base, so this stays +# cheap enough to run unconditionally without duplicating path filters. +file-size-check: + node --test scripts/check-file-sizes-core.test.mjs + node desktop/scripts/check-file-sizes.mjs + node web/scripts/check-file-sizes.mjs + node mobile/scripts/check-file-sizes.mjs # Format all Rust code fmt: @@ -120,7 +129,7 @@ desktop-check: # Fix desktop lint and format issues desktop-fix: - cd {{desktop_dir}} && pnpm exec biome check --write . && pnpm check:file-sizes + cd {{desktop_dir}} && pnpm exec biome check --write . # Run desktop TS helper unit tests desktop-test: @@ -641,7 +650,7 @@ web-check: # Fix web lint and format issues web-fix: - cd {{web_dir}} && pnpm exec biome check --write . && pnpm check:file-sizes + cd {{web_dir}} && pnpm exec biome check --write . # Run web TypeScript checks web-typecheck: @@ -673,7 +682,7 @@ mobile-fix: # Run mobile lint and format checks mobile-check: - unset GIT_DIR GIT_WORK_TREE; cd {{mobile_dir}} && dart format --output=none --set-exit-if-changed . && flutter analyze && node ./scripts/check-file-sizes.mjs + unset GIT_DIR GIT_WORK_TREE; cd {{mobile_dir}} && dart format --output=none --set-exit-if-changed . && flutter analyze # Run mobile tests mobile-test: diff --git a/desktop/package.json b/desktop/package.json index 29d7d44863..073a764a1b 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -12,7 +12,7 @@ "check:px-text": "node ./scripts/check-px-text.mjs", "check:pubkey-truncation": "node ./scripts/check-pubkey-truncation.mjs", "lint": "biome lint .", - "check": "biome check . && pnpm check:file-sizes && pnpm check:px-text && pnpm check:pubkey-truncation", + "check": "biome check . && pnpm check:px-text && pnpm check:pubkey-truncation", "format": "biome format --write .", "test": "node --import ./test-loader.mjs --experimental-strip-types --test \"src/**/*.test.mjs\"", "preview": "vite preview", diff --git a/lefthook.yml b/lefthook.yml index 5b992f19af..d3c8bddb83 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -6,6 +6,9 @@ # changes, though CI's Desktop Core job does. Those commands are pure TS # (biome + tsc + node:test) with no Rust dependency, so the extra trigger # would be spurious locally. +# - The repository-wide `file-size-check` is deliberately unfiltered. Its own +# merge-base diff is the path filter; duplicating its governed roots here is +# the coverage drift this gate is intended to prevent. # - Deletion-only surface changes do not trigger local hooks: lefthook 2.1.x # drops deleted paths from push-file discovery (`extractFiles` existence # check, repository.go). CI's dorny/paths-filter catches deletions. @@ -51,6 +54,10 @@ pre-push: commands: branch-skew: run: ./scripts/check-branch-skew.sh + file-size-check: + # The ratchet computes its own merge-base diff, so path filtering here + # would only duplicate policy and create another place for coverage drift. + run: just file-size-check rust-tests: glob: ["crates/**", "migrations/**", "schema/**", "Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "deny.toml", "scripts/run-tests.sh", "justfile"] run: just test-unit diff --git a/scripts/check-file-sizes-core.mjs b/scripts/check-file-sizes-core.mjs index 1365424628..6c3b8c4bb9 100644 --- a/scripts/check-file-sizes-core.mjs +++ b/scripts/check-file-sizes-core.mjs @@ -3,10 +3,16 @@ import { promises as fs } from "node:fs"; import path from "node:path"; function git(args, cwd, options = {}) { + // Git hooks export repository-local GIT_* variables. Child commands that + // intentionally target `cwd` must not be redirected back to the hook's repo. + const env = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_")), + ); return execFileSync("git", args, { cwd, encoding: "utf8", maxBuffer: 10 * 1024 * 1024, + env, ...options, }); } diff --git a/scripts/check-file-sizes-core.test.mjs b/scripts/check-file-sizes-core.test.mjs index 14dfe4b8da..9b4b910404 100644 --- a/scripts/check-file-sizes-core.test.mjs +++ b/scripts/check-file-sizes-core.test.mjs @@ -13,7 +13,17 @@ import { } from "./check-file-sizes-core.mjs"; function git(repo, ...args) { - return execFileSync("git", args, { cwd: repo, encoding: "utf8" }).trim(); + // These fixture repositories inherit both hook configuration and Git's + // repository-local environment when this test runs from pre-push. Isolate + // them completely so fixture commits cannot recurse into the real checkout. + const env = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_")), + ); + return execFileSync("git", ["-c", "core.hooksPath=/dev/null", ...args], { + cwd: repo, + encoding: "utf8", + env, + }).trim(); } test("local base resolution uses the branch merge-base and fails without origin/main", () => { diff --git a/web/package.json b/web/package.json index a074a581dc..d932a612c8 100644 --- a/web/package.json +++ b/web/package.json @@ -10,7 +10,7 @@ "check:file-sizes": "node ./scripts/check-file-sizes.mjs", "check:pubkey-truncation": "node ./scripts/check-pubkey-truncation.mjs", "lint": "biome lint .", - "check": "biome check . && pnpm check:file-sizes && pnpm check:pubkey-truncation", + "check": "biome check . && pnpm check:pubkey-truncation", "format": "biome format --write .", "preview": "vite preview", "test:e2e": "pnpm build && playwright test", From 081910424a5b6f01b283ad632b0718240c6b3cbf Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 18 Aug 2026 07:36:50 -0600 Subject: [PATCH 2/2] fix(desktop): bind presence retry timers (#6213) ## Summary - preserve the WebKit-required `Window` receiver when scheduling and clearing presence subscription retries - add a receiver-sensitive regression test covering both retry creation and disposal ## Impact When subscription opening failed, WebKit rejected the detached timer call before `retryTimer` could be set. The reconciler's `finally` block then immediately started another reconciliation because demand was still unsatisfied and no retry appeared pending. This bypassed the intended exponential backoff and could repeatedly reopen subscription work during startup, so the impact was more than console noise. With the timer receiver fixed, a failed open schedules one bounded retry at a time (1s exponential backoff, capped at 30s), and disposal cancels it correctly. ## Validation - `cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/presence/lib/presenceSubscriptionReconciler.test.mjs` (11 passed) - `cd desktop && pnpm test` (4,993 passed) - `cd desktop && pnpm typecheck` - `cd desktop && pnpm check` (passes with existing unrelated warnings) - pre-push hook on `578b9a0b5c851b21de0b8746238b04dc69032e78` (desktop check/typecheck/tests, Rust tests, Tauri checks, mobile tests all passed) Signed-off-by: Wes Co-authored-by: Carl --- .../presenceSubscriptionReconciler.test.mjs | 35 +++++++++++++++++++ .../lib/presenceSubscriptionReconciler.ts | 7 ++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/desktop/src/features/presence/lib/presenceSubscriptionReconciler.test.mjs b/desktop/src/features/presence/lib/presenceSubscriptionReconciler.test.mjs index f86000dfbf..19b0249839 100644 --- a/desktop/src/features/presence/lib/presenceSubscriptionReconciler.test.mjs +++ b/desktop/src/features/presence/lib/presenceSubscriptionReconciler.test.mjs @@ -75,6 +75,41 @@ test("a stale async open is closed and never installed", async () => { reconciler.dispose(); }); +test("default timers preserve their global receiver when scheduling retries", async () => { + const originalSetTimeout = globalThis.setTimeout; + const originalClearTimeout = globalThis.clearTimeout; + const timers = []; + const cleared = []; + + globalThis.setTimeout = function (callback) { + assert.equal(this, globalThis); + timers.push(callback); + return timers.length; + }; + globalThis.clearTimeout = function (timer) { + assert.equal(this, globalThis); + cleared.push(timer); + }; + + try { + const reconciler = new PresenceSubscriptionReconciler({ + open: async () => { + throw new Error("relay unavailable"); + }, + }); + + reconciler.setAuthors([A]); + await Promise.resolve(); + assert.equal(timers.length, 1); + + reconciler.dispose(); + assert.deepEqual(cleared, [1]); + } finally { + globalThis.setTimeout = originalSetTimeout; + globalThis.clearTimeout = originalClearTimeout; + } +}); + test("failed replacement preserves the previous subscription and retries", async () => { const timers = []; const actions = []; diff --git a/desktop/src/features/presence/lib/presenceSubscriptionReconciler.ts b/desktop/src/features/presence/lib/presenceSubscriptionReconciler.ts index 2831785ccf..dccadf8544 100644 --- a/desktop/src/features/presence/lib/presenceSubscriptionReconciler.ts +++ b/desktop/src/features/presence/lib/presenceSubscriptionReconciler.ts @@ -41,8 +41,11 @@ export class PresenceSubscriptionReconciler { this.retryDelay = options.retryDelay ?? ((attempt) => Math.min(1000 * 2 ** attempt, 30_000)); - this.setTimer = options.setTimer ?? setTimeout; - this.clearTimer = options.clearTimer ?? clearTimeout; + this.setTimer = + options.setTimer ?? + ((callback, delayMs) => globalThis.setTimeout(callback, delayMs)); + this.clearTimer = + options.clearTimer ?? ((timer) => globalThis.clearTimeout(timer)); } setAuthors(authors: string[]) {