Skip to content

fix(capabilities): require refresh after client updates - #4373

Merged
thetopham merged 4 commits into
stagingfrom
fix/capability-client-refresh-4372
Aug 1, 2026
Merged

thetopham merged 4 commits into
stagingfrom
fix/capability-client-refresh-4372

Conversation

@thetopham

@thetopham thetopham commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Closes #4372

Why this change

An already-open browser tab cannot replace a registered custom-element constructor when a downloaded capability package updates. Marinara imported the new module and reported it as ready, but continued rendering the old client implementation until the user happened to refresh the page. This made World Maps 1.2.3 appear to be missing its shared-world Game Setup action even though the installed and served artifact contained it.

What changed

  • Preserve the version of capability client code that is actually registered for the lifetime of the page, including across restart-required package states.
  • Detect a newer ready package when the previous version's custom element is still registered.
  • Serialize in-flight client imports per package/version so an older pending import cannot race a newly installed version.
  • When an older import settles after a newer version is observed, require refresh if its custom element registered; otherwise retry the latest installed client.
  • Block the stale interface and show a localized Refresh now state instead of reporting the new module as ready.
  • Add focused regression coverage for old, current, fresh-page, unregistered-element, and overlapping-import version combinations.

Validation

  • pnpm check
  • focused capability-client version/import race regression
  • pnpm localization:check
  • git diff --check

Manual verification

  • Update a client-capability package while its previous version is loaded and verify its surfaces show Refresh now rather than stale UI.
  • Refresh and verify the newly installed client UI loads without another refresh prompt.
  • Complete a restart-required client package update, restart Marinara Engine, and verify the same refresh state appears.
  • Verify a first-time package install loads normally without an unnecessary refresh prompt.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@thetopham, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ce572b1f-4d00-4822-b45b-4d3ce017421c

📥 Commits

Reviewing files that changed from the base of the PR and between aff14f8 and 3a13435.

📒 Files selected for processing (3)
  • packages/client/src/hooks/use-capability-packages.ts
  • packages/client/src/lib/capability-client-version.ts
  • scripts/regressions/capability-client-version-refresh.regression.ts
📝 Walkthrough

Walkthrough

The client now detects stale registered capability elements after package updates, marks affected modules as refresh-required, and renders localized refresh controls. Regression coverage validates version and registration combinations.

Changes

Capability client refresh

Layer / File(s) Summary
Version refresh detection and regression coverage
packages/client/src/lib/capability-client-version.ts, scripts/regressions/capability-client-version-refresh.regression.ts, package.json
Adds capabilityClientNeedsRefresh and regression coverage for stale and current registered versions, fresh pages, and unregistered elements.
Capability module refresh state
packages/client/src/hooks/use-capability-packages.ts
Retains loaded versions, detects stale registered custom elements, publishes refresh-required, and skips repeated loading.
Refresh-required UI and localization
packages/client/src/components/capabilities/CapabilityElement.tsx, packages/client/src/localization/locales/en.json
Adds localized toolbar and inline refresh notices with reload and close actions. CapabilityElement renders them for refresh-required modules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CapabilityModuleLoader
  participant CustomElementsRegistry
  participant CapabilityElement
  participant Browser
  CapabilityModuleLoader->>CustomElementsRegistry: Check registered custom element
  CapabilityModuleLoader->>CapabilityElement: Publish refresh-required status
  CapabilityElement->>Browser: Reload page on refresh action
Loading

Possibly related PRs

Suggested reviewers: spicymarinara

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #4372 by detecting stale registered clients, showing a refresh state, preserving restart behavior, and adding regression coverage.
Out of Scope Changes check ✅ Passed All changes support capability client refresh detection, user messaging, lifecycle handling, or focused regression coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely describes the main capability client refresh change.
Description check ✅ Passed The description explains the problem, changes, linked issue, validation commands, and manual verification steps.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/capability-client-refresh-4372

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

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thetopham
thetopham marked this pull request as ready for review July 31, 2026 23:29
@thetopham
thetopham requested a review from SpicyMarinara as a code owner July 31, 2026 23:29

@coderabbitai coderabbitai Bot 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/client/src/hooks/use-capability-packages.ts (1)

169-202: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Track in-flight client imports per package version.

If installed.data changes from v1 to v2 while a /client?v=${v1} import is pending, the current loop can accept v2 as ready after the v1 module registers marinara-capability-${item.id}. If the package later unregisters/re-registers or throws for the duplicate, v2 can end up stuck or failed despite the constructor already present. Keep per-package in-flight state so a later version completion that arrives after a tag is already registered publishes "refresh-required" instead of accepting the stale constructor.

🤖 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/client/src/hooks/use-capability-packages.ts` around lines 169 - 202,
Update the client-import flow around capabilityClientNeedsRefresh and
loadedClientModules to track in-flight imports per package version, rather than
treating a registered custom-element tag as sufficient for every later version.
When an older import completes after a newer installed version is observed and
its tag is already registered, publish "refresh-required" for the newer version
instead of marking it ready; preserve existing ready/error handling for imports
matching the tracked version and clear the in-flight state when each import
settles.
🤖 Prompt for all review comments with 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.

Outside diff comments:
In `@packages/client/src/hooks/use-capability-packages.ts`:
- Around line 169-202: Update the client-import flow around
capabilityClientNeedsRefresh and loadedClientModules to track in-flight imports
per package version, rather than treating a registered custom-element tag as
sufficient for every later version. When an older import completes after a newer
installed version is observed and its tag is already registered, publish
"refresh-required" for the newer version instead of marking it ready; preserve
existing ready/error handling for imports matching the tracked version and clear
the in-flight state when each import settles.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 50f07dbd-201a-4de1-ad29-d2ee72750999

📥 Commits

Reviewing files that changed from the base of the PR and between ed0e242 and aff14f8.

📒 Files selected for processing (6)
  • package.json
  • packages/client/src/components/capabilities/CapabilityElement.tsx
  • packages/client/src/hooks/use-capability-packages.ts
  • packages/client/src/lib/capability-client-version.ts
  • packages/client/src/localization/locales/en.json
  • scripts/regressions/capability-client-version-refresh.regression.ts

@thetopham

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thetopham
thetopham merged commit df92902 into staging Aug 1, 2026
13 checks passed
@thetopham
thetopham deleted the fix/capability-client-refresh-4372 branch August 1, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant