fix(hub-ui): initialize dock page scripts before activation

[dvcolomban]

Background (Why)

An iframe dock's page script can register commands used by notifications before the user opens its panel. The reference hub UI waits for activation, leaving those commands unavailable until then.

Changes (What)

Add optional eager: true to the existing ClientScriptEntry descriptor, defaulting to lazy initialization as discussed in review. Both the reference hub UI and the headless runtime wait for RPC trust before eager initialization. Activation shares pending setup, and failed setup remains retryable. Cache setup per RPC connection and import descriptor; action clicks still execute each time.

Keep the existing script fields: clientScript on iframe docks, action on action docks, and renderer on custom-render docks. The common dock base and JSON-render API are unchanged.

Verification (Testing)

53 focused tests pass across the headless runtime, renderer fixtures, and reference UI context/script suites. Coverage includes opt-in eager initialization, default lazy behavior, trust gating, separate RPC connections, concurrent activation and setup failure/retry. Scoped hub/hub-ui type checks and changed-file lint pass.

The rebuilt assets at 51d41004 were checked in the in-app Chromium browser before the subsequent trust-race correction. Embedded iframe setup remained lazy unless opted in; eager setup ran before panel activation, stayed deduplicated on activation, and registered a working page-local command. A standalone page initialized its own script instance and its command remained functional after closing the embedded page.

Commit 91291de9 adds six regression cases for trust revocation during dynamic import and while iframe setup completes. Both runtimes recheck trust before invocation/activation, and interrupted setup remains retryable after re-authentication. All 53 focused tests, changed-file lint and both package type checks pass. CI is running on this revision. The previous revision failed two Node 22 plugin-code-server declaration snapshots; that failure is retained pending the new run. The PR remains draft.

[coldtea-pr-lens]

◈ PR Lens

🟢 +0 new · 🟠 ~6 changed · 🔴 -0 removed · 2 flows · 9 files · commit b3ff169

Architecture

6 components touched across 3 lanes.

Open the interactive canvas

Inside the changed components — 2 views

Component view — Client script lifecycle & UI state

How Hub UI State and Headless Client Runtime coordinate eager and lazy script execution gated by RPC trust.

Component view — Framework host client script configuration

Host configurations registering eager action and page scripts alongside core type contracts.

Data flow

Eager script setup on RPC trust · Lazy script execution on dock activation

Open the interactive canvas

The other flows — 1 sequence

Lazy script execution on dock activation

View

• Architecture lens
• Data flow lens
• Expand every detail

Tip

Switch GitHub to dark mode and the diagrams follow. The moving dots are this pull request's data in motion.

🪧 More tips

• Run npx skills add coldteadotai/pr-lens, then tell your coding agent: "Diagram the change you just made with PR Lens and attach it to the pull request."
• Run npx @coldtea/pr-lens-cli analyze --base origin/main on a branch, then npx @coldtea/pr-lens-cli render .pr-lens/graph.json. Same lenses, your own model key, before the pull request exists.
• Untick Architecture lens or Data flow lens under View to hide a diagram, or tick Expand every detail to open every section. The comment redraws in a few seconds.
• Click the link under each diagram to open it on a canvas you can zoom, pan and step through.
• The diagrams are links. Click one to open it on the canvas, then press W or click play to walk through the change.
• Open a diagram on the canvas, then press W or click play to walk through the change one step at a time.
• The CLI's render reads .github/pr-lens.yml and applies your renames, exclusions and lane pins at draw time.
• Set github.comment.collapsed: true in .github/pr-lens.yml to fold the comment behind one View architecture and data flow row. Drawing still runs on every push.
• Add .github/workflows/pr-lens.yml with coldteadotai/pr-lens/packages/action@v0 and your model provider's key as its api-key to run PR Lens from your own CI. Any /chat/completions endpoint works.
• Push a commit and the comment redraws for the new head. A slow older run never overwrites a newer one.

---

Thanks for using PR Lens! It's built by Coldtea, free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

[vercel]

@dvcolomban is attempting to deploy a commit to the NuxtLabs Team on Vercel.

A member of the Team first needs to authorize it.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate issues affect runtime script initialization and activation behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates dock page-script initialization so trusted scripts load before activation, including JSON-render entries.

Changes:

• Moves clientScript to the common dock entry type.
• Adds per-RPC setup caching and retry behavior.
• Expands regression and integration coverage.

File summaries

File	Review findings
packages/hub/src/types/docks.ts	Critical (2 votes): The headless runtime can skip the base clientScript when an activation script exists.
packages/hub-ui/src/client/state/setup-script.ts	Critical (1 vote): The cache key does not distinguish page scripts from activation renderers.
packages/hub-ui/src/client/state/context.ts	Moderate (2 votes): Action entries with both action and clientScript skip the page script. Moderate (1 vote): Page setup is fire-and-forget and may complete after the panel becomes active.
packages/hub-ui/src/client/state/context.test.ts	No final review findings.
packages/hub-ui/src/client/state/client-script.integration.test.ts	No final review findings.
docs/content/8.references/6.hub-api.md	Nit (3 votes): The linked guide contradicts the new initialization contract and should cover JSON-render entries and trust/activation lifetime.

Review details

Suppressed comments (1)

packages/hub-ui/src/client/state/context.ts:254

• The setup is only started fire-and-forget here. A slow import or async initializer can therefore leave the panel visible while its page commands are still unregistered: switchEntry() publishes the selected/open state before awaiting its setup, and for an entry with a separate page clientScript plus renderer it awaits only the renderer setup. Track the page-setup promise and await it before exposing the active panel so this notification race is actually eliminated.

      void executeSetupScript(entry, scriptContext(entry), entry.clientScript).catch(() => {})

• Files reviewed: 6/6 changed files
• Comments generated: 4
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

Unresolved moderate issues remain in script lifecycle handling, runtime support, and documentation.

Review details

Suppressed comments (5)

docs/content/8.references/6.hub-api.md:134

• Please update the linked guide as well: docs/content/1.guide/17.client-context.md:70 still describes clientScript as an iframe-only field and omits the JSON-render/renderer-backed cases and the trust/activation lifetime added here. The new reference row otherwise contradicts the guide users are sent to.

| `iframe` and renderer-backed dock entries | `clientScript` (optional) | inside the host page, after trust and before dock activation |

packages/hub-ui/src/client/state/context.ts:251

• Because clientScript now lives on DevframeDockEntryBase, an action entry can declare both its activation action and an independent page-level clientScript. This condition skips every action entry before checking the common field, so that page script never initializes before activation even though the new field's contract says it does. Skip only the activation script (the explicit entry.clientScript passed below is still separate), or otherwise make this combination impossible in the type.

      if (entry.type === '~builtin' || entry.type === 'action' || !entry.clientScript)

packages/hub-ui/src/client/state/context.ts:243

• When a custom-render entry has both clientScript and renderer, an eager page-script failure is evicted from the cache, but this activation path calls only clientScriptOf(entry), which resolves to renderer. Opening the dock therefore never retries the failed page script despite the retry behavior promised by executeSetupScript; invoke the common page script separately before the renderer.

  async function runDockSetupScript(entry: DevframeDockEntry): Promise<void> {
    if (entry.type === '~builtin' || !clientScriptOf(entry))
      return
    await executeSetupScript(entry, scriptContext(entry))

packages/hub-ui/src/client/state/setup-script.ts:65

• The cache key does not encode the script's role. A custom-render entry may carry both a page-level clientScript and an activation-time renderer; if those descriptors point to the same module/export, the page initialization stores this key and activation returns that promise without invoking the renderer. Include the page-versus-activation role (or use separate caches) so these two lifecycles cannot collide.

  const key = JSON.stringify([entry.id, script?.importFrom, script?.importName ?? 'default'])

packages/hub/src/types/docks.ts:85

• clientScript is now permitted on every dock variant, including an entry that also has action or renderer, but the headless runtime still resolves only action ?? renderer ?? clientScript (packages/hub/src/client/host.ts:527). Such an entry therefore works in hub-ui as two scripts but silently loses its page script for createDevframeClientRuntime; load both declared scripts there or constrain the public contract.

  /** Page script initialized in the host page independently of dock activation or renderer type. */
  clientScript?: ClientScriptEntry

• Files reviewed: 6/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[antfu]

I wonder if we should have ClientScriptEntry as { importFrom, importName?, eager?: boolean }, which defaults to lazy (eager: false), for people to opt in to eager: true if they want to initialize the script immediately. WDYT?

[Copilot]

🟡 Changes recommended

Critical trust-revocation gaps and activation reliability issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

docs/content/8.references/6.hub-api.md:134

• This table is the API's timing contract, but the iframe row says the script runs on first activation without qualifying the new eager: true path, which runs it after trust before activation. Please make the row conditional so readers do not infer that eager page scripts are still lazy.

| `iframe` | `clientScript` (optional) | inside the host page on first activation |

packages/hub-ui/src/client/state/context.ts:311

• The same stale-activation race exists here: runPageScript(entry) can finish after a later iframe selection and then this call commits the older entry. Recheck the latest requested id/token after the await before setting selectedDockId and opening the panel.

    await runPageScript(entry)

packages/hub/src/client/host.ts:450

• Because this awaits page setup before committing selectedId, two rapid switchEntry() calls for different iframe docks can resolve out of order: the slower first setup will overwrite the selection made by the later click. Capture and validate an activation request (or serialize/cancel pending switches) after this await before mutating selection.

    if (entry?.type === 'iframe' && entry.clientScript && loadScriptsEnabled && !await preparePageScript(entry))
      return false

packages/hub/src/client/host.ts:610

• When trust is revoked during the dynamic import, this branch returns normally, so the promise stays cached as a successful setup. After trust is restored, loadClientScripts() reuses that resolved promise and never invokes the eager script, leaving its commands/subscriptions uninitialized for the new trusted session. Invalidate the cached attempt on trust loss (or make it reject/retry) before allowing re-authentication to proceed.

      if (!current || disposed || !rpc.isTrusted)
        return

packages/hub/src/client/host.ts:622

• Rethrowing here makes activation failures retryable, but the host's existing fire-and-forget callers (activateHandler and the selectedId setter) call void switchEntry(...) without a rejection handler. A lazy script failure now produces an unhandled promise rejection whenever activation is triggered through those paths; preserve the rejection for awaited callers but catch/log it at each fire-and-forget call site.

      throw error

• Files reviewed: 11/13 changed files
• Comments generated: 3
• Review effort level: Lite

[posva]

Isn't this implementing the same as #376 ?

[Copilot]

🔵 Needs a closer look

Unresolved lifecycle and error-handling issues remain around custom-render timing, stale descriptors, repeated action clicks, and unhandled rejections.

Review details

Suppressed comments (7)

docs/content/1.guide/17.client-context.md:70

• With eager: true, clientScriptOf includes renderer and startPageScripts() invokes it before the entry is selected, so the unconditional statement that a custom-render renderer initializes after selection is inaccurate. Qualify this as the default/lazy behavior and note that eager custom-render scripts cannot assume current.domElements.panel is mounted yet.

A client script is a `ClientScriptEntry`: `{ importFrom, importName?, eager? }`. `importName` defaults to `'default'` and `eager` defaults to `false`. An `iframe` entry's optional `clientScript` runs inside the host page when the dock entry is first activated. An `action` entry runs its `action` on each activation, while a `custom-render` entry initializes its `renderer` after selection so it can mount into the panel.

packages/hub-ui/src/client/state/context.ts:261

• The shared eager path also runs custom-render renderers before ViewCustomRenderer has mounted the panel, so current.domElements.panel is unavailable. Because activation reuses the cached setup promise, a renderer that mounts into that panel will not run again after selection. Keep custom-render execution activation-gated, or only preload its module eagerly and execute it once the panel exists.

      if (!clientScriptOf(entry)?.eager)
        continue
      /** Setup reports failures and allows the next activation or publication to retry. */
      void executeSetupScript(entry, scriptContext(entry), true).catch(() => {})

packages/hub-ui/src/client/state/setup-script.ts:40

• An entry can be updated or disposed while its eager import is pending, but this path only rechecks trust before invoking the function. The old descriptor can therefore run after a replacement has started its own setup (or after the dock was removed), leaving stale commands/subscriptions behind. Check that the current dock still carries this descriptor before calling fn, and do not cache a skipped setup.

    /** Trust may change while the module is loading; rejection keeps setup retryable. */
    if (!context.rpc.isTrusted)
      throw new Error('[@devframes/hub-ui] RPC client is no longer trusted')
    await fn(context)

packages/hub-ui/src/client/state/setup-script.ts:54

• Although this cache bypass makes direct executeSetupScript calls re-run action scripts, the reference UI's selected action button still routes a second click through DockEntries.toggleDockEntry, which calls switchEntry(null) instead of this activation path. The second click therefore closes the dock without executing action, contrary to the documented “action clicks execute on every activation” behavior and preventing an immediate retry after a failure. Make a selected action dispatch its id rather than toggling closed, and cover repeated clicks.

/** Cache setup per RPC connection and dock; explicit action clicks always run again. */

packages/hub/src/client/host.ts:610

• If a dock is updated or replaced while this import is pending, current only verifies that the id still exists, so the old descriptor can still invoke fn against the replacement state. Since the replacement gets a different cache key, both scripts may run and leave stale commands or subscriptions installed. Before invoking the function, verify that the current entry still carries this descriptor (and treat a skipped setup as non-cacheable).

      const current = entryToStateMap.get(entryId)
      if (!current || disposed)
        return

packages/hub/src/client/host.ts:565

• This eagerly executes a custom-render renderer before ViewCustomRenderer has mounted its panel, so current.domElements.panel is still unset. Activation then reuses the cached promise instead of running the renderer again, meaning a renderer that mounts into the panel never gets a usable container. Keep custom-render setup activation-gated, or separate eager module preloading from the panel-dependent execution.

      else if (entry.type === 'custom-render')
        startEagerScript(entry.id, entry.renderer)

packages/hub/src/client/host.ts:625

• Re-throwing here makes lazy import/setup failures reject switchEntry(), but the runtime's internal fire-and-forget paths (activateHandler and the selectedId setter) do not attach a catch. A failed script triggered through either path therefore becomes an unhandled rejection in addition to this logged error. Keep the rejection so failed cache entries can be retried, but handle it at those fire-and-forget call sites (and audit other internal activations).

      throw error

• Files reviewed: 11/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[dvcolomban]

@posva

Hello, indeed, the eager client-script part overlaps, I should have checked open PR before opening this one 😅

This PR is more targeted to just expanding script injection timing and the trusted race to solve a bug I hit in a downstream implementation.

Would it make sense to split the fixes and address the narrower fix first ?

If not we can close this one in favor of #376 indeed

[posva]

After running a small check, I think your PR seems to be better, it's also more focused, so I would say no, don't close this one in favor of #376 😄
I think I could base that other PR on this one once it gets merged

[posva]

I found only one bug not covered by the copilot review, I will send a PR to your branch

[posva]

Found a bug in the playgrounds.
Adding eager: true fix it:

• examples/custom-hub-vite/vite.config.ts:187

  action: { importFrom: 'demo-dock-client', eager: true },

• examples/custom-hub-next/src/client/devframe/next-devframe-hub.ts:299

  action: { importFrom: demoDockClient.importFrom, eager: true },

To verify you can run pnpm run --filter custom-hub-vite dev and click on Client Script Demo on the sidebar, it will now add messages (right now it doesn't)

[Copilot]

🔵 Needs a closer look

Unresolved activation race and descriptor-replacement issues remain, with missing eager action coverage.

Review details

Suppressed comments (5)

packages/hub-ui/src/client/state/context.ts:315

• After the awaited page setup, this path only rechecks trust. If the shared dock entry is replaced while its import is pending, dockEntryStateMap now points at the replacement but this activation still selects the old request and returns true; the old script can therefore activate against a changed descriptor. The headless preparePageScript already rejects when entryMeta !== entry; apply the same identity check here before mutating selection.

    if (!rpc.isTrusted)
      return false

packages/hub-ui/src/client/state/context.ts:315

• Because iframe setup is awaited before updating selectedDockId, overlapping switchEntry calls can commit out of order. A slow setup for dock A can finish after the user has selected dock B or closed the panel and restore A as the selected dock. Add a request-generation/serialization guard so only the latest activation may commit selection.

    if (!rpc.isTrusted)
      return false
    await runPageScript(entry)
    if (!rpc.isTrusted)
      return false

packages/hub-ui/src/client/state/context.ts:263

• The reference UI's eager action path is not covered: the integration suite tests eager iframes, lazy actions, and renderers, but never verifies eager action trust gating or the required per-click execution behavior. Add a regression case here because actions intentionally bypass the non-action setup cache.

      if (entry.type !== 'iframe' && entry.type !== 'action')
        continue
      if (!clientScriptOf(entry)?.eager)
        continue
      /** Setup reports failures and allows the next activation or publication to retry. */
      void executeSetupScript(entry, scriptContext(entry), true).catch(() => {})

packages/hub/src/client/host.ts:454

• switchEntry now waits for the iframe setup before committing selectedId, but concurrent calls are not serialized or versioned. If a slow setup for dock A is followed by a switch to dock B (or a close), B/null is committed first and A can resume here and overwrite the newer selection. Track the latest activation request (or cancel stale setup) before committing the selection.

    if (entry?.type === 'iframe' && entry.clientScript && loadScriptsEnabled && !await preparePageScript(entry))
      return false

packages/hub/src/client/host.ts:567

• The new eager action branch has no regression coverage in this runtime: the added tests cover eager iframes and activation-gated renderers, while the only action test is lazy. Add a test that an eager action initializes after trust and that repeated action activations still execute according to the documented per-click behavior; this path uses different cache semantics from iframes.

      else if (entry.type === 'action')
        startEagerScript(entry.id, entry.action)

• Files reviewed: 13/15 changed files
• Comments generated: 0 new
• Review effort level: Lite