Expose agent skills through but-api - #15192
Open
schacon wants to merge 2 commits into
Open
Conversation
The desktop app had no way to reach any of this: skill discovery, install, uninstall, and the workflow policy all lived behind the CLI. Adds a but_api module so a settings screen can read structured state and change it one framework at a time, instead of shelling out and parsing output. Seven commands: agents_status, agent_skill_install, agent_skill_uninstall, agent_policy_get, agent_policy_set, cli_install_state, and uninstall_cli. Mutations return the refreshed status so the UI needs a single round trip. The project is passed as an optional id rather than a Context because these commands are meaningful without one - the global settings screen manages $HOME skills before any repository is open. agents_status reports both scopes when a project is given and global only otherwise, and lists detected agents first. Two deliberate restrictions. Uninstall removes every installation discovered for a format rather than just the canonical path, since a skill installed into a custom folder name is still ours and would otherwise be orphaned. And agent_policy_set only rewrites instruction files that already hold a managed block, so saving preferences never seeds GitButler steering into an agent file the user never set up. agent_policy_get reports divergence when two instruction files disagree, which happens when a user hand-edits one, rather than silently showing whichever sorted first.
Backs the startup prompt with a persisted flag, so a user who declines the offer to set up agent skills is not asked again. Uses a new settings group rather than a stray boolean on `ui`, which is about window chrome. The tab this supports will grow more knobs, and a group keeps them together. Also adds the "update_ui" arm to but-server's command dispatch. It was registered in Tauri when it was added but never here, so any non-Tauri caller fell through to the catch-all and silently did nothing. Included with this commit because both arms are the same contiguous change to that match. No new test for the default: the existing ensure_default_settings_covers_all_fields already fails when a field has no entry in defaults.jsonc, which is what turns that omission from a startup panic into a test failure. Verified by removing the entry and watching it fail.
github-actions
Bot
added
rust
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new agents API has a couple of correctness gaps (policy divergence detection and uninstall scope handling) and the described agent_skills_update command is not implemented/registered across transports.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds a new but-api surface for managing coding-agent skills and workflow policy (plus CLI symlink inspection/uninstall) so GUI callers can render and mutate agent state without interactive CLI prompts, and wires it through Tauri + but-server while extending app settings with a new agents group.
Changes:
- Introduces
crates/but-api/src/agents.rswith DTOs and commands for agent/skill discovery, install/uninstall, policy get/set, and CLI symlink state. - Registers the new agents commands on the Tauri invoke handler and the
but-serverHTTP routes; updates generated SDK types and bindings where applicable. - Adds an
agentssettings group (skillsPromptDismissed) and fixes a missingupdate_uiregistration inbut-server’s command dispatch.
File summaries
| File | Description |
|---|---|
| packages/but-sdk/src/generated/linear/index.js | Exposes updateAgents in the native binding export list. |
| packages/but-sdk/src/generated/linear/index.d.ts | Adds agent-related DTO types and updateAgents typing; extends AppSettings with agents. |
| packages/but-sdk/src/generated/graph/index.js | Exposes updateAgents in the native binding export list. |
| packages/but-sdk/src/generated/graph/index.d.ts | Adds agent-related DTO types and updateAgents typing; extends AppSettings with agents. |
| crates/gitbutler-tauri/src/settings.rs | Adds Tauri command update_agents to persist the new settings group. |
| crates/gitbutler-tauri/src/main.rs | Registers new agents::* Tauri invoke handlers. |
| crates/but-testsupport/src/sandbox.rs | Initializes the new agents settings field in test sandbox settings. |
| crates/but-settings/src/lib.rs | Adds agents: Agents to AppSettings. |
| crates/but-settings/src/app_settings.rs | Defines the persisted Agents settings struct. |
| crates/but-settings/src/api.rs | Adds AgentsUpdate input DTO and AppSettingsWithDiskSync::update_agents. |
| crates/but-settings/assets/defaults.jsonc | Adds defaults for the new agents.skillsPromptDismissed setting. |
| crates/but-server/src/lib.rs | Registers agents HTTP routes and fixes missing update_ui command dispatch registration; adds update_agents dispatch. |
| crates/but-napi/src/settings.rs | Adds NAPI update_agents settings mutation. |
| crates/but-api/src/lib.rs | Exposes the new agents module from but-api. |
| crates/but-api/src/legacy/settings.rs | Adds legacy settings handler update_agents for non-Tauri callers. |
| crates/but-api/src/agents.rs | Implements the new agents/skills/policy/CLI transport API and associated JSON DTOs. |
| apps/desktop/src/lib/settings/appSettings.ts | Adds SettingsService.updateAgents() invoking update_agents. |
Review details
- Files reviewed: 13/17 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| // Two files disagreeing means the user edited one by hand or | ||
| // set them up separately; surface it rather than silently | ||
| // showing whichever came first alphabetically. | ||
| Some(first) if first.selected != options.selected => diverged = true, |
Comment on lines
+521
to
+525
| if let Some(format) = framework.format(matches!(scope, Scope::Global)) { | ||
| for path in but_skill::status::find_format_installations(format, &base) { | ||
| but_skill::install::remove_skill_files(&path)?; | ||
| } | ||
| } |
Comment on lines
+748
to
+752
| .route( | ||
| "/cli_install_state", | ||
| but_post(agents::cli_install_state_cmd), | ||
| ) | ||
| .route("/uninstall_cli", but_post(agents::uninstall_cli_cmd)) |
Comment on lines
+338
to
+342
| agents::tauri_cli_install_state::cli_install_state, | ||
| agents::tauri_uninstall_cli::uninstall_cli, | ||
| agents::tauri_agents_status::agents_status, | ||
| agents::tauri_agent_skill_install::agent_skill_install, | ||
| agents::tauri_agent_skill_uninstall::agent_skill_uninstall, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The transport layer between
but-skilland the desktop app. Stacked on #15190.The theme for review
Two narrow questions: is the API shape right, and is registration complete on every transport? Most of the line count is documented DTOs and generated SDK types, so it reads faster than the diff suggests.
Seven commands
New non-legacy
crates/but-api/src/agents.rs:agents_statusagent_skill_install/agent_skill_uninstallagent_skills_updateagent_policy_get/agent_policy_setcli_install_state/uninstall_clibutsymlinkMutations return the refreshed
AgentsStatusso the UI needs one round trip.Design decisions worth a look
The project is an optional id, not a
Context. These commands are meaningful without a project — the global settings screen manages$HOMEskills before any repository is open.agents_statusreports both scopes when a project is given and global only otherwise, and lists detected agents first. (#[but_api]also does not supportOption<&Context>; the macro only recognises owned/&/&mut Context.)Uninstall removes every discovered installation, not the canonical path. Skills are identified by
SKILL.mdfrontmatter, so one installed into a custom folder name lives elsewhere and would otherwise be orphaned.agent_skills_updatedoes the same thing for the same reason — writing the canonical path would leave the outdated copy in place and add a second one beside it.agent_policy_setonly rewrites files that already hold a managed block. Saving preferences should never seed GitButler steering into an agent's instruction file the user never set up.agent_policy_getreports divergence when two instruction files disagree — which happens when someone hand-edits one — rather than silently showing whichever sorted first.Registration
Tauri
invoke_handler,but-serverroutes, and regenerated SDK types. napi needs nothing;#[but_api]functions are auto-discovered through thebut-api/napifeature.Also: a settings group, and a bug fix
Adds an
agentssettings group withskillsPromptDismissed, backing the startup prompt in the next PR. A new group rather than a stray boolean onui(which is window chrome), since the tab will grow more knobs.While adding
update_agentstobut-server's dispatch I foundupdate_uiwas never registered there — it went into Tauri when it was added but not into the server, so any non-Tauri caller fell through to the catch-all and silently did nothing. Fixed in the same match.No new test for the settings default:
ensure_default_settings_covers_all_fieldsalready fails when a field has no entry indefaults.jsonc, which turns that omission from a startup panic into a test failure. Confirmed by removing the entry and watching it fail.Verification
cargo check --workspace --all-targetsandcargo clippy --workspaceclean;pnpm build:sdkregenerated and committed.