Skip to content

Commit 3f59065

Browse files
authored
Merge pull request #3272 from GCWing/gcwing/ship-build-cache-and-pet-focus
fix: repair build cache preflight and hide companion bubbles on focus
2 parents 751c7d1 + 1d1393b commit 3f59065

7 files changed

Lines changed: 386 additions & 36 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,10 @@ jobs:
660660
if: needs.build-impact.outputs.frontend_required != 'false'
661661
run: pnpm --dir src/mobile-web run type-check
662662

663+
- name: Validate mobile-web build cache contract
664+
if: needs.build-impact.outputs.frontend_required != 'false'
665+
run: node --test scripts/mobile-web-build.test.mjs
666+
663667
- name: Test mobile web account login and reload
664668
if: needs.build-impact.outputs.frontend_required != 'false'
665669
run: pnpm --dir src/mobile-web run test:account-login

‎scripts/check-build-prereqs.mjs‎

Lines changed: 118 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
*
88
* - Root node_modules missing → pnpm scripts fail with "node_modules missing,
99
* did you mean to install?"
10+
* - A package directory inside node_modules/.pnpm unexpectedly empty (or a
11+
* dangling link) → the matching node_modules/.bin shim resolves to a path
12+
* without content, and nested tools fail with a confusing "Cannot find
13+
* module .../bin/<tool>" error, for example
14+
* ".../design-system/packages/ui/node_modules/typescript/bin/tsc"
1015
* - src/mobile-web/dist missing → cargo check -p openbitfun-desktop and
1116
* cargo check --workspace fail with "resource path '../../mobile-web/dist'
1217
* doesn't exist" in the openbitfun-desktop build script
@@ -24,8 +29,8 @@
2429
*/
2530

2631
import { execFileSync } from 'node:child_process';
27-
import { existsSync } from 'node:fs';
28-
import { join, dirname } from 'node:path';
32+
import { existsSync, readdirSync, rmSync } from 'node:fs';
33+
import { join, dirname, relative, sep } from 'node:path';
2934
import { fileURLToPath } from 'node:url';
3035

3136
const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -36,6 +41,69 @@ const FIX = process.argv.includes('--fix');
3641

3742
// --- Check logic (extracted for re-use and testing) ---
3843

44+
const VIRTUAL_STORE_SAMPLE_LIMIT = 5;
45+
46+
function readdirWithTypes(dir) {
47+
try {
48+
return readdirSync(dir, { withFileTypes: true });
49+
} catch {
50+
return [];
51+
}
52+
}
53+
54+
/**
55+
* Package directories inside node_modules/.pnpm that pnpm recorded but whose
56+
* files are gone: an existing but empty package directory, or a link whose
57+
* target disappeared. Either one makes the workspace bin shims resolve to a
58+
* path without content.
59+
*/
60+
function findBrokenVirtualStorePackages(rootDir) {
61+
const virtualStoreDir = join(rootDir, 'node_modules', '.pnpm');
62+
if (!existsSync(virtualStoreDir)) {
63+
return [];
64+
}
65+
66+
const isBroken = (packageDir, entry) => {
67+
// readdir reports the name of a dangling link, but the path cannot be opened.
68+
if (!existsSync(packageDir)) {
69+
return true;
70+
}
71+
// Links to peer packages hold no files themselves; their target is checked
72+
// through its own entry in the virtual store.
73+
if (entry.isSymbolicLink()) {
74+
return false;
75+
}
76+
return readdirWithTypes(packageDir).length === 0;
77+
};
78+
79+
const broken = [];
80+
for (const storeEntry of readdirWithTypes(virtualStoreDir)) {
81+
if (!storeEntry.isDirectory()) {
82+
continue;
83+
}
84+
const storeNodeModules = join(virtualStoreDir, storeEntry.name, 'node_modules');
85+
86+
for (const entry of readdirWithTypes(storeNodeModules)) {
87+
if (entry.name.startsWith('@')) {
88+
const scopeDir = join(storeNodeModules, entry.name);
89+
for (const scopedEntry of readdirWithTypes(scopeDir)) {
90+
const packageDir = join(scopeDir, scopedEntry.name);
91+
if (isBroken(packageDir, scopedEntry)) {
92+
broken.push(packageDir);
93+
}
94+
}
95+
continue;
96+
}
97+
const packageDir = join(storeNodeModules, entry.name);
98+
if (isBroken(packageDir, entry)) {
99+
broken.push(packageDir);
100+
}
101+
}
102+
}
103+
104+
return broken;
105+
}
106+
39107
function runChecks(rootDir) {
40108
const errors = [];
41109
const warnings = [];
@@ -49,7 +117,30 @@ function runChecks(rootDir) {
49117
});
50118
}
51119

52-
// --- Check 2: mobile-web dist (required by openbitfun-desktop build script) ---
120+
// --- Check 2: pnpm virtual store integrity ---
121+
const brokenPackages = findBrokenVirtualStorePackages(rootDir);
122+
if (brokenPackages.length > 0) {
123+
const samples = brokenPackages
124+
.slice(0, VIRTUAL_STORE_SAMPLE_LIMIT)
125+
.map((packageDir) => relative(rootDir, packageDir));
126+
const remaining = brokenPackages.length - samples.length;
127+
const sampleText = remaining > 0 ? `${samples.join(', ')}, ...(+${remaining} more)` : samples.join(', ');
128+
129+
errors.push({
130+
name: 'pnpm virtual store',
131+
message:
132+
`${brokenPackages.length} installed package(s) are missing their files: ${sampleText}. ` +
133+
'The matching node_modules/.bin shims point at a path without content, so nested tools fail with ' +
134+
'a confusing "Cannot find module .../bin/<tool>" error. pnpm install alone does not repair this: ' +
135+
'pnpm leaves an existing empty package directory untouched, so the broken directories must be removed first.',
136+
fix: ['pnpm', 'install'],
137+
cleanPaths: brokenPackages,
138+
fixNote:
139+
'--fix removes the broken package directories first, because pnpm install would otherwise leave them empty. To repair by hand, delete the paths above and run pnpm install.',
140+
});
141+
}
142+
143+
// --- Check 3: mobile-web dist (required by openbitfun-desktop build script) ---
53144
if (!existsSync(join(rootDir, 'src', 'mobile-web', 'dist', 'index.html'))) {
54145
errors.push({
55146
name: 'mobile-web dist',
@@ -59,7 +150,7 @@ function runChecks(rootDir) {
59150
});
60151
}
61152

62-
// --- Check 3: OpenCode extension Host dist (product runtime resource) ---
153+
// --- Check 4: OpenCode extension Host dist (product runtime resource) ---
63154
const pluginHostDist = join(
64155
rootDir,
65156
'src',
@@ -77,7 +168,7 @@ function runChecks(rootDir) {
77168
});
78169
}
79170

80-
// --- Check 4: sherpa-onnx prebuilt libs ---
171+
// --- Check 5: sherpa-onnx prebuilt libs ---
81172
// sherpa-onnx-sys build.rs auto-detects target/sherpa-onnx-prebuilt/<version>/lib/
82173
// and returns immediately without downloading. Only warn for the first-build
83174
// scenario where no prebuilt cache exists yet.
@@ -106,7 +197,11 @@ function runChecks(rootDir) {
106197
function collectPendingFixes(errors) {
107198
return errors
108199
.filter((e) => e.fix)
109-
.map((e) => ({ name: e.name, fix: e.fix }));
200+
.map((e) => ({
201+
name: e.name,
202+
fix: e.fix,
203+
cleanPaths: e.cleanPaths ?? [],
204+
}));
110205
}
111206

112207
function reportResults({ errors, warnings }) {
@@ -117,6 +212,9 @@ function reportResults({ errors, warnings }) {
117212
if (e.fix) {
118213
console.error(` Fix: ${e.fix.join(' ')}`);
119214
}
215+
if (e.fixNote) {
216+
console.error(` Note: ${e.fixNote}`);
217+
}
120218
}
121219
console.error();
122220
}
@@ -132,7 +230,20 @@ function reportResults({ errors, warnings }) {
132230

133231
function runFixes(pendingFixes, rootDir) {
134232
let allSucceeded = true;
135-
for (const { name, fix } of pendingFixes) {
233+
for (const { fix, cleanPaths } of pendingFixes) {
234+
for (const target of cleanPaths) {
235+
// Only ever delete inside the virtual store: a broken path must not be
236+
// allowed to escape into source or user data.
237+
const virtualStorePrefix = `${join(rootDir, 'node_modules', '.pnpm')}${sep}`;
238+
if (!target.startsWith(virtualStorePrefix)) {
239+
console.error(`Refusing to remove path outside the pnpm virtual store: ${target}\n`);
240+
allSucceeded = false;
241+
continue;
242+
}
243+
console.log(`Removing broken package directory ${relative(rootDir, target)}`);
244+
rmSync(target, { recursive: true, force: true });
245+
}
246+
136247
const [cmd, ...args] = fix;
137248
console.log(`$ ${fix.join(' ')}`);
138249
try {

‎scripts/check-build-prereqs.test.mjs‎

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import assert from 'node:assert/strict';
2-
import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
2+
import {
3+
chmodSync,
4+
mkdirSync,
5+
mkdtempSync,
6+
readFileSync,
7+
rmSync,
8+
writeFileSync,
9+
} from 'node:fs';
310
import { tmpdir } from 'node:os';
411
import path from 'node:path';
512
import { spawnSync } from 'node:child_process';
@@ -17,13 +24,26 @@ function createTestRoot({
1724
mobileWebDist = false,
1825
pluginHostDist = false,
1926
sherpaOnnx = null,
27+
virtualStoreEntries = null,
2028
} = {}) {
2129
const root = mkdtempSync(path.join(tmpdir(), 'openbitfun-build-prereqs-'));
2230

2331
if (nodeModules) {
2432
mkdirSync(path.join(root, 'node_modules'), { recursive: true });
2533
}
2634

35+
// Maps a virtual store package path, relative to the root, to the files it
36+
// should contain. An empty file list models a package whose files are gone.
37+
if (virtualStoreEntries) {
38+
for (const [relativeDir, files] of Object.entries(virtualStoreEntries)) {
39+
const packageDir = path.join(root, relativeDir);
40+
mkdirSync(packageDir, { recursive: true });
41+
for (const file of files) {
42+
writeFileSync(path.join(packageDir, file), '{}');
43+
}
44+
}
45+
}
46+
2747
if (mobileWebDist) {
2848
const distDir = path.join(root, 'src', 'mobile-web', 'dist');
2949
mkdirSync(distDir, { recursive: true });
@@ -65,10 +85,24 @@ function createFakePnpm() {
6585
writeFileSync(
6686
fakePnpmPath,
6787
`
68-
const { mkdirSync, writeFileSync } = require('fs');
88+
const { existsSync, mkdirSync, writeFileSync } = require('fs');
89+
const path = require('path');
6990
const args = process.argv.slice(2);
7091
if (args[0] === 'install') {
7192
mkdirSync('node_modules', { recursive: true });
93+
const restorePath = process.env.FAKE_PNPM_RESTORE_PATH;
94+
if (restorePath) {
95+
// Records whether the broken package was still present when install ran,
96+
// which is what pnpm itself would trip over.
97+
if (process.env.FAKE_PNPM_REPORT_PATH) {
98+
writeFileSync(
99+
process.env.FAKE_PNPM_REPORT_PATH,
100+
JSON.stringify({ dirExistedWhenInstallRan: existsSync(restorePath) }),
101+
);
102+
}
103+
mkdirSync(restorePath, { recursive: true });
104+
writeFileSync(path.join(restorePath, 'package.json'), '{}');
105+
}
72106
} else if (args[0] === 'run' && args[1] === 'prepare:mobile-web') {
73107
mkdirSync('src/mobile-web/dist', { recursive: true });
74108
writeFileSync('src/mobile-web/dist/index.html', '<html></html>');
@@ -94,7 +128,10 @@ if (args[0] === 'install') {
94128
return binDir;
95129
}
96130

97-
function runCheck(root, { fix = false, extraPath = null, sherpaEnv = null } = {}) {
131+
function runCheck(
132+
root,
133+
{ fix = false, extraPath = null, sherpaEnv = null, extraEnv = null } = {},
134+
) {
98135
const env = {
99136
...process.env,
100137
OPENBITFUN_BUILD_PREREQS_TEST_ROOT: root,
@@ -109,6 +146,9 @@ function runCheck(root, { fix = false, extraPath = null, sherpaEnv = null } = {}
109146
env.SHERPA_ONNX_LIB_DIR = sherpaEnv;
110147
}
111148
}
149+
if (extraEnv) {
150+
Object.assign(env, extraEnv);
151+
}
112152

113153
const args = fix ? [scriptPath, '--fix'] : [scriptPath];
114154

@@ -124,6 +164,12 @@ test('passes when all prerequisites are present (including sherpa-onnx prebuilt)
124164
mobileWebDist: true,
125165
pluginHostDist: true,
126166
sherpaOnnx: ['sherpa-onnx-v1.13.4-osx-arm64-static-lib'],
167+
virtualStoreEntries: {
168+
'node_modules/.pnpm/typescript@5.8.3/node_modules/typescript': ['package.json'],
169+
'node_modules/.pnpm/@openbitfun+ui@0.1.0/node_modules/@openbitfun/ui': [
170+
'package.json',
171+
],
172+
},
127173
});
128174
t.after(() => rmSync(root, { recursive: true, force: true }));
129175

@@ -134,6 +180,72 @@ test('passes when all prerequisites are present (including sherpa-onnx prebuilt)
134180
assert.doesNotMatch(result.stderr, /\[WARN\]/);
135181
});
136182

183+
test('fails when a pnpm virtual store package has no files', (t) => {
184+
const root = createTestRoot({
185+
nodeModules: true,
186+
mobileWebDist: true,
187+
pluginHostDist: true,
188+
sherpaOnnx: ['sherpa-onnx-v1.13.4-osx-arm64-static-lib'],
189+
virtualStoreEntries: {
190+
'node_modules/.pnpm/typescript@5.8.3/node_modules/typescript': [],
191+
},
192+
});
193+
t.after(() => rmSync(root, { recursive: true, force: true }));
194+
195+
const result = runCheck(root, { sherpaEnv: '' });
196+
197+
assert.notEqual(result.status, 0);
198+
assert.match(result.stderr, /\[FAIL\] pnpm virtual store/);
199+
assert.match(
200+
result.stderr,
201+
/node_modules[\\/]\.pnpm[\\/]typescript@5\.8\.3[\\/]node_modules[\\/]typescript/,
202+
);
203+
assert.match(result.stderr, /Fix: pnpm install/);
204+
assert.match(result.stderr, /does not repair this/);
205+
});
206+
207+
test('--fix removes broken virtual store packages before running pnpm install', (t) => {
208+
const binDir = createFakePnpm();
209+
const brokenRelativeDir =
210+
'node_modules/.pnpm/typescript@5.8.3/node_modules/typescript';
211+
const root = createTestRoot({
212+
nodeModules: true,
213+
mobileWebDist: true,
214+
pluginHostDist: true,
215+
sherpaOnnx: ['sherpa-onnx-v1.13.4-osx-arm64-static-lib'],
216+
virtualStoreEntries: { [brokenRelativeDir]: [] },
217+
});
218+
const reportPath = path.join(root, 'fake-pnpm-report.json');
219+
t.after(() => {
220+
rmSync(root, { recursive: true, force: true });
221+
rmSync(binDir, { recursive: true, force: true });
222+
});
223+
224+
const result = runCheck(root, {
225+
fix: true,
226+
extraPath: binDir,
227+
sherpaEnv: '',
228+
extraEnv: {
229+
FAKE_PNPM_RESTORE_PATH: path.join(root, brokenRelativeDir),
230+
FAKE_PNPM_REPORT_PATH: reportPath,
231+
},
232+
});
233+
234+
assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`);
235+
assert.match(
236+
result.stdout,
237+
/Removing broken package directory node_modules[\\/]\.pnpm[\\/]typescript@5\.8\.3[\\/]node_modules[\\/]typescript/,
238+
);
239+
assert.match(result.stdout, /\$ pnpm install/);
240+
assert.match(result.stdout, /All errors resolved/);
241+
// The empty directory has to be gone before install runs; pnpm does not
242+
// repair a package directory that still exists with no files.
243+
assert.equal(
244+
JSON.parse(readFileSync(reportPath, 'utf8')).dirExistedWhenInstallRan,
245+
false,
246+
);
247+
});
248+
137249
test('fails when root node_modules is missing', (t) => {
138250
const root = createTestRoot({
139251
mobileWebDist: true,

0 commit comments

Comments
 (0)