feat(ui): add skills discovery status to sidebar and landing page#2384
feat(ui): add skills discovery status to sidebar and landing page#2384andreynering merged 5 commits intocharmbracelet:mainfrom
Conversation
8543e9c to
55bb2d2
Compare
|
Very nice. Haven't vetted the code yet, but overall this is a great feature. |
55bb2d2 to
f4b3ea6
Compare
|
@Amolith I believe this is in line with what you mentioned the other day. @huaiyuWangh we intend to merge this: thanks for your patience with this one (as well as with your other, excellent PRs). |
eb3cc83 to
999028a
Compare
|
Thanks for your patience with this @huaiyuWangh. This is a great feature. Pending @andreynering's review we'll merge this. Some other changes I made:
|
andreynering
left a comment
There was a problem hiding this comment.
Thank you @huaiyuWangh!
We have a conflict. Can you fix so we can merge?
e306803 to
8b893a0
Compare
- Display loaded/errored skills in sidebar and landing page - Use pubsub event to carry skill states directly to UI, avoiding global mutable state - Subscribe to skills discovery events via broker, consistent with MCP/LSP patterns - Rename State to DiscoveryState for clarity
8b893a0 to
23cb495
Compare
|
Hi @andreynering, I’ve resolved the rebase conflicts, and tests are passing locally. Everything appears to be working as expected. Since quite some time has passed, I’m a bit concerned there could be subtle interactions with newer features. It would be great if you could give it another review before merge. |
|
Fwiw @huaiyuWangh and @andreynering, I spent some time with this the other day and got it to up to date with the current codebase and new features. I believe it's in good shape and I stand by it. |
|
Let's get this in. Thank you @huaiyuWangh!! |
Summary
Discover()publishes skill states via broker event, UI subscribes and updates automaticallyTest plan
go vetpasses on all changed packagesgo test ./internal/skills/...passes (including new event-based assertions)