Skip to content

feat(embedded-web): send wrapper init metadata before host ready - #37

Open
Morten Barklund (barklund) wants to merge 11 commits into
mainfrom
add-analytics-attribution-to-corti-api-requests-dxa-3862
Open

feat(embedded-web): send wrapper init metadata before host ready#37
Morten Barklund (barklund) wants to merge 11 commits into
mainfrom
add-analytics-attribution-to-corti-api-requests-dxa-3862

Conversation

@barklund

Copy link
Copy Markdown
Member

Summary

Implements the Embedded Web side of DXA-3862 by sending wrapper metadata to Assistant via an automatic private _init handshake and gating readiness to ensure metadata is registered before host commands.

Changes

  • Adds internal _init protocol action for wrapper-owned initialization.
  • Sends _init automatically when embedded.ready is observed from the iframe.
  • Awaits _init completion before forwarding ready flow to host consumers.
  • Includes wrapper package name and generated wrapper version in _init payload.
  • Adds localhost HTTP allowance for local development base URL validation while keeping production host/protocol strictness.
  • Adds metadata generation script and publish flow wiring to keep wrapper version attribution accurate.
  • Updates demo and tests for the new initialization behavior.

Cross-repo context

Companion Assistant PR: https://github.com/corticph/assistant/pull/4230

These two PRs should be validated together locally using the same branch name in each repository.

Copilot AI lite review requested due to automatic review settings August 24, 2026 08:33
@linear-code

linear-code Bot commented Aug 24, 2026

Copy link
Copy Markdown

DXA-3862

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an internal initialization handshake to ensure the wrapper sends metadata to the embedded Assistant before consumers observe the embedded.ready flow, and relaxes baseURL validation to allow localhost HTTP for local development while keeping production URL strictness.

Changes:

  • Introduces a private _init action that is automatically sent when embedded.ready is received, and delays forwarding the ready event until initialization completes.
  • Adds generated wrapper package metadata (name + version) and wires generation into build/publish flows.
  • Updates baseURL validation, demo UX, and tests to cover the new initialization and localhost behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/post-message-handler.test.ts Adds coverage to ensure embedded.ready is not forwarded until the ready callback completes; covers _init request behavior.
test/corti-embedded.test.ts Verifies localhost HTTP baseURL support and that _init is sent before emitting embedded.ready to consumers.
src/utils/PostMessageHandler.ts Adds onReady callback support and awaits it before forwarding embedded.ready.
src/utils/baseUrl.ts Allows http://localhost (and loopback) for local dev while keeping production host/protocol validation strict.
src/types/protocol.ts Extends protocol types to include _init in request/action unions.
src/packageMetadata.ts Adds generated constants for wrapper package name/version used in _init payload.
src/CortiEmbedded.ts Sends _init (wrapper metadata) on iframe embedded.ready before dispatching ready to host consumers.
scripts/write-package-metadata.mjs Generates src/packageMetadata.ts from package.json name/version.
package.json Adds a metadata build script and wires it into the publish lifecycle.
demo/styles.css Styles auth payload inputs in the demo UI.
demo/index.html Updates demo markup and switches demo baseUrl to localhost for local development.
demo/demo.ts Enhances demo status reporting with message-bridge readiness indicators.
demo/demo.js Mirrors demo status reporting changes in the compiled JS.
.github/workflows/release.yml Adjusts release workflow build steps (but currently misses metadata generation in the publish build job).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/release.yml
Comment thread package.json Outdated
Comment thread src/types/protocol.ts

@hriczzoli Zoltan Hricz (hriczzoli) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! And I guess we'll wait for publishing this until Assistant is released, to make sure we don't break the ready event, right?

Comment thread package.json Outdated
barklund

This comment was marked as off-topic.

@barklund

Copy link
Copy Markdown
Member Author

Re publish timing: yes, we should publish this only after the Assistant side with _init support is released, so the ready flow cannot break against older Assistant deployments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Comment thread src/utils/baseUrl.ts Outdated
Comment thread src/utils/PostMessageHandler.ts Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/types/protocol.ts:92

  • InitRequest doesn't currently type its payload, even though _init is expected to carry wrapper metadata (web_component and web_component_version). Adding a concrete payload type here will improve type-safety for the internal handshake and prevent accidental shape drift.
export interface InitRequest extends EmbeddedRequest {
  action: "_init";
}

Comment thread src/utils/PostMessageHandler.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

test/post-message-handler.test.ts:314

  • This test forces (handler as any).isReady = true, which makes it not actually verify the new behavior that _init can be posted before public readiness is exposed. As written it would still pass even if _init required readiness. Consider asserting handler.ready is false instead, and remove the manual mutation.
    const { handler } = makeRealHandler();
    (handler as any).isReady = true;
    try {

src/utils/PostMessageHandler.ts:236

  • After destroy() is called, new waitForReady() callers will still wait until timeout because readyError is not set. Since the handler is permanently unusable after destroy(), set readyError so future readiness waits fail fast and deterministically.
   * Wait for the iframe to signal readiness via the 'embedded.ready' event.
   * @param timeout - Optional timeout in milliseconds (default: 30000ms)

Comment thread src/public-types.ts
Comment thread src/utils/PostMessageHandler.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/PostMessageHandler.ts:236

  • After destroy(), future waitForReady() calls will never be able to resolve (the message listener is removed) and will only time out because readyError is not set. Setting readyError here makes later waitForReady() calls reject immediately with a deterministic error and keeps behavior consistent with other terminal states (e.g. initialization failure).
    this.pendingRequests.clear();
    this.rejectReadyWaiters(new Error("PostMessageHandler destroyed"));

src/utils/PostMessageHandler.ts:297

  • The waitForReady() timeout error message still says "ready" even though the method now waits for both the embedded.ready signal and completion of the initialization callback. Updating the message will make debugging timeouts clearer.
      timeoutId = setTimeout(
        () =>
          waiter.reject(new Error("Timeout waiting for iframe to be ready")),
        timeout,
      );

Comment thread demo/index.html

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/CortiEmbedded.ts:183

  • onReady uses optional chaining when calling postMessageHandler.postMessage(...). If postMessageHandler is unexpectedly null (e.g., teardown races), the _init handshake will be silently skipped and readiness will still resolve, undermining the guarantee that metadata is registered before host consumers see embedded.ready. Consider failing initialization if the handler is missing so readiness remains gated.
        onReady: async () => {
          await this.postMessageHandler?.postMessage({
            type: "CORTI_EMBEDDED",
            version: "v1",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants