-
Notifications
You must be signed in to change notification settings - Fork 56
fix(hooks): skip db boot for dispatch — Mac CPU fix C #1476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -402,6 +402,34 @@ describe('retention cleanup', () => { | |||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('GENIE_SKIP_DB_BOOT (Mac CPU fix C — hook-dispatch coldstart)', () => { | ||||||
| test('runPostConnectSetup honors GENIE_SKIP_DB_BOOT alongside isTestMode', () => { | ||||||
| const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8'); | ||||||
| // The skipBoot guard must combine isTestMode + the env var so the | ||||||
| // hook-dispatch entrypoint can short-circuit migrations + seed | ||||||
| expect(source).toContain("process.env.GENIE_SKIP_DB_BOOT === '1'"); | ||||||
| expect(source).toMatch(/skipBoot\s*=\s*isTestMode\s*\|\|\s*process\.env\.GENIE_SKIP_DB_BOOT/); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regular expression used here is too broad. It matches the assignment prefix but does not verify the actual condition value (=== '1'). A change that incorrectly checks for a different value (e.g., === '0') would still pass this test. Consider tightening the regex to include the full comparison logic.
Suggested change
|
||||||
| // Both migrations and seed gated by skipBoot | ||||||
| expect(source).toMatch(/if \(!skipBoot\) await runMigrations/); | ||||||
| expect(source).toMatch(/if \(!skipBoot && needsSeed\(\)\) await runSeed/); | ||||||
| }); | ||||||
|
|
||||||
| test('hook dispatch entrypoint sets GENIE_SKIP_DB_BOOT before invoking dispatch()', () => { | ||||||
| const dispatchSource = readFileSync(join(__dirname, '..', 'hooks', 'dispatch-command.ts'), 'utf-8'); | ||||||
| // Env must be set inside dispatchAction (not at module load — that would | ||||||
| // affect the entire genie binary including the daemon path) | ||||||
| const dispatchActionIdx = dispatchSource.indexOf('async function dispatchAction'); | ||||||
| expect(dispatchActionIdx).toBeGreaterThan(-1); | ||||||
| const fnBody = dispatchSource.slice(dispatchActionIdx, dispatchSource.indexOf('}', dispatchActionIdx + 100)); | ||||||
| expect(fnBody).toContain("process.env.GENIE_SKIP_DB_BOOT = '1'"); | ||||||
| // Must be set BEFORE dispatch(stdin) so the first getConnection() inside | ||||||
| // any handler skips migrations + seed | ||||||
| const envSetIdx = dispatchSource.indexOf("GENIE_SKIP_DB_BOOT = '1'"); | ||||||
| const dispatchCallIdx = dispatchSource.indexOf('await dispatch(stdin)'); | ||||||
| expect(envSetIdx).toBeLessThan(dispatchCallIdx); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('pool error recovery', () => { | ||||||
| test('migration failure resets both sqlClient and activePort', () => { | ||||||
| const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8'); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting
process.env.GENIE_SKIP_DB_BOOT = '1'here changes the entire hook process environment, and hook handlers likeauto-spawnforwardprocess.envintospawnSync('genie', ...)child commands. That means non-hook subcommands (for examplegenie spawn) can inherit this flag and skiprunMigrations/runSeedinrunPostConnectSetup, which can break those commands on fresh installs or immediately after schema changes before bootstrapping has completed.Useful? React with 👍 / 👎.