Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion source/core/self-update-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export interface SelfUpdateResult {
export const CACHE_FILENAME = 'self-update.json';
export const TTL_MS = 24 * 60 * 60 * 1000;
export const FETCH_TIMEOUT_MS = 3000;
/** Node's `setTimeout` delay ceiling (2^31−1 ms); larger values overflow to ~1ms. */
const TIMER_MAX_MS = 2_147_483_647;
export const NPM_REGISTRY_URL = 'https://registry.npmjs.org/shardmind/latest';
const CACHE_SCHEMA_VERSION = 1 as const;
const SHARDMIND_DIRNAME = 'shardmind';
Expand Down Expand Up @@ -130,6 +132,29 @@ export function getSelfUpdateCacheDir(): string {
return path.join(os.homedir(), '.cache', SHARDMIND_DIRNAME);
}

/**
* Resolve the fetch timeout at call time. Production default is
* `FETCH_TIMEOUT_MS` (3s) — short, because a courtesy notifier must never make
* a real command feel slow. It can be raised via
* `SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS` for two cases: a user on a genuinely
* slow link, and Layer-1 flow tests where heavy parallel CPU load can starve
* the event loop past 3s before the (fast, local) stub response is processed —
* which would spuriously abort the fetch and the banner would never render.
* Mirrors the call-time env reads in `getRegistryUrl` / `getSelfUpdateCacheDir`.
* A non-numeric or non-positive value falls back to the default (never disables
* the timeout).
*/
export function getConfiguredFetchTimeoutMs(): number {
const raw = process.env['SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS'];
if (raw === undefined) return FETCH_TIMEOUT_MS;
const n = Number(raw.trim());
if (!Number.isFinite(n) || n <= 0) return FETCH_TIMEOUT_MS;
// Node's setTimeout delay is a 32-bit signed int; a value above that overflows
// to ~1ms (the OPPOSITE of a longer timeout — it would abort the fetch almost
// immediately). Truncate fractions and clamp to the timer ceiling.
return Math.min(Math.trunc(n), TIMER_MAX_MS);
}
Comment on lines +147 to +156

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 3dbc420. getConfiguredFetchTimeoutMs now Math.truncs and clamps to 2_147_483_647 (2^31−1), so a value above the setTimeout ceiling no longer overflows to ~1ms (which would have made a large value abort sooner, not later). Added a unit case covering the fractional-truncate and clamp paths.


function cachePath(cacheDir: string): string {
return path.join(cacheDir, CACHE_FILENAME);
}
Expand Down Expand Up @@ -253,7 +278,7 @@ export async function checkSelfUpdate(
currentVersion,
cacheDir = getSelfUpdateCacheDir(),
ttlMs = TTL_MS,
fetchTimeoutMs = FETCH_TIMEOUT_MS,
fetchTimeoutMs = getConfiguredFetchTimeoutMs(),
now = Date.now(),
signal,
} = opts;
Expand Down
12 changes: 10 additions & 2 deletions tests/component/flows/self-update-flow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ describe('self-update notifier — Layer 1 flow tests (#113)', () => {
'SHARDMIND_SELF_UPDATE_FORCE_TTY',
'SHARDMIND_SELF_UPDATE_REGISTRY_URL',
'SHARDMIND_SELF_UPDATE_CACHE_DIR',
'SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS',
'CI',
] as const;
let envSnapshot: Partial<Record<(typeof TOUCHED_ENV)[number], string | undefined>>;
Expand Down Expand Up @@ -165,6 +166,13 @@ describe('self-update notifier — Layer 1 flow tests (#113)', () => {
delete process.env['CI'];
process.env['SHARDMIND_SELF_UPDATE_FORCE_TTY'] = '1';
process.env['SHARDMIND_SELF_UPDATE_REGISTRY_URL'] = npmStub.url;
// The local stub answers in single-digit ms, but under heavy parallel CPU
// load the event loop can starve past the production 3s fetch timeout
// before the response is processed — aborting the fetch so the banner
// never renders and the banner-wait below times out. Give the fetch a wide
// budget here (above the 30s banner waitFor below, under the 60s test
// timeout); production keeps the 3s default. (Fixes the flaky scenario 7.)
process.env['SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS'] = '45000';
// Per-test cache dir keeps the dev's real ~/.cache/shardmind clean
// and avoids one test's cache hit suppressing the next test's fetch.
const cacheDir = path.join(cacheDirParent, crypto.randomUUID());
Expand Down Expand Up @@ -293,7 +301,7 @@ describe('self-update notifier — Layer 1 flow tests (#113)', () => {
f.includes(`shardmind ${NEWER_VERSION}`) &&
f.includes('npm install -g shardmind@latest') &&
/shardmind\/minimal/.test(f),
15_000,
30_000,
);
// Banner is above the status header (StatusView's first line is
// the namespace/name + version badge). String-position check
Expand Down Expand Up @@ -402,7 +410,7 @@ describe('self-update notifier — Layer 1 flow tests (#113)', () => {
await waitFor(
r.lastFrame,
(f) => f.includes(`shardmind ${NEWER_VERSION}`),
15_000,
30_000,
);
} finally {
if (vault) await vault.cleanup();
Expand Down
41 changes: 41 additions & 0 deletions tests/unit/self-update-check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import crypto from 'node:crypto';
import {
checkSelfUpdate,
getSelfUpdateCacheDir,
getConfiguredFetchTimeoutMs,
CACHE_FILENAME,
TTL_MS,
FETCH_TIMEOUT_MS,
} from '../../source/core/self-update-check.js';

function npmLatestResponse(version: string): Response {
Expand Down Expand Up @@ -614,4 +616,43 @@ describe('self-update-check', () => {
expect(result).toEqual({ outdated: true, latest: '1.0.0' });
});
});

describe('getConfiguredFetchTimeoutMs — SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS', () => {
const KEY = 'SHARDMIND_SELF_UPDATE_FETCH_TIMEOUT_MS';
let original: string | undefined;
beforeEach(() => {
original = process.env[KEY];
});
afterEach(() => {
if (original === undefined) delete process.env[KEY];
else process.env[KEY] = original;
});

it('defaults to FETCH_TIMEOUT_MS when unset', () => {
delete process.env[KEY];
expect(getConfiguredFetchTimeoutMs()).toBe(FETCH_TIMEOUT_MS);
});

it('honors a positive numeric override (trimmed)', () => {
process.env[KEY] = ' 45000 ';
expect(getConfiguredFetchTimeoutMs()).toBe(45000);
});

it('falls back to the default on non-numeric / non-positive / empty input', () => {
for (const bad of ['', ' ', 'abc', '0', '-5', 'NaN', 'Infinity']) {
process.env[KEY] = bad;
expect(getConfiguredFetchTimeoutMs()).toBe(FETCH_TIMEOUT_MS);
}
});

it('truncates fractions and clamps to the 32-bit setTimeout ceiling', () => {
// Fractional → truncated (setTimeout would truncate anyway).
process.env[KEY] = '45000.9';
expect(getConfiguredFetchTimeoutMs()).toBe(45000);
// Above 2^31−1 would overflow setTimeout to ~1ms; clamp to the ceiling
// so a fat-fingered huge value never makes the timeout *shorter*.
process.env[KEY] = '9999999999';
expect(getConfiguredFetchTimeoutMs()).toBe(2_147_483_647);
});
});
});
Loading