mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-23 12:44:02 +00:00
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 in38a214d0ec). - continues env rollback even when settings.restore itself throws (pins the nested try/catch around restore added in38a214d0ec). - 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:
parent
cdc17cbba0
commit
7d8b4785ad
2 changed files with 167 additions and 0 deletions
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue