Conversation
Kuberwastaken
left a comment
There was a problem hiding this comment.
Thanks! The wiring itself is correct — the fields land on App, App::new populates them, and the footer segment renders in the right place between the token-budget and rate-limit blocks.
The blocker is where the count is computed. App::new is a synchronous constructor called from crates/cli/src/main.rs:1876 inside the async runtime, and discover_skills is not cheap:
- it walks from
cwdto the filesystem root, scanning.claurst/skillsand.agents/skillsat every level (crates/core/src/skill_discovery.rs:161-172); - for every entry in
skills.urlsit callsfetch_git_skills, which shells out togit cloneon a cache miss (skill_discovery.rs:214-231).
So for anyone with a skills.urls entry, this puts a blocking network fetch on the TUI startup path just to render a number. It also fires in every test that builds an App (crates/tui/src/lib.rs:431, crates/tui/src/render.rs:3510 and :3539), which both slows the suite and lets the developer's real ~/.claurst/skills leak into test state.
Two other things:
- The counts go stale. They're computed once at startup and never refreshed, so connecting or removing an MCP server leaves the indicator wrong for the rest of the session.
0is rendered. The||guard means a user with skills but no MCP servers sees3 skills · 0 mcp. Worth emitting only the non-zero halves.- Nit: the new
// Skills + MCP count indicator.comment sits at column 0 inside an indented block inrender.rs.
The shape I'd suggest: pass the already-discovered skill count in from the caller rather than re-running discovery inside App::new, and update mcp_server_count wherever the MCP server list changes. Also heads-up that #401 is adding a discovered_skills field on App as part of its skill-discovery fix — once that lands you can just read .len() off it, which deletes most of this PR.
Sequencing note: I'm landing #403 (the app.rs → app/ module split) ahead of the feature queue, so please rebase before your next push — the App fields and App::new body will live in app/mod.rs. For a diff this small that should be quick.
Rework per review on Kuberwastaken#399, rebased on the app/ module split: - discovery no longer runs inside App::new: the count is computed on a spawn_blocking background task (discover_skills can shell out to git clone via skills.urls) and drained through a one-shot channel, mirroring recent_sessions_pending - mcp count reads McpManager::server_count() (connected servers), set by the CLI at startup and on every reconnect, not the configured list - footer emits only the non-zero halves: 3 skills / 2 mcp instead of '3 skills 0 mcp' - comment indentation fixed
968142c to
61485c6
Compare
|
Reworked per review, rebased onto main (post app/ split).
Happy to rebase onto #401's
|
|
All review points addressed in Skill discovery moved off
MCP count tracks connected servers, not configured list. Counts go stale less. MCP count is refreshed on reconnect. Skill count is a one-shot (discovery is expensive), which matches the review's suggestion. Once #401's
Comment indentation fixed. The
|
Summary
Adds skills and MCP server count indicator in the status bar, showing active skills and connected MCP servers.
Clean branch from current main, no shared payload. 24 lines, 2 files.