Skip to content
Open
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
14 changes: 7 additions & 7 deletions packages/davinci-client/api-report/davinci-client.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
resume: (input: {
continueToken: string;
}) => Promise<InternalErrorResponse | NodeStates>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
poll: (collector: PollingCollector) => Poller;
pollStatus: (collector: PollingCollector) => Poller;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Public API rename is breaking without a compatibility bridge.

Renaming poll to pollStatus changes the exported client contract and will break existing integrations unless you provide a transitional alias (or explicitly gate this behind a major-version release plan).

Suggested compatibility bridge (in source, then regenerate API report)
- pollStatus: (collector: PollingCollector) => Poller;
+ pollStatus: (collector: PollingCollector) => Poller;
+ /** `@deprecated` Use pollStatus instead. */
+ poll: (collector: PollingCollector) => Poller;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/davinci-client/api-report/davinci-client.api.md` at line 273, The
public API renamed the method from poll to pollStatus (seen as pollStatus:
(collector: PollingCollector) => Poller), which is breaking; restore backwards
compatibility by adding a transitional alias named poll that delegates to
pollStatus (e.g., implement a poll method or export a poll symbol that
calls/returns pollStatus(collector)), keep both signatures matching
PollingCollector => Poller, mark the alias as deprecated in source comments, and
then regenerate the API report so the exported contract includes both poll and
pollStatus.

getClient: () => {
status: "start";
} | {
action: string;
collectors: Collectors[];
description?: string;
Expand All @@ -287,6 +285,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
status: "error";
} | {
status: "failure";
} | {
status: "start";
} | {
authorization?: {
code?: string;
Expand All @@ -297,7 +297,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
getCollectors: () => Collectors[];
getError: () => DaVinciError | null;
getErrorCollectors: () => CollectorErrors[];
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
getServer: () => {
_links?: Links;
id?: string;
Expand All @@ -306,8 +306,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
href?: string;
eventName?: string;
status: "continue";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand All @@ -323,6 +321,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
interactionId?: string;
interactionToken?: string;
status: "failure";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand Down
14 changes: 7 additions & 7 deletions packages/davinci-client/api-report/davinci-client.types.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,11 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
resume: (input: {
continueToken: string;
}) => Promise<InternalErrorResponse | NodeStates>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode>;
start: <QueryParams extends OutgoingQueryParams = OutgoingQueryParams>(options?: StartOptions<QueryParams> | undefined) => Promise<ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode>;
update: <T extends SingleValueCollectors | MultiSelectCollector | ObjectValueCollectors | AutoCollectors>(collector: T) => Updater<T>;
validate: (collector: SingleValueCollectors | ObjectValueCollectors | MultiValueCollectors | AutoCollectors) => Validator;
poll: (collector: PollingCollector) => Poller;
pollStatus: (collector: PollingCollector) => Poller;
getClient: () => {
status: "start";
} | {
action: string;
collectors: Collectors[];
description?: string;
Expand All @@ -287,6 +285,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
status: "error";
} | {
status: "failure";
} | {
status: "start";
} | {
authorization?: {
code?: string;
Expand All @@ -297,7 +297,7 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
getCollectors: () => Collectors[];
getError: () => DaVinciError | null;
getErrorCollectors: () => CollectorErrors[];
getNode: () => ContinueNode | StartNode | ErrorNode | FailureNode | SuccessNode;
getNode: () => ContinueNode | ErrorNode | FailureNode | StartNode | SuccessNode;
getServer: () => {
_links?: Links;
id?: string;
Expand All @@ -306,8 +306,6 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
href?: string;
eventName?: string;
status: "continue";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand All @@ -323,6 +321,8 @@ export function davinci<ActionType extends ActionTypes = ActionTypes>(input: {
interactionId?: string;
interactionToken?: string;
status: "failure";
} | {
status: "start";
} | {
_links?: Links;
eventName?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
*
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { iFrameManager } from './iframe-manager.effects.js';

/**
* Patches an iframe's contentWindow.location.href to simulate navigation,
* then fires a 'load' event so the onLoadHandler runs.
*/
function simulateIframeLoad(iframe: HTMLIFrameElement, href: string): void {
Object.defineProperty(iframe, 'contentWindow', {
value: { location: { href } },
writable: true,
configurable: true,
});
iframe.dispatchEvent(new Event('load'));
}

describe('iFrameManager', () => {
describe('getParamsByRedirect – input validation', () => {
it('throws synchronously when successParams is empty', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
successParams: [],
errorParams: ['error'],
}),
).toThrow('successParams and errorParams must be provided');
});

it('throws synchronously when errorParams is empty', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
successParams: ['code'],
errorParams: [],
}),
).toThrow('successParams and errorParams must be provided');
});

it('throws synchronously when successParams or errorParams is undefined', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
successParams: undefined as any,
errorParams: ['error'],
}),
).toThrow('successParams and errorParams must be provided');
});
Comment on lines +50 to +61
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Test name is misleading and the errorParams: undefined case is missing.

The description says "successParams or errorParams is undefined" but the test body only exercises successParams: undefined. errorParams: undefined is never passed, so the guard for that branch is untested.

🛠️ Proposed fix: cover both `undefined` cases
-    it('throws synchronously when successParams or errorParams is undefined', () => {
-      const manager = iFrameManager();
-      expect(() =>
-        manager.getParamsByRedirect({
-          url: 'https://example.com',
-          timeout: 1000,
-          // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
-          successParams: undefined as any,
-          errorParams: ['error'],
-        }),
-      ).toThrow('successParams and errorParams must be provided');
-    });
+    it('throws synchronously when successParams is undefined', () => {
+      const manager = iFrameManager();
+      expect(() =>
+        manager.getParamsByRedirect({
+          url: 'https://example.com',
+          timeout: 1000,
+          // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+          successParams: undefined as any,
+          errorParams: ['error'],
+        }),
+      ).toThrow('successParams and errorParams must be provided');
+    });
+
+    it('throws synchronously when errorParams is undefined', () => {
+      const manager = iFrameManager();
+      expect(() =>
+        manager.getParamsByRedirect({
+          url: 'https://example.com',
+          timeout: 1000,
+          successParams: ['code'],
+          // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+          errorParams: undefined as any,
+        }),
+      ).toThrow('successParams and errorParams must be provided');
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('throws synchronously when successParams or errorParams is undefined', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
successParams: undefined as any,
errorParams: ['error'],
}),
).toThrow('successParams and errorParams must be provided');
});
it('throws synchronously when successParams is undefined', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
successParams: undefined as any,
errorParams: ['error'],
}),
).toThrow('successParams and errorParams must be provided');
});
it('throws synchronously when errorParams is undefined', () => {
const manager = iFrameManager();
expect(() =>
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 1000,
successParams: ['code'],
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
errorParams: undefined as any,
}),
).toThrow('successParams and errorParams must be provided');
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk-effects/iframe-manager/src/lib/iframe-manager.effects.test.ts`
around lines 50 - 61, The test "throws synchronously when successParams or
errorParams is undefined" only exercises the successParams-undefined branch;
update the tests for iFrameManager().getParamsByRedirect to either (a) split
into two assertions or add a second it block: one that passes successParams:
undefined and a second that passes errorParams: undefined (both with other
required fields like url/timeout) and assert they throw 'successParams and
errorParams must be provided'; alternatively rename the existing test to
accurately reflect it only covers successParams if you prefer separate coverage.
Use the manager variable and getParamsByRedirect symbol to locate the test to
modify.

});

describe('getParamsByRedirect – iframe lifecycle', () => {
beforeEach(() => {
vi.useFakeTimers();
});

afterEach(() => {
vi.useRealTimers();
document.body.replaceChildren();
});

it('creates a hidden iframe with display:none and appends it to document.body', () => {
const manager = iFrameManager();
manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe');
expect(iframe).not.toBeNull();
expect(iframe?.style.display).toBe('none');
});

it('sets iframe.src to the provided URL', () => {
const manager = iFrameManager();
manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;
expect(iframe.src).toBe('https://example.com/start');
});

it('rejects with timeout error when iframe never resolves', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 3000,
successParams: ['code'],
errorParams: ['error'],
});

vi.advanceTimersByTime(3000);

await expect(promise).rejects.toEqual({
type: 'internal_error',
message: 'iframe timed out',
});
});

it('removes the iframe from the DOM after timeout', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com',
timeout: 3000,
successParams: ['code'],
errorParams: ['error'],
});

vi.advanceTimersByTime(3000);
await promise.catch(vi.fn());

expect(document.querySelector('iframe')).toBeNull();
});

it('resolves with all query params when any successParam key is present in the redirect URL', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;
simulateIframeLoad(iframe, 'https://app.example.com/callback?code=abc123&state=xyz');

const result = await promise;
expect(result).toEqual({ code: 'abc123', state: 'xyz' });
});

it('removes the iframe from the DOM after resolving with success params', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;
simulateIframeLoad(iframe, 'https://app.example.com/callback?code=abc123');

await promise;
expect(document.querySelector('iframe')).toBeNull();
});

it('resolves (not rejects) with all query params when an errorParam key is present', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error', 'error_description'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;
simulateIframeLoad(
iframe,
'https://app.example.com/callback?error=access_denied&error_description=User+cancelled',
);

const result = await promise;
expect(result).toEqual({
error: 'access_denied',
error_description: 'User cancelled',
});
});

it('ignores the initial about:blank load event and keeps waiting', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;

simulateIframeLoad(iframe, 'about:blank');
vi.advanceTimersByTime(100);

simulateIframeLoad(iframe, 'https://app.example.com/callback?code=abc123');

const result = await promise;
expect(result).toEqual({ code: 'abc123' });
});

it('waits through intermediate redirects before resolving', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;

simulateIframeLoad(iframe, 'https://example.com/authorize');
simulateIframeLoad(iframe, 'https://app.example.com/callback?code=final');

const result = await promise;
expect(result).toEqual({ code: 'final' });
});

it('rejects with internal_error when contentWindow access throws (cross-origin)', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;

Object.defineProperty(iframe, 'contentWindow', {
get() {
throw new DOMException('Blocked a frame with origin', 'SecurityError');
},
configurable: true,
});

iframe.dispatchEvent(new Event('load'));

await expect(promise).rejects.toEqual({
type: 'internal_error',
message: 'unexpected failure',
});
});

it('removes the iframe from the DOM after cross-origin rejection', async () => {
const manager = iFrameManager();
const promise = manager.getParamsByRedirect({
url: 'https://example.com/start',
timeout: 5000,
successParams: ['code'],
errorParams: ['error'],
});

const iframe = document.querySelector('iframe') as HTMLIFrameElement;

Object.defineProperty(iframe, 'contentWindow', {
get() {
throw new DOMException('Blocked a frame with origin', 'SecurityError');
},
configurable: true,
});

iframe.dispatchEvent(new Event('load'));
await promise.catch(vi.fn());

expect(document.querySelector('iframe')).toBeNull();
});
});
});
Binary file modified packages/sdk-effects/iframe-manager/vite.config.ts
Binary file not shown.
Loading