fix(skills): match ClawHub entries by slug when gateway skillKey differs#792
fix(skills): match ClawHub entries by slug when gateway skillKey differs#792octo-patch wants to merge 2 commits intoValueCell-ai:mainfrom
Conversation
…es during active send During an active send, the history poll (which fires every 4s) can race with a gateway reconnection. If the gateway's view of the session is incomplete at that moment -- because it's mid-reconnect or hasn't fully persisted the conversation yet -- the loaded history contains fewer messages than the local state, causing the entire conversation to vanish from the UI. The user then has to restart ClawX to see their messages again. Add a guard in applyLoadedMessages: if a send is in progress AND the loaded history contains fewer messages than the current local state, keep the local messages instead of replacing them. The next history load after the run completes will reconcile the final state. Fixes ValueCell-ai#709
When a skill's gateway skillKey differs from its ClawHub slug, the merge logic failed to find the existing skill, causing it to appear as a duplicate placeholder with 'Recently installed, initializing...' description that never resolves. Fix the matching predicate to also compare against the skill's slug field (which defaults to skillKey when absent from the gateway response), so skills are properly merged regardless of naming differences between the gateway and ClawHub. Fixes ValueCell-ai#317
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 691d73c104
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| finalMessages.length < preApplyState.messages.length && | ||
| preApplyState.messages.length > 1 | ||
| ) { | ||
| finalMessages = preApplyState.messages; |
There was a problem hiding this comment.
Exempt truncated history polls from the send-time length guard
loadHistory always requests chat.history with limit: 200, but this new guard drops any polled result whose finalMessages.length is smaller than the current local list during sending. In long threads, sending an optimistic user turn can make local state 201 messages while the gateway response is capped at 200, so every poll is discarded even when it contains the new assistant output. In the no-streaming fallback path this prevents completion detection and can leave the run stuck until timeout instead of converging to gateway history.
Useful? React with 👍 / 👎.
Fixes #317
Problem
When a skill's gateway
skillKeydiffers from its ClawHubslug, the merge logic infetchSkillsfailed to find the matching skill incombinedSkills. This caused the skill to be pushed again as a placeholder entry with the description"Recently installed, initializing...", which never resolved — the skill appeared stuck and could not be enabled.The root cause is the matching predicate on line 141:
s.idis set tos.skillKeyfrom the gateway response, but the gateway can also return a separateslugfield that differs fromskillKey. The ClawHub list usesslugas the identifier, so ifskillKey !== slug, the lookup returns nothing even when the skill is already present incombinedSkills.Solution
Extended the matching predicate to also check
s.slug:Since
slugin the mapped Skill defaults tos.skillKeywhen the gateway doesn't return a slug, this is backward-compatible: existing behavior is preserved whenskillKey === slug, and the new case handles when the gateway provides a slug that differs from the skillKey.Testing
Added a unit test in
tests/unit/skills-errors.test.tscovering the scenario where the gateway returns a skill withskillKey: "foo-v2"andslug: "foo", while ClawHub lists"foo"as installed. The test verifies that only one skill entry is produced (the gateway entry), not a duplicate placeholder.All existing tests continue to pass.