diff --git a/DESIGN.md b/DESIGN.md index be3545a83..4c0d7bf11 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -64,5 +64,6 @@ Central manager only ever sends the boolean, never the other organizations' ids; - **Data fetching**: TanStack React Query 5. Query keys must be instance-scoped when the request targets a specific instance. - **Forms**: react-hook-form with a zod `zodResolver`. A form whose submit button waits on `formState.isValid` must not use the default `onSubmit` mode, which keeps `isValid` current but shows errors only after a submit the disabled button never allows, so the user is blocked with no reason given (HarperFast/studio#1550). Use `mode: 'onTouched'`: a field's error appears once it is first left and then follows each change, while `isValid` still tracks every keystroke (`onBlur` recomputes `isValid` only on blur, so the button lags the text). Enter neither submits past a disabled button nor leaves the field, so give the `
` `onKeyDown={revealFieldErrorOnEnter(form)}`, which treats Enter in a text input as leaving it. A refine that compares two fields reports on one of them; re-check that field from the other's `onChange` once it has been touched (`getFieldState(name).isTouched`, then `trigger(name)`), because `rules={{ deps }}` fires on the other field's first blur, before the user reaches it. react-hook-form only updates the `formState` fields a render has read, and `disabled={!isDirty || !isValid}` skips `isValid` while the form is pristine, so where one change can complete the form (a single field, a paste, a checkbox) the button stays disabled on a valid form. Read the fields before the JSX (`const { isDirty, isValid } = form.formState;`) and pin it with a test that makes one change and checks the button. - **Cloud permission gates** ([`usePermissions.ts`](src/hooks/usePermissions.ts)) must mirror the check the central manager endpoint runs, not the nearest role flag. An org admin appears in `/User/current` as `roles[orgId].role === 'admin'`; central manager builds every org role with `permission.super_user: false`, so the member helpers' `super_user` short-circuits never identify one — an admin gets full access there from its all-true flags. Container lifecycle ops are where the two diverge: their endpoints admit only org admins and staff holding `instance:update`, so gate them on `useContainerOpsPermission`, never on cluster `update`. +- **Never set `pnpm-workspace.yaml`'s `engineStrict`.** `engines.node` (`package.json`) stays advisory-only on `pnpm install`/`pnpm dev`/`pnpm build`; the Node-version guard is enforced only inside vitest, via [`scripts/check-node-version.mjs`](scripts/check-node-version.mjs) as `vitest.config.ts`'s `globalSetup`. `engineStrict` would also gate `pnpm install` and `pnpm run build:local`, which harper-pro's `build-tools/build-studio.sh` runs against studio's `prod` branch on Node 22 and `latest` (26.x) to produce the release/Docker bundle — that build breaks the moment this repo rejects those majors. - [`src/features/search/DESIGN.md`](src/features/search/DESIGN.md) — global search loading, shared-query ownership, session isolation, and shortcut behavior. diff --git a/README.md b/README.md index 037bb3ec9..59edbff5f 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ Please see .github/CONTRIBUTING.MD for detailed guidelines, including how to run ## Troubleshooting -- Dev server won’t start: ensure Node 20+ and pnpm installed; remove `node_modules` and reinstall. +- Dev server won’t start: ensure Node matches `.nvmrc` and pnpm is installed; remove `node_modules` and reinstall. - API calls failing in dev: verify `VITE_CENTRAL_MANAGER_API_URL` and any required auth are correct for your environment/mode. - Local Studio not showing: ensure your Harper process has `localStudio: { enabled: true }` and is listening on the port you expect (default 9925). diff --git a/package.json b/package.json index 857f14f56..eea4fe52d 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,9 @@ "private": true, "version": "0.0.0", "type": "module", + "engines": { + "node": ">=24 <25" + }, "scripts": { "dev": "npm run dev:fabric", "dev:fabric": "dotenv -e .env.local -- npm run vite-host", diff --git a/scripts/check-node-version.mjs b/scripts/check-node-version.mjs new file mode 100644 index 000000000..63d5d7b6d --- /dev/null +++ b/scripts/check-node-version.mjs @@ -0,0 +1,22 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +export function isSupported(range, version) { + const match = /^>=(\d+) <(\d+)$/.exec(range); + if (!match) { + throw new Error(`check-node-version can't parse engines.node "${range}" — update its parser`); + } + const [, low, high] = match; + const major = Number(version.split('.')[0]); + return major >= Number(low) && major < Number(high); +} + +export default function checkNodeVersion() { + const pkg = JSON.parse(readFileSync(fileURLToPath(new URL('../package.json', import.meta.url)), 'utf8')); + const range = pkg.engines?.node; + if (range && !isSupported(range, process.versions.node)) { + throw new Error( + `Unsupported Node.js version: running ${process.version}, studio requires node ${range} (see .nvmrc).`, + ); + } +} diff --git a/scripts/check-node-version.test.mjs b/scripts/check-node-version.test.mjs new file mode 100644 index 000000000..8c8bc50f1 --- /dev/null +++ b/scripts/check-node-version.test.mjs @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; +import { isSupported } from './check-node-version.mjs'; + +describe('isSupported', () => { + it('accepts a version inside the range', () => { + expect(isSupported('>=24 <25', '24.21.0')).toBe(true); + }); + + it('rejects a version above the range', () => { + expect(isSupported('>=24 <25', '26.2.0')).toBe(false); + }); + + it('rejects a version below the range', () => { + expect(isSupported('>=24 <25', '23.9.0')).toBe(false); + }); + + it('throws instead of silently accepting a range it cannot parse', () => { + expect(() => isSupported('^24.0.0', '22.0.0')).toThrow(); + }); + + it.each(['>23 <25', '>=24.5.0 <25', '<=24', '>=24 25'])( + 'throws on a partial match instead of evaluating it loosely: %s', + (range) => { + expect(() => isSupported(range, '22.0.0')).toThrow(); + }, + ); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 72f746401..86d752cad 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -9,6 +9,8 @@ export default defineConfig({ // per-file startup overhead. Isolation stays on (the default) because // several suites rely on vi.mock, which is unreliable without it. pool: 'threads', + // jsdom-based suites fail wholesale, with no hint why, above the Node major .nvmrc pins. + globalSetup: ['./scripts/check-node-version.mjs'], include: ['**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], exclude: [ '**/node_modules/**',