test(auth): cover the rollback safety nets in applyProviderInstallPlan + useAuth failure path

Addresses the missing-coverage points in the latest review pass: every deliberately-engineered rollback path in install.ts and the visible side effects of handleAuthFailure now have a regression test, so a future refactor that 'simplifies' these paths can't silently break them.

applyProviderInstallPlan (install.test.ts, +4 cases):
- restores runtime model providers when refreshAuth rejects after
  reloadModelProviders ran (asserts the second reloadModelProviders call
  receives the pre-install snapshot).
- still rolls back env vars when backup() throws before persist (pins
  the 'backup inside try' invariant added in 38a214d0ec).
- continues env rollback even when settings.restore itself throws
  (pins the nested try/catch around restore added in 38a214d0ec).
- continues throw + env rollback when the rollback-time
  reloadModelProviders itself throws (the original error must still
  surface; env vars must still revert).

useAuth (useAuth.test.ts, +1 case):
- surfaces install-plan rejection as an auth error and records
  telemetry — refreshAuth throws, the test asserts authError is set,
  the dialog reopens, isAuthenticating clears, no success toast is
  added, and pendingAuthType is populated (which is what the new
  setPendingAuthType call lets handleAuthFailure key the AuthEvent on).
- createSettings now mocks recomputeMerged + forScope.settings so the
  loaded-settings-adapter restore() path doesn't emit a noisy stderr.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
pomelo-nwu 2026-05-19 16:22:12 +08:00
parent cdc17cbba0
commit 7d8b4785ad
2 changed files with 167 additions and 0 deletions

View file

@ -45,8 +45,11 @@ const createSettings = () => ({
modelProviders: {},
},
setValue: vi.fn(),
recomputeMerged: vi.fn(),
forScope: vi.fn(() => ({
path: '/tmp/settings.json',
settings: {},
originalSettings: {},
})),
});
@ -235,6 +238,42 @@ describe('useAuthCommand', () => {
);
expect(config.refreshAuth).toHaveBeenCalledWith(AuthType.USE_OPENAI);
});
it('surfaces install-plan rejection as an auth error and records telemetry', async () => {
const settings = createSettings();
const config = createConfig();
config.refreshAuth = vi.fn(async () => {
throw new Error('refreshAuth rejected: bad endpoint');
});
const addItem = vi.fn();
const { result } = renderHook(() =>
useAuthCommand(settings as never, config as never, addItem),
);
await act(async () => {
await result.current.handleProviderSubmit(deepseekProvider, {
baseUrl: resolveBaseUrl(deepseekProvider),
apiKey: 'sk-bad',
modelIds: ['deepseek-v4-flash'],
});
});
// handleAuthFailure should have set the error, reopened the dialog, and
// cleared the in-flight flag. The success toast must NOT have fired.
expect(result.current.authError).toEqual(
expect.stringContaining('refreshAuth rejected'),
);
expect(result.current.isAuthDialogOpen).toBe(true);
expect(result.current.isAuthenticating).toBe(false);
expect(addItem).not.toHaveBeenCalled();
// pendingAuthType was set before applyProviderInstallPlan ran, so
// handleAuthFailure had it available — the AuthEvent path is no longer
// silently dropped on failure. (We can't assert the telemetry sink
// directly here, but the visible side effects above all depend on
// handleAuthFailure having seen pendingAuthType.)
expect(result.current.pendingAuthType).toBe(AuthType.USE_OPENAI);
});
});
describe('generateCustomApiKeyEnvKey', () => {

View file

@ -328,4 +328,132 @@ describe('applyProviderInstallPlan', () => {
expect(process.env['BRAND_NEW_KEY']).toBeUndefined();
});
// -- Rollback safety nets -------------------------------------------------
// The catch path in applyProviderInstallPlan has three deliberate
// safety nets that were previously untested. These tests pin them down so
// a future refactor that "simplifies" the catch can't silently regress.
it('restores runtime model providers when refreshAuth rejects after reloadModelProviders ran', async () => {
const previousProviders = {
[AuthType.USE_OPENAI]: [{ id: 'previous', envKey: 'OLD_KEY' }],
};
const adapter = createAdapter(previousProviders);
const reloadModelProviders = vi.fn();
const refreshAuth = vi.fn(async () => {
throw new Error('refreshAuth rejected');
});
const plan: ProviderInstallPlan = {
providerId: 'test-provider',
authType: AuthType.USE_OPENAI,
env: { TEST_API_KEY: 'sk-new' },
modelProviders: [
{
authType: AuthType.USE_OPENAI,
models: [{ id: 'new-model', envKey: 'TEST_API_KEY' }],
mergeStrategy: 'prepend-and-remove-owned',
ownsModel: (model) => model.envKey === 'TEST_API_KEY',
},
],
};
await expect(
applyProviderInstallPlan(plan, {
settings: adapter,
reloadModelProviders,
refreshAuth,
}),
).rejects.toThrow('refreshAuth rejected');
// Two reload calls: the success-path one with the patched providers,
// then a rollback one that hands back the snapshot we took *before*
// applying any patches.
expect(reloadModelProviders).toHaveBeenCalledTimes(2);
expect(reloadModelProviders).toHaveBeenLastCalledWith(previousProviders);
});
it('still rolls back env vars when backup() throws before persist', async () => {
process.env['TEST_API_KEY'] = 'old-value';
const adapter = createAdapter();
adapter.backup.mockImplementation(() => {
throw new Error('backup failed');
});
const plan: ProviderInstallPlan = {
providerId: 'test-provider',
authType: AuthType.USE_OPENAI,
env: { TEST_API_KEY: 'new-value' },
};
await expect(
applyProviderInstallPlan(plan, { settings: adapter }),
).rejects.toThrow('backup failed');
// backup() throwing inside the try must still hand control to the
// catch path so env vars are restored. (Before this commit's
// "backup inside try" fix the throw escaped uncaught and env vars
// leaked.)
expect(process.env['TEST_API_KEY']).toBe('old-value');
});
it('continues env rollback even when settings.restore itself throws', async () => {
process.env['TEST_API_KEY'] = 'before-install';
const adapter = createAdapter();
adapter.restore.mockImplementation(() => {
throw new Error('restore failed');
});
const refreshAuth = vi.fn(async () => {
throw new Error('original error');
});
const plan: ProviderInstallPlan = {
providerId: 'test-provider',
authType: AuthType.USE_OPENAI,
env: { TEST_API_KEY: 'during-install' },
};
await expect(
applyProviderInstallPlan(plan, { settings: adapter, refreshAuth }),
).rejects.toThrow('original error');
// restore() throwing must not mask the original error and must not skip
// the env-var rollback loop that runs after it.
expect(adapter.restore).toHaveBeenCalled();
expect(process.env['TEST_API_KEY']).toBe('before-install');
});
it('continues throw + env rollback when reloadModelProviders rollback itself throws', async () => {
process.env['TEST_API_KEY'] = 'before';
const previousProviders = {
[AuthType.USE_OPENAI]: [{ id: 'previous', envKey: 'OLD' }],
};
const adapter = createAdapter(previousProviders);
let reloadCalls = 0;
const reloadModelProviders = vi.fn(() => {
reloadCalls += 1;
if (reloadCalls === 2) {
// The rollback-time reload (the second call) explodes.
throw new Error('reload restore failed');
}
});
const refreshAuth = vi.fn(async () => {
throw new Error('original error');
});
const plan: ProviderInstallPlan = {
providerId: 'test-provider',
authType: AuthType.USE_OPENAI,
env: { TEST_API_KEY: 'during' },
};
await expect(
applyProviderInstallPlan(plan, {
settings: adapter,
reloadModelProviders,
refreshAuth,
}),
).rejects.toThrow('original error');
// The rethrow must still carry the original error, env vars must still
// be rolled back, and the broken rollback reload must not mask anything.
expect(reloadModelProviders).toHaveBeenCalledTimes(2);
expect(process.env['TEST_API_KEY']).toBe('before');
});
});