Add Claude Code Integration Guide + Fix Badge Priority Bug#3040
Conversation
WalkthroughAdds a new docs page for Claude Code Tab Badges and small restructuring in an existing wsh reference doc. Frontend onboarding components now use the useWaveEnv hook and waveEnv.electron.openExternal to open docs links. Badge handling was changed in both frontend and backend from unconditional overwrite to priority-then-ID comparison (cmpBadge) that merges/accepts only higher-priority incoming badges; sorting uses the same comparator. The mock wave env gained an openExternal implementation. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
Deploying waveterm with
|
| Latest commit: |
7bbf7b0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2521c719.waveterm.pages.dev |
| Branch Preview URL: | https://sawka-cc-hooks.waveterm.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/claude-code.mdx`:
- Around line 68-78: The two examples use mismatched gold hex codes: update the
hex so both examples use the same value (make the "AskUserQuestion" command's
color match the permission prompt example and prose); locate the "PreToolUse" ->
"AskUserQuestion" block and change the color in the command string ("wsh badge
message-question --color '#e0b965' --priority 20 --beep") to the canonical hex
used elsewhere (or change the other example to '#e0b965') so both JSON snippets
and the surrounding prose reference the identical gold hex.
In `@frontend/app/store/badge.ts`:
- Around line 215-220: The client currently replaces the atom's badge with
sortBadges(candidates)[0], which loses suppressed badges; change the atom
storage to maintain a map of badgeid→badge (use the same
curAtom/globalStore.get/set and sortBadges helpers) so on incoming data.badge
you set map[badge.badgeid]=badge, on clear/clearbyid you delete the badgeid
entry (or clear the map), and after each update derive and set the visible
winner as sortBadges(Object.values(map))[0]; keep the map in globalStore for
that curAtom so clears restore lower-priority active badges.
In `@pkg/wcore/badge.go`:
- Around line 97-102: The current logic overwrites
globalBadgeStore.transient[orefStr] with a single Badge, causing cleared
BadgeIds to remove other active badges; change transient to hold a map of
BadgeId->Badge per oref (keep symbol names: globalBadgeStore.transient,
incoming, BadgeId, Priority, orefStr) so Set path inserts/updates incoming into
that per-oref map and ClearById removes by BadgeId, and after each set/clear
recompute and publish the visible winner (choose highest Priority, break ties by
BadgeId) instead of storing a single Badge directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2b74d66-0101-456b-9ffb-7cf9a4efd1e0
📒 Files selected for processing (6)
docs/docs/claude-code.mdxfrontend/app/onboarding/onboarding-upgrade-v0140.tsxfrontend/app/onboarding/onboarding-upgrade-v0142.tsxfrontend/app/store/badge.tsfrontend/preview/mock/mockwaveenv.tspkg/wcore/badge.go
frontend/app/store/badge.ts
Outdated
| if (data.clear) { | ||
| globalStore.set(curAtom, null); | ||
| } else if (data.badge != null) { | ||
| const existing = globalStore.get(curAtom); | ||
| const candidates = existing != null ? [existing, data.badge] : [data.badge]; | ||
| globalStore.set(curAtom, sortBadges(candidates)[0]); |
There was a problem hiding this comment.
The client subscription still forgets suppressed badges.
sortBadges(candidates)[0] only keeps the displayed badge. If a higher-priority badge temporarily overrides a lower-priority active badge on the same oref, the later clear / clearbyid event leaves the atom empty instead of restoring the still-active lower-priority badge. Mirror the backend fix here too by tracking badges per badgeid and deriving the winner from that set.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/store/badge.ts` around lines 215 - 220, The client currently
replaces the atom's badge with sortBadges(candidates)[0], which loses suppressed
badges; change the atom storage to maintain a map of badgeid→badge (use the same
curAtom/globalStore.get/set and sortBadges helpers) so on incoming data.badge
you set map[badge.badgeid]=badge, on clear/clearbyid you delete the badgeid
entry (or clear the map), and after each update derive and set the visible
winner as sortBadges(Object.values(map))[0]; keep the map in globalStore for
that curAtom so clears restore lower-priority active badges.
pkg/wcore/badge.go
Outdated
| incoming := *data.Badge | ||
| existing, hasExisting := globalBadgeStore.transient[orefStr] | ||
| if !hasExisting || incoming.Priority > existing.Priority || (incoming.Priority == existing.Priority && incoming.BadgeId > existing.BadgeId) { | ||
| globalBadgeStore.transient[orefStr] = incoming | ||
| log.Printf("badge store: badge set: oref=%s badge=%+v\n", orefStr, incoming) | ||
| } |
There was a problem hiding this comment.
Don't collapse per-badge state into a single stored badge.
ClearById makes badge lifecycle per badge, but this still retains only one winner in transient[orefStr]. If badge A is active, badge B with higher priority arrives, and later B clears by ID, A is gone forever even though it was never cleared. Please store active badges per oref keyed by BadgeId, then recompute the visible winner after each set/clear instead of overwriting the store with a single badge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/wcore/badge.go` around lines 97 - 102, The current logic overwrites
globalBadgeStore.transient[orefStr] with a single Badge, causing cleared
BadgeIds to remove other active badges; change transient to hold a map of
BadgeId->Badge per oref (keep symbol names: globalBadgeStore.transient,
incoming, BadgeId, Priority, orefStr) so Set path inserts/updates incoming into
that per-oref map and ClearById removes by BadgeId, and after each set/clear
recompute and publish the visible winner (choose highest Priority, break ties by
BadgeId) instead of storing a single Badge directly.
Code Review SummaryStatus: 2 Ongoing Issues | Recommendation: Address before merge Overview
Ongoing Issues (from prior review)The following issues were identified in prior review and remain present in the current code: Previously Flagged - Not Yet Resolved
Resolved Issues
Files Reviewed (7 files)
The badge tracking issue represents a data integrity problem where badges can be prematurely lost. Please consider storing badges per badgeid and deriving the visible winner from that set rather than overwriting with a single badge. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/app/store/badge.ts (1)
240-251: Consider documenting the intentional difference in tie-breaking between sort functions.
sortBadgesuses descending badgeid (viacmpBadge(b, a)), whilesortBadgesForTabuses ascending badgeid. The ascending order insortBadgesForTabmatches the documented behavior inclaude-code.mdx: "Ties at the same priority go to the earliest badge set."A brief inline comment explaining why these differ would help future maintainers understand the intentional design.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/store/badge.ts` around lines 240 - 251, Add a short inline comment above sortBadges and sortBadgesForTab explaining the intentional tie-breaker difference: note that sortBadges uses descending badgeid (via cmpBadge(b, a)) while sortBadgesForTab uses ascending badgeid so "ties at the same priority go to the earliest badge set" per claude-code.mdx; reference the functions sortBadges and sortBadgesForTab and briefly state that this discrepancy is deliberate to preserve documented behavior.docs/docs/claude-code.mdx (1)
85-87: External issue references may become stale.The links to Claude Code issues (
#5186,#19627) are helpful context, but external issue URLs can change or become unavailable. Consider periodically verifying these links remain valid, or adding the issue titles inline so the context is preserved even if links break.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/claude-code.mdx` around lines 85 - 87, Update the "Known Issue" warning block that mentions Claude Code and `Notification` hooks to include the titles (or short summaries) of the referenced external issues (`#5186` and `#19627`) inline next to their links and add a short note to periodically verify the URLs; specifically edit the warning text containing "Notification" and the two issue links so it reads like: link + issue title (or brief one-line summary) and append a parenthetical reminder to check links for staleness, ensuring readers keep context if the external URLs change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/docs/claude-code.mdx`:
- Around line 85-87: Update the "Known Issue" warning block that mentions Claude
Code and `Notification` hooks to include the titles (or short summaries) of the
referenced external issues (`#5186` and `#19627`) inline next to their links and add
a short note to periodically verify the URLs; specifically edit the warning text
containing "Notification" and the two issue links so it reads like: link + issue
title (or brief one-line summary) and append a parenthetical reminder to check
links for staleness, ensuring readers keep context if the external URLs change.
In `@frontend/app/store/badge.ts`:
- Around line 240-251: Add a short inline comment above sortBadges and
sortBadgesForTab explaining the intentional tie-breaker difference: note that
sortBadges uses descending badgeid (via cmpBadge(b, a)) while sortBadgesForTab
uses ascending badgeid so "ties at the same priority go to the earliest badge
set" per claude-code.mdx; reference the functions sortBadges and
sortBadgesForTab and briefly state that this discrepancy is deliberate to
preserve documented behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c92f412-d337-4ce7-b4d0-d55223520164
📒 Files selected for processing (4)
docs/docs/claude-code.mdxdocs/docs/wsh-reference.mdxfrontend/app/store/badge.tspkg/wcore/badge.go
✅ Files skipped from review due to trivial changes (1)
- docs/docs/wsh-reference.mdx
1 participant
No description provided.