From f7c8dedb46b01d6be5f86832448ead1ff33130c9 Mon Sep 17 00:00:00 2001 From: tusharbhardwaj-bk Date: Tue, 1 Sep 2026 16:07:43 +0000 Subject: [PATCH 1/2] fix(mobile): restore the build SHA in the reported app version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phone reports `1.0.4` where a fork build must report `1.0.4+bk.`. The `+bk.` suffix is the only way to tell which commit a binary came from, and docs/operations/bk-mobile-build.md leans on it to detect client/server version skew — so losing it made the fork blind to exactly the failure mode that document exists to prevent. I caused it. To stop `expo-constants` dragging expo-modules-core onto the import graph of tests reaching `authClientMetadata` (it reads `__DEV__` at import time, which vitest does not define), I made the manifest read a fail-soft `require`. That swallowed the real read too, so `bkBuildGitSha()` returned null in a genuine Expo runtime and every build since has reported a bare version. The repo already had the right answer: `features/cloud/publicConfig.test.ts` mocks expo-constants. So this restores the static import — matching every other consumer in the app — and mocks the module in `connection.test.ts` instead, which is where the import-graph problem actually was. Verified: 245 tests across 26 files, `vp run typecheck` clean. Co-Authored-By: Claude Opus 5 (1M context) --- apps/mobile/src/lib/bkBuildManifest.ts | 31 +++++++++----------------- apps/mobile/src/lib/connection.test.ts | 5 +++++ 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/apps/mobile/src/lib/bkBuildManifest.ts b/apps/mobile/src/lib/bkBuildManifest.ts index 810a5d9149f8..82028de50a3c 100644 --- a/apps/mobile/src/lib/bkBuildManifest.ts +++ b/apps/mobile/src/lib/bkBuildManifest.ts @@ -1,29 +1,18 @@ // T3-CUSTOM(expbkt3): The Expo-manifest half of the fork build identity. // // Split from bkBuildIdentity.ts so the version-formatting logic stays testable -// without pulling react-native into the unit test environment. That split was -// incomplete: `authClientMetadata` imports this module, so a module-scope -// `expo-constants` import put expo-modules-core on the import graph of every -// test reaching authClientMetadata, and expo-modules-core reads React Native's -// `__DEV__` global as a side effect — which vitest does not define. +// without pulling react-native into the unit test environment. // -// So the manifest read is both function-scoped and fail-soft. Outside a real -// Expo runtime there is no manifest to read, and "no manifest" already has a -// defined meaning here: no SHA, so `bkAppVersion` returns the plain version. -import { readBkGitSha } from "./bkBuildIdentity"; +// The import is static, like every other expo-constants consumer in the app. +// An earlier version made it a fail-soft `require` to keep expo-modules-core off +// the import graph of tests reaching `authClientMetadata` — that silently +// swallowed the real read too, so builds reported a bare `1.0.4` with no +// `+bk.` suffix and the version-skew check the fork depends on was blind. +// Tests mock this module instead, which is what the rest of the suite does. +import Constants from "expo-constants"; -function expoConfigExtra(): unknown { - try { - const loaded = require("expo-constants") as { - readonly default?: { readonly expoConfig?: { readonly extra?: unknown } }; - readonly expoConfig?: { readonly extra?: unknown }; - }; - return (loaded.default ?? loaded).expoConfig?.extra; - } catch { - return null; - } -} +import { readBkGitSha } from "./bkBuildIdentity"; export function bkBuildGitSha(): string | null { - return readBkGitSha(expoConfigExtra()); + return readBkGitSha(Constants.expoConfig?.extra); } diff --git a/apps/mobile/src/lib/connection.test.ts b/apps/mobile/src/lib/connection.test.ts index a76db40a49f7..36ec8a7f4976 100644 --- a/apps/mobile/src/lib/connection.test.ts +++ b/apps/mobile/src/lib/connection.test.ts @@ -1,4 +1,9 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +// T3-CUSTOM(expbkt3): authClientMetadata reads the Expo manifest for the fork +// build SHA, which pulls in expo-modules-core; that reads React Native's +// `__DEV__` global at import time and vitest does not define it. +vi.mock("expo-constants", () => ({ default: { expoConfig: null } })); import { EnvironmentId } from "@t3tools/contracts"; import { From 5c4f9f552b21ab2f274eb78e54e5070cd7bbb8e9 Mon Sep 17 00:00:00 2001 From: tusharbhardwaj-bk Date: Tue, 1 Sep 2026 18:55:08 +0000 Subject: [PATCH 2/2] fix(mobile): render the phase sidebar before the list that always wins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third attempt at the same bug, so this time the reachability is verified rather than assumed. `HomeScreen` has an early return `if (threadListV2Enabled)`, and `resolveThreadListV2Enabled` returns true unless a device opted into the legacy list — so that return fires on every phone. My phase-sidebar branch sat *after* it and was unreachable dead code. The toggle wrote its preference, the pane existed, and nothing could ever render it. Moved above that return. Verified order in HomeScreen is now: empty state → phase sidebar → V2 list. `ThreadNavigationSidebarPane` had the same shape of defect: it renders a thread list from two paths (`props.nativeChrome` and the fallback) and I had gated only the first, so the flag was half-wired on tablets too. Both paths now gate. The pattern across all three failures is identical — I put code somewhere plausible and never traced whether that line executes. Existence is not reachability, and neither typecheck nor unit tests can tell the difference. Verified: 280 tests across 31 files, `vp run typecheck` clean, lint clean, and the return order in both files read back explicitly. Co-Authored-By: Claude Opus 5 (1M context) --- apps/mobile/src/features/home/HomeScreen.tsx | 33 +++++++++---------- .../threads/ThreadNavigationSidebar.tsx | 16 +++++++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/apps/mobile/src/features/home/HomeScreen.tsx b/apps/mobile/src/features/home/HomeScreen.tsx index 2ae486cb64ab..e8ab5a492958 100644 --- a/apps/mobile/src/features/home/HomeScreen.tsx +++ b/apps/mobile/src/features/home/HomeScreen.tsx @@ -1140,6 +1140,22 @@ export function HomeScreen(props: HomeScreenProps) { listEmpty ); + // T3-CUSTOM(expbkt3): BEGIN — the experimental phase sidebar replaces the + // whole list. This MUST sit above the threadListV2Enabled return below: + // that flag defaults to true, so anything after it is unreachable. + if (phaseSidebarEnabled) { + return ( + + + + ); + } + // T3-CUSTOM(expbkt3): END + if (threadListV2Enabled) { return ( @@ -1186,23 +1202,6 @@ export function HomeScreen(props: HomeScreenProps) { ); } - // T3-CUSTOM(expbkt3): BEGIN — the experimental phase sidebar replaces this - // list entirely when enabled. It has to be wired here as well as in - // ThreadNavigationSidebar: that pane only renders in split view, so on a - // phone (compact layout) this screen IS the thread list. - if (phaseSidebarEnabled) { - return ( - - - - ); - } - // T3-CUSTOM(expbkt3): END - return ( {/* Sticky headers are deliberately not wired up: LegendList's JS sticky diff --git a/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx b/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx index 5308019ebc0e..7d2a242d0fe7 100644 --- a/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx +++ b/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx @@ -1264,6 +1264,22 @@ function ThreadNavigationSidebarPane( ); } + // T3-CUSTOM(expbkt3): BEGIN — the experimental sidebar replaces this render + // path too. The nativeChrome branch above has its own copy; both paths render + // a thread list, so gating only one leaves the flag half-wired. + if (phaseSidebarEnabled) { + return ( + + + + ); + } + // T3-CUSTOM(expbkt3): END + return (