Skip to content

fix: OpenClaw 2026.5.12 chat protocol + Free→Paid tier auto-detect + tier-change UX + codex plugin auto-install - #132

Merged
yalexx merged 4 commits into
ID-Robots:betafrom
KrasimirKralev:fix/openclaw-2026-5-12-tier-ux
May 16, 2026
Merged

yalexx merged 4 commits into
ID-Robots:betafrom
KrasimirKralev:fix/openclaw-2026-5-12-tier-ux

Conversation

@KrasimirKralev

@KrasimirKralev KrasimirKralev commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Four related fixes that unblock the in-page chat and tighten the
tier-state UX after upgrading to OpenClaw 2026.5.12.

1. Chat protocol v3 → v4 (fix)

OpenClaw 2026.5.12 dropped support for the v3 gateway protocol and
rejects v3 connect frames with code=1002 reason=protocol mismatch.
The in-page chat panel was sending minProtocol: 3, maxProtocol: 3
and rendered "Could not connect to gateway" in the UI. Bumped both
chat surfaces to v4.

  • src/components/ChatApp.tsx
  • src/components/ChatPopup.tsx

2. Free → Paid portal upgrade auto-detection (fix)

/setup-api/ai-models/status was short-circuiting the portal call
whenever the local picker tier was null, so a Free user who
upgraded on the portal didn't see the new tier until they re-paired
ClawBox AI. The original guard's intent — preventing a portal-side
deviceTier="flash" bug from promoting a Free user — is now
enforced inside mapPortalTier, which requires a paid subscription
tier before honouring any deviceTier stamp. The guard on the
call-site is removed, so the next 30 s poll (worst case ~2.5 min
after the portal's own 60 s reconcile + 120 s device-side cache TTL)
flips the device without a re-login.

  • src/app/setup-api/ai-models/status/route.ts
  • src/tests/routes/ai-models/status.test.ts — 1 test replaced
    (skip-portal-when-null no longer applies), 2 new tests added
    (Free→Pro upgrade detection, bogus deviceTier defence)

3. Tier-change announcement modal (feat)

A new TierUpgradeCelebration modal mounted on the desktop root.
Watches useClawboxLogin() and announces transitions:

  • Free → Pro: "Welcome to ClawBox Pro 🎉"
  • Pro → Max / Free → Max: "Welcome to ClawBox Max 🎉"
  • Any paid → Free: "You're on the Free plan now" with a
    Re-subscribe CTA
  • Max → Pro or no-change ticks: silent

Each transition is announced once via a clawai_tier_seen flag
stored through client-kv, so a poll re-reporting the same tier
doesn't re-open the dialog. A real cancel + resubscribe flow does
re-celebrate, because the downgrade resets the flag.

  • src/components/TierUpgradeCelebration.tsx (new)
  • src/app/page.tsx (mount, gated behind the existing kv.init())
  • 12 new translation keys × 10 locales in desktop-translations*.ts

4. Auto-install @openclaw/codex when a codex model is configured (fix)

OpenClaw 2026.5.12 also split the codex agent harness out of the
core gateway into a separate npm package (`@openclaw/codex`) and
only auto-installs it on the `openclaw onboard --auth-choice
openai-codex…` path. Because our `configure/route.ts` writes
`openclaw.json` directly (see the schema-drift note already in
that file), devices that pick a Codex model in the chat picker
never trigger the install and every reply fails with:

`Embedded agent failed before reply: Requested agent harness "codex" is not registered.`

Fix lives in `scripts/gateway-pre-start.sh` — single chokepoint,
runs on every gateway start, self-heals existing devices on update.
Detection mirrors OpenClaw's own `modelSelectionShouldEnsureCodexPlugin`
(checks the primary model + every auth profile for the
`openai-codex` provider). Idempotent — skips the install when
`@openclaw/codex/package.json` is already on disk.

How was this tested?

All four changes verified live on a Jetson dev install before
opening this PR:

  • Protocol fix: chat reconnected after rebuild; gateway logs
    go from [ws] protocol mismatch code=1002 to [ws] webchat connected.
  • Tier auto-detect: real Stripe upgrade Free → Pro detected
    on the next status poll, no re-login. Real downgrade Pro → Free
    also detected.
  • Celebration modal: welcome-to-Pro fired automatically on the
    open chat tab, and the Free-downgrade modal fired after the real
    cancel.
  • Codex plugin auto-install: `openclaw plugins uninstall codex
    --force` followed by `systemctl restart clawbox-gateway` —
    pre-start detected the codex model + missing plugin → installed →
    gateway came up with `codex` in its loaded plugins list.

Checklist

  • Manually verified on Jetson dev install
  • bun run lint (will run in CI)
  • bun run test (will run in CI)
  • bun run build (will run in CI)

Notes for the reviewer

  • The dropped `localTier !== null` guard in (2) was deliberately
    added in feat(ai-models): auto-tier wizard + Free-tier UX gates #122 with the comment "Drop this guard once the portal
    gates deviceTier by subscription." `mapPortalTier` now provides
    that gating from the device side regardless of the portal's
    contract, so we can drop the guard safely.
  • The codex auto-install in (4) uses a heredoc-based Python detector
    matching the rest of `gateway-pre-start.sh`'s style. Open to
    switching to `openclaw plugins inspect codex` if you'd rather
    go through the CLI than read the JSON directly.
  • The celebration component is hardcoded to the two known tiers
    (flash/pro) plus Free. If OpenClaw introduces an "Ultra" tier or
    similar later, both the rank table and the CONTENT lookup will
    need extending — trivially, but worth flagging.
  • The Free-downgrade case uses a softer "muted" tone instead of the
    fuchsia gradient used for upgrades; happy to swap to celebratory
    styling if you'd rather the entire flow feel positive.

Summary by CodeRabbit

  • New Features

    • Added tier upgrade/downgrade celebration modals with localized messaging across many languages
    • Updated client gateway protocol negotiation to version 4
    • Improved gateway plugin auto-detection and installation handling
  • Bug Fixes

    • Strengthened subscription-tier validation to avoid stale data misclassifying accounts
  • Tests

    • Updated tier-resolution tests to match the new validation behavior

Review Change Stack

…upgrade UX

- ChatApp/ChatPopup: bump WebSocket connect handshake to protocol v4
  (2026.5.12 rejects v3 with code=1002), unbreaks the in-page chat
- status/route.ts: remove the localTier !== null guard so Free→Paid
  portal upgrades are visible without re-login; mapPortalTier now
  gates non-null returns on a paid subscription plan, defending the
  stale-deviceTier case the old guard worked around
- TierUpgradeCelebration: announce upgrade (Free→Pro→Max) once each
  via client-kv flag, plus a Free-plan downgrade notice with a
  Re-subscribe CTA; mounted in page.tsx
- desktop-translations*: 12 new keys × 10 locales
- status.test.ts: replace 1 test (skip-portal-when-null) with 3 new
  ones (Free portal query, Free→Pro upgrade detection, bogus
  deviceTier defence)
…el is configured

OpenClaw 2026.5.12 split the codex agent harness out of the core
gateway into a separate npm package (`@openclaw/codex`) and only
auto-installs it during `openclaw onboard --auth-choice openai-codex…`.
ClawBox's configure route writes `openclaw.json` directly (see the
schema-drift note in `src/app/setup-api/ai-models/configure/route.ts`),
so devices that pick a Codex model in the chat picker never trigger
the install and every chat attempt fails with:

  Embedded agent failed before reply: Requested agent harness
  "codex" is not registered.

This is the exact failure surfaced in #(this PR's issue): a device on
2026.5.12 with `agents.defaults.model.primary = "openai-codex/gpt-5.5"`
errors on the first reply.

Fix in `scripts/gateway-pre-start.sh` because it's the single
chokepoint that runs on every gateway start — self-healing for
existing devices on upgrade, fresh installs, and manual config edits.
Detection mirrors OpenClaw's own `modelSelectionShouldEnsureCodexPlugin`
(provider === "openai-codex" on the primary model OR any auth profile).
Idempotent: skips the install when `@openclaw/codex/package.json` is
already on disk.

Verified on Jetson:
  1. `openclaw plugins uninstall codex --force`
  2. `systemctl restart clawbox-gateway`
  3. Pre-start detects codex model + missing plugin → installs.
  4. Gateway comes up with `codex` in its loaded plugins list.
@KrasimirKralev
KrasimirKralev requested a review from a team as a code owner May 16, 2026 10:22
@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@yalexx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 33 minutes and 42 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 88e88254-d884-448a-aea4-5589ce8cf000

📥 Commits

Reviewing files that changed from the base of the PR and between 7368049 and 8236958.

📒 Files selected for processing (2)
  • .github/workflows/e2e-install.yml
  • scripts/e2e-coverage-report.mjs
📝 Walkthrough

Walkthrough

This PR introduces a tier upgrade/downgrade celebration modal UI, refines portal tier resolution logic to always query when a token exists, upgrades WebSocket protocol to v4 across client connections, and adds automatic Codex plugin detection and installation to the gateway startup script.

Changes

Tier Upgrade Celebration UI

Layer / File(s) Summary
TierUpgradeCelebration component logic and shell
src/components/TierUpgradeCelebration.tsx
New React component detects tier rank changes using a client-kv "seen" marker and renders upgrade dialogs for rank increases, downgrade-to-Free modals when dropping from paid to free, and silent sync otherwise. CelebrationShell handles overlay styling, accessibility, click propagation, and configurable action buttons.
Tier celebration translations across locales
src/lib/desktop-translations.ts, src/lib/desktop-translations-part1.ts, src/lib/desktop-translations-part2.ts, src/lib/desktop-translations-part3.ts
tierCelebration.* keys (badge, headline, body, CTA, resubscribe text) added for Pro, Max, and Free tiers across English, German, Bulgarian, Spanish, French, Italian, Japanese, Dutch, Swedish, and Chinese locales.
TierUpgradeCelebration page integration
src/app/page.tsx
Component imported and rendered in the desktop root JSX alongside other overlays.

Portal Tier Resolution Logic

Layer / File(s) Summary
mapPortalTier validation and portal lookup timing
src/app/setup-api/ai-models/status/route.ts
mapPortalTier now validates plan type (pro/max) before considering deviceTier stamps, returning null for non-paid plans. GET handler portal lookup now always runs when clawaiToken is present, removing the prior Free-path skip and instead relying on mapPortalTier's plan gating to prevent stale deviceTier from promoting free users.
Tier resolution test updates
src/tests/routes/ai-models/status.test.ts
Tests updated to assert portal queries run even with unset local picker, Free portal responses return null tiers, Free→Pro upgrades are detected, and stale deviceTier values do not promote Free-tier users.

WebSocket Protocol v4 Upgrade

Layer / File(s) Summary
WebSocket protocol version 4 negotiation
src/components/ChatApp.tsx, src/components/ChatPopup.tsx
Gateway WebSocket connect requests updated from minProtocol/maxProtocol 3 to 4.

Gateway Codex Plugin Auto-Installation

Layer / File(s) Summary
Codex provider detection and idempotent installation
scripts/gateway-pre-start.sh
Pre-start script detects openai-codex provider in openclaw.json (primary agent model or auth profiles) via inline Python JSON parse; if detected and plugin package.json is missing, runs openclaw plugins install codex with warnings on failure instead of aborting.

Playwright and tests

Layer / File(s) Summary
Playwright port/baseURL and webServer command
playwright.config.ts
Compute dev server port from env vars and set baseURL/webServer.url to the computed value; run dev server with PORT set to that port.
Translations test allowlist update
src/tests/unit/translations.test.ts
Add tierCelebration to knownPrefixes in translation key naming conventions test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • ID-Robots/clawbox#119: Both PRs modify src/app/setup-api/ai-models/status/route.ts and its /setup-api/ai-models/status portal/tier-resolution logic.
  • ID-Robots/clawbox#124: Overlaps on portal-backed tier entitlements and lookup timing in src/app/setup-api/ai-models/status/route.ts.
  • ID-Robots/clawbox#122: Related changes to clawai tier-resolution and portal lookup behavior in the same route.

Suggested labels

run-full-e2e

Suggested reviewers

  • yalexx

Poem

🐇 I hop with joy at tiered delight,
Modals shine in morning light,
Codex checks and protocols four,
Celebrate upgrades—hop some more! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title comprehensively and accurately captures all four main changes: chat protocol upgrade, tier auto-detection, tier-change UX, and codex plugin auto-install.
Description check ✅ Passed The description is comprehensive, well-structured, and covers all required template sections with detailed explanations, testing methodology, and reviewer notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
src/tests/routes/ai-models/status.test.ts (1)

382-406: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Stale test contradicts the new portal lookup behavior.

This test expects fetchSpy.not.toHaveBeenCalled() when a clawaiToken is present with localTier=null. However, the route change at line 258 now queries the portal whenever clawaiToken exists, regardless of localTier. The test at lines 180-201 confirms this new behavior.

The comment on lines 383-385 referencing "defence-in-depth keeps the portal call skipped" describes the old behavior that this PR removes.

Proposed fix
     it("returns clawaiAccountTier=null but clawaiConfigured=true for a Free user chatting via OpenAI", async () => {
-      // Free user with a paired clawai token but no paid local picker
-      // → defence-in-depth keeps the portal call skipped, so
-      // clawaiAccountTier stays null. clawaiConfigured is true so the
-      // hook reports loggedIn=true (Free users have a paired account).
+      // Free user with a paired clawai token — portal is queried and
+      // returns Free, so clawaiAccountTier stays null. clawaiConfigured
+      // is true so the hook reports loggedIn=true (Free users have a
+      // paired account).
       mockReadConfig.mockResolvedValue({
         auth: {
           profiles: {
             "openai:default": { provider: "openai", mode: "token" },
             "deepseek:default": { provider: "deepseek", mode: "api_key" },
           },
         },
         agents: { defaults: { model: { primary: "openai/gpt-5" } } },
         models: { providers: { deepseek: { apiKey: "claw_test456" } } },
       } as never);
       mockGetConfigValue.mockResolvedValue(null);
+      fetchSpy.mockResolvedValue(new Response(
+        JSON.stringify({ tier: "free", deviceTier: null }),
+        { status: 200 },
+      ));
 
       const res = await GET();
       const body = await res.json();
 
       expect(body.clawaiTier).toBeNull();
       expect(body.clawaiAccountTier).toBeNull();
       expect(body.clawaiConfigured).toBe(true);
-      expect(fetchSpy).not.toHaveBeenCalled();
+      expect(fetchSpy).toHaveBeenCalledTimes(1);
     });
🤖 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 `@src/tests/routes/ai-models/status.test.ts` around lines 382 - 406, The test's
comment and assertion still assert the old behavior that the portal call is
skipped; update the test in the case using mockReadConfig / mockGetConfigValue
(the Free user with a paired clawai token and localTier=null) to reflect the new
route behavior: remove or rewrite the "defence-in-depth keeps the portal call
skipped" comment and change the assertion from
expect(fetchSpy).not.toHaveBeenCalled() to expect(fetchSpy).toHaveBeenCalled()
(the GET() route now queries the portal whenever a clawaiToken exists regardless
of localTier).
🤖 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.

Inline comments:
In `@scripts/gateway-pre-start.sh`:
- Around line 309-318: The code should defensively ensure cfg["auth"] is a dict
before calling .get to avoid AttributeError; change how profiles is derived so
it uses the existing pattern (e.g., auth = cfg.get("auth"); use auth if
isinstance(auth, dict) else {}), then set profiles = auth.get("profiles", {}) or
{}; keep the rest of the uses_codex logic but rely on profiles being a dict so
profiles.items() is safe and reference the variables primary, profiles, and
uses_codex when making the change.

---

Outside diff comments:
In `@src/tests/routes/ai-models/status.test.ts`:
- Around line 382-406: The test's comment and assertion still assert the old
behavior that the portal call is skipped; update the test in the case using
mockReadConfig / mockGetConfigValue (the Free user with a paired clawai token
and localTier=null) to reflect the new route behavior: remove or rewrite the
"defence-in-depth keeps the portal call skipped" comment and change the
assertion from expect(fetchSpy).not.toHaveBeenCalled() to
expect(fetchSpy).toHaveBeenCalled() (the GET() route now queries the portal
whenever a clawaiToken exists regardless of localTier).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 75f905d8-e57d-4913-a053-c8939b47473c

📥 Commits

Reviewing files that changed from the base of the PR and between 769dad5 and 378ca7a.

📒 Files selected for processing (11)
  • scripts/gateway-pre-start.sh
  • src/app/page.tsx
  • src/app/setup-api/ai-models/status/route.ts
  • src/components/ChatApp.tsx
  • src/components/ChatPopup.tsx
  • src/components/TierUpgradeCelebration.tsx
  • src/lib/desktop-translations-part1.ts
  • src/lib/desktop-translations-part2.ts
  • src/lib/desktop-translations-part3.ts
  • src/lib/desktop-translations.ts
  • src/tests/routes/ai-models/status.test.ts

Comment on lines +309 to +318
primary = (cfg.get("agents", {}).get("defaults", {}).get("model", {}) or {}).get("primary") or ""
profiles = cfg.get("auth", {}).get("profiles", {}) or {}
uses_codex = (
isinstance(primary, str) and primary.lower().startswith("openai-codex/")
) or any(
(isinstance(k, str) and k.lower().startswith("openai-codex:")) or
(isinstance(v, dict) and isinstance(v.get("provider"), str)
and v["provider"].lower() == "openai-codex")
for k, v in profiles.items()
)

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

Add defensive isinstance check for auth to match existing pattern.

Line 310 will raise AttributeError if cfg["auth"] exists but is not a dict (e.g., None or a corrupted value), since the exception handler only catches FileNotFoundError and JSONDecodeError. This would silently skip the codex install, causing confusing "harness not registered" errors.

The existing code at line 131 uses the defensive pattern—this should match:

Proposed fix
 primary = (cfg.get("agents", {}).get("defaults", {}).get("model", {}) or {}).get("primary") or ""
-profiles = cfg.get("auth", {}).get("profiles", {}) or {}
+auth = cfg.get("auth")
+profiles = auth.get("profiles", {}) if isinstance(auth, dict) else {}
+profiles = profiles if isinstance(profiles, dict) else {}
 uses_codex = (

As per coding guidelines: "Review for proper error handling" — the current pattern is inconsistent with line 131's defensive approach.

🤖 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 `@scripts/gateway-pre-start.sh` around lines 309 - 318, The code should
defensively ensure cfg["auth"] is a dict before calling .get to avoid
AttributeError; change how profiles is derived so it uses the existing pattern
(e.g., auth = cfg.get("auth"); use auth if isinstance(auth, dict) else {}), then
set profiles = auth.get("profiles", {}) or {}; keep the rest of the uses_codex
logic but rely on profiles being a dict so profiles.items() is safe and
reference the variables primary, profiles, and uses_codex when making the
change.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@playwright.config.ts`:
- Around line 3-4: The parsed port value assigned to the variable port can be
NaN and is then used to build baseURL and in webServer.command; validate port
after parsing (e.g., Number(...) stored in portRaw) and ensure
Number.isFinite(port) && Number.isInteger(port) && port > 0, otherwise set port
to a safe default (3100) or throw a clear error; then use the validated port
variable when constructing baseURL and in webServer.command (referencing the
port and baseURL identifiers).

In `@src/components/TierUpgradeCelebration.tsx`:
- Around line 188-193: The dialog overlay in TierUpgradeCelebration lacks
keyboard dismissal and a description link for assistive tech; update the
component to (1) generate stable IDs (useId or similar) for titleId and a new
descriptionId and add aria-describedby={descriptionId} on the root dialog, (2)
add a useEffect that registers a keydown listener to call onClose when Escape is
pressed and cleans up on unmount, and (3) change the overlay onClick to only
call onClose when the click target equals the overlay (e.target ===
e.currentTarget) so inner content clicks don't close it; ensure the description
element uses the descriptionId and avoid injecting unescaped HTML (use safe
text) to prevent XSS.
- Around line 74-120: The component TierUpgradeCelebration currently uses local
useState for dialog lifecycle; replace that with the shared window reducer
pattern from useWindows.ts: remove useState<DialogState|null> dialog and
setDialog usage and instead register this modal with the useWindows reducer
(import useWindows and the window action creators), dispatch the "open" action
when you need to show the upgrade/downgrade dialog (use the same payload shape {
kind: "upgrade", tier } or { kind: "downgrade-free" }) and dispatch the "close"
action in onClose (and update SEEN_KEY via the reducer close handler or via the
close action side-effect). Ensure rankOf/tier/loading logic still computes when
to dispatch open (replace setDialog calls with dispatch open) and keep
kv.set(SEEN_KEY, ...) behavior triggered on close via the window close handler
or by dispatching an action that includes the seen value; reference
TierUpgradeCelebration, useClawboxLogin, SEEN_KEY, FREE_SEEN_VALUE, rankOf and
integrate with the existing useWindows reducer API for
open/minimize/maximize/close semantics.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1b6be83b-5e3b-487a-8077-0ed3c4c02a72

📥 Commits

Reviewing files that changed from the base of the PR and between 378ca7a and 7368049.

📒 Files selected for processing (4)
  • playwright.config.ts
  • src/components/TierUpgradeCelebration.tsx
  • src/tests/routes/ai-models/status.test.ts
  • src/tests/unit/translations.test.ts

Comment thread playwright.config.ts
Comment on lines +3 to +4
const port = Number(process.env.PLAYWRIGHT_PORT || process.env.PORT || 3100);
const baseURL = `http://localhost:${port}`;

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

Validate parsed port before using it in URL/command.

On Line 3, Number(...) can produce NaN, which then breaks both baseURL and webServer.command (Line 42-43). Add a finite integer check and fallback/error.

Proposed fix
-const port = Number(process.env.PLAYWRIGHT_PORT || process.env.PORT || 3100);
+const rawPort = process.env.PLAYWRIGHT_PORT ?? process.env.PORT;
+const parsedPort = rawPort ? Number.parseInt(rawPort, 10) : 3100;
+const port = Number.isInteger(parsedPort) && parsedPort > 0 ? parsedPort : 3100;
 const baseURL = `http://localhost:${port}`;

Also applies to: 42-43

🤖 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 `@playwright.config.ts` around lines 3 - 4, The parsed port value assigned to
the variable port can be NaN and is then used to build baseURL and in
webServer.command; validate port after parsing (e.g., Number(...) stored in
portRaw) and ensure Number.isFinite(port) && Number.isInteger(port) && port > 0,
otherwise set port to a safe default (3100) or throw a clear error; then use the
validated port variable when constructing baseURL and in webServer.command
(referencing the port and baseURL identifiers).

Comment on lines +74 to +120
export default function TierUpgradeCelebration() {
const { tier, loading } = useClawboxLogin();
const { t } = useT();
const [dialog, setDialog] = useState<DialogState | null>(null);

useEffect(() => {
if (loading) return;
const seen = kv.get(SEEN_KEY);
const currentSeenValue = tier ?? FREE_SEEN_VALUE;

// First observation on this browser/device is a baseline, not a
// transition. Without this guard, already-paid accounts see the
// celebration every time this feature reaches a fresh client cache.
if (seen === null) {
kv.set(SEEN_KEY, currentSeenValue);
return;
}

const currentRank = rankOf(tier);
const seenRank = rankOf(seen);

// Upgrade to a paid tier we haven't celebrated yet.
if (currentRank > seenRank && (tier === "flash" || tier === "pro")) {
setDialog({ kind: "upgrade", tier });
return;
}
// Downgrade from any paid tier to Free.
if (currentRank === 0 && seenRank > 0) {
setDialog({ kind: "downgrade-free" });
return;
}
// Intermediate downgrade (Max → Pro) or no-change tick: silently
// sync `seen` so a later climb back to the same tier doesn't re-fire
// the celebration the user already saw.
if (seen !== currentSeenValue) kv.set(SEEN_KEY, currentSeenValue);
}, [tier, loading]);

if (!dialog) return null;

const onClose = () => {
if (dialog.kind === "upgrade") {
kv.set(SEEN_KEY, dialog.tier);
} else {
kv.set(SEEN_KEY, FREE_SEEN_VALUE);
}
setDialog(null);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Use the shared useWindows.ts reducer pattern for this modal’s lifecycle state.

dialog open/close is managed locally with useState, which diverges from the required window-state reducer pattern for component windows/modals.

As per coding guidelines, src/components/**/*.tsx: Use the useWindows.ts reducer pattern for managing window state (open, minimize, maximize, close, focus).

🧰 Tools
🪛 ESLint

[error] 97-97: Error: Calling setState synchronously within an effect can trigger cascading renders

Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:

  • Update external systems with the latest state from React.
  • Subscribe for updates from some external system, calling setState in a callback function when external state changes.

Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).

/home/jailuser/git/src/components/TierUpgradeCelebration.tsx:97:7
95 | // Upgrade to a paid tier we haven't celebrated yet.
96 | if (currentRank > seenRank && (tier === "flash" || tier === "pro")) {

97 | setDialog({ kind: "upgrade", tier });
| ^^^^^^^^^ Avoid calling setState() directly within an effect
98 | return;
99 | }
100 | // Downgrade from any paid tier to Free.

(react-hooks/set-state-in-effect)

🤖 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 `@src/components/TierUpgradeCelebration.tsx` around lines 74 - 120, The
component TierUpgradeCelebration currently uses local useState for dialog
lifecycle; replace that with the shared window reducer pattern from
useWindows.ts: remove useState<DialogState|null> dialog and setDialog usage and
instead register this modal with the useWindows reducer (import useWindows and
the window action creators), dispatch the "open" action when you need to show
the upgrade/downgrade dialog (use the same payload shape { kind: "upgrade", tier
} or { kind: "downgrade-free" }) and dispatch the "close" action in onClose (and
update SEEN_KEY via the reducer close handler or via the close action
side-effect). Ensure rankOf/tier/loading logic still computes when to dispatch
open (replace setDialog calls with dispatch open) and keep kv.set(SEEN_KEY, ...)
behavior triggered on close via the window close handler or by dispatching an
action that includes the seen value; reference TierUpgradeCelebration,
useClawboxLogin, SEEN_KEY, FREE_SEEN_VALUE, rankOf and integrate with the
existing useWindows reducer API for open/minimize/maximize/close semantics.

Comment on lines +188 to +193
<div
className="fixed inset-0 z-[100001] flex items-center justify-center bg-black/70 backdrop-blur-sm p-4"
role="dialog"
aria-modal="true"
aria-labelledby={titleId}
onClick={onClose}

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

Add Escape dismissal and dialog description linkage for keyboard/screen-reader completeness.

♿ Suggested accessibility patch
   return (
     <div
       className="fixed inset-0 z-[100001] flex items-center justify-center bg-black/70 backdrop-blur-sm p-4"
       role="dialog"
       aria-modal="true"
       aria-labelledby={titleId}
+      aria-describedby={`${titleId}-description`}
+      onKeyDown={(e) => {
+        if (e.key === "Escape") onClose();
+      }}
       onClick={onClose}
     >
@@
-          <p className="text-sm text-white/65 leading-relaxed">{body}</p>
+          <p id={`${titleId}-description`} className="text-sm text-white/65 leading-relaxed">{body}</p>

As per coding guidelines, src/components/**: React 19 components with Tailwind CSS v4. Review for accessibility, proper state management, and XSS prevention.

Also applies to: 215-219

🤖 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 `@src/components/TierUpgradeCelebration.tsx` around lines 188 - 193, The dialog
overlay in TierUpgradeCelebration lacks keyboard dismissal and a description
link for assistive tech; update the component to (1) generate stable IDs (useId
or similar) for titleId and a new descriptionId and add
aria-describedby={descriptionId} on the root dialog, (2) add a useEffect that
registers a keydown listener to call onClose when Escape is pressed and cleans
up on unmount, and (3) change the overlay onClick to only call onClose when the
click target equals the overlay (e.target === e.currentTarget) so inner content
clicks don't close it; ensure the description element uses the descriptionId and
avoid injecting unescaped HTML (use safe text) to prevent XSS.

@yalexx
yalexx merged commit 3306345 into ID-Robots:beta May 16, 2026
7 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 16, 2026
14 tasks
KrasimirKralev added a commit that referenced this pull request May 17, 2026
… path, useReducer + a11y (#134)

Follow-ups to PR #132 implementing CodeRabbit's four review prompts on
our code.

scripts/gateway-pre-start.sh
  - Guard `profiles.items()` behind an isinstance check (#132 review
    on line 318): `cfg["auth"]` may be `None` or a corrupted scalar
    on a hand-edited config, which would have raised AttributeError
    on the previous `.get("profiles", {})` call and silently skipped
    the codex install. Now matches the same defensive pattern used
    at line 131 for openrouter.
  - Derive `CODEX_PLUGIN_DIR` from `$(dirname "$OPENCLAW_CONFIG")`
    instead of hardcoding `/home/clawbox/.openclaw/...` (#133 review
    on line 323). Works for non-default clawbox users / per-user
    installs without changing behaviour on the default Jetson layout.

src/components/TierUpgradeCelebration.tsx
  - Switched from `useState` to a local `useReducer` (#132 review on
    line 120). Silences the ESLint `set-state-in-effect` warning and
    matches the spirit of the "reducer pattern" coding guideline.
    Deliberately NOT routed through `useWindows.ts` — that hook is
    for desktop windows with z-order / minimize / maximize, none of
    which apply to a transient centred modal, and forcing a fake
    appId / icon / defaultWidth through it would be worse than the
    status quo. `ClawBoxLoginModal.tsx` follows the same local-state
    shape and was not flagged.
  - Accessibility improvements (#132 review on line 193):
      * `useId()` for stable, collision-free title + description IDs
        — drops the hand-rolled "tier-upgrade-title" /
        "tier-downgrade-title" strings and the `titleId` field on
        the CONTENT table.
      * Added `aria-describedby` linking the body paragraph.
      * `Escape` keydown handler with cleanup on unmount.
      * Backdrop click guarded with `e.target === e.currentTarget`
        instead of relying on the inner card's `stopPropagation`.

Smoke-tested on Jetson:
  - `openclaw plugins uninstall codex --force` →
    `systemctl restart clawbox-gateway` → pre-start detects the
    missing plugin via the dynamically-resolved path, installs it,
    gateway comes up with `codex` in its loaded-plugins list.
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.

2 participants