diff --git a/docs/superpowers/plans/2026-09-07-agent-skill-switches.md b/docs/superpowers/plans/2026-09-07-agent-skill-switches.md new file mode 100644 index 0000000000..7d4902e303 --- /dev/null +++ b/docs/superpowers/plans/2026-09-07-agent-skill-switches.md @@ -0,0 +1,380 @@ +# Per-Agent Skill Switches Implementation Plan + +> **Superseded:** The shared-root fan-out design in this document must not be +> implemented or restored. It was replaced by +> `2026-09-09-agent-skill-switch-hardening.md`: shared roots are non-toggleable, +> only private roots may use identity-scoped vaults, and project vaults live in +> Codeg's data directory. + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add real per-agent availability switches to Settings > Skills without letting a shared skill toggle silently affect another Codeg-managed agent. + +**Architecture:** Codex uses its official ordered `skills.config` rules, so toggles never move Codex files and can cover read-only system skills. Other agents keep native skill roots authoritative: disabled entries live in deterministic sibling vaults, and shared entries are fanned out into agent-unique roots before the shared source is hidden. The existing list/read/save/delete surface is extended so disabled skills remain manageable, and one new command performs serialized toggles over both transports. + +**Tech Stack:** Rust 2021, Tauri 2, Axum, Next.js 16, React 19, TypeScript, next-intl, Vitest. + +## Global Constraints + +- The switch is per agent and per scope; turning a shared skill off for one agent must preserve availability for every other Codeg-managed agent that already sees it. +- A disabled skill must be excluded from the selected agent's effective native discovery result. Codex does this through `skills.config`; other agents use filesystem isolation. +- Read-only CLI skills cannot be moved or edited. Codex system skills can still be toggled because its official configuration controls availability without mutating skill content. +- Existing user files and unrelated worktree changes must be preserved. +- Both Tauri desktop and Axum server transports must expose the same behavior. + +--- + +### Task 1: Backend Skill State And Private Toggle + +**Files:** +- Modify: `src-tauri/src/acp/types.rs` +- Modify: `src-tauri/src/commands/acp.rs` + +**Interfaces:** +- Produces: `AgentSkillItem { enabled: bool, can_toggle: bool, ... }` +- Produces: `acp_set_agent_skill_enabled(agent_type, scope, skill_id, workspace_path, enabled) -> Result` + +- [ ] **Step 1: Write failing filesystem tests** + +Add tests that create directory and flat-file skills in temporary active roots, +then exercise the wished-for helpers: + +```rust +let disabled = disabled_skill_root(&skills); +set_skill_enabled_in_roots(&peers, AgentType::Gemini, AgentSkillScope::Global, "demo", false)?; +assert!(!skills.join("demo").exists()); +assert!(disabled.join("demo").join("SKILL.md").is_file()); +let listed = list_skills_from_roots(AgentSkillScope::Global, &[skills], kind)?; +assert!(!listed[0].enabled); +``` + +- [ ] **Step 2: Run the focused Rust test and verify RED** + +Run: + +```bash +cd src-tauri && cargo test --features test-utils skill_enabled -- --nocapture +``` + +Expected: compilation fails because the new state fields and helpers do not +exist. + +- [ ] **Step 3: Implement disabled-root discovery and private moves** + +Add deterministic path and entry helpers, active-first list merging, lookup in +both active and disabled roots, a serialized mutation lock, collision +preflight, and idempotent private enable/disable moves. Extend read/save/delete +to resolve disabled entries. + +The command contract is: + +```rust +pub async fn acp_set_agent_skill_enabled( + agent_type: AgentType, + scope: AgentSkillScope, + skill_id: String, + workspace_path: Option, + enabled: bool, +) -> Result; +``` + +- [ ] **Step 4: Run focused tests and verify GREEN** + +Run the command from Step 2. Expected: private directory/flat-file, lookup, +idempotency, collision, and read-only tests pass. + +### Task 2: Shared Skill Fan-Out And Isolation + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` +- Reuse: `src-tauri/src/commands/experts.rs` + +**Interfaces:** +- Consumes: native roots from `skill_storage_spec` and `scoped_skill_dirs` +- Produces: an internal peer plan containing scan roots and one unique managed root per affected agent + +- [ ] **Step 1: Write failing shared-root tests** + +Use temporary roots for two peer agents and one shared root. Assert that +disabling for agent B moves the shared source to its vault, links agent A's +unique root to the canonical entry, leaves B's unique root empty, and reports A +enabled/B disabled. Add a peer-without-unique-root case that fails without any +move. + +```rust +assert!(peer_a.join("demo").join("SKILL.md").is_file()); +assert!(!peer_b.join("demo").exists()); +assert!(shared_disabled.join("demo").join("SKILL.md").is_file()); +``` + +- [ ] **Step 2: Run the focused Rust test and verify RED** + +Run: + +```bash +cd src-tauri && cargo test --features test-utils shared_skill -- --nocapture +``` + +Expected: the shared source disappears for both peers or the new planning API +is missing. + +- [ ] **Step 3: Implement shared preflight, fan-out, and rollback** + +Derive peers from `all_acp_agents()`, compare resolved scan-root paths, choose a +root used by exactly one peer, and create compatible directory/file links. Move +the shared source only after every destination has passed preflight. Track and +remove links plus restore the source if a later filesystem action fails. + +- [ ] **Step 4: Run focused tests and verify GREEN** + +Run the command from Step 2. Expected: fan-out, isolation, refusal, and rollback +tests pass. + +### Task 3: Expose The Toggle Over Desktop And Server Transports + +**Files:** +- Modify: `src-tauri/src/lib.rs` +- Modify: `src-tauri/src/web/handlers/acp.rs` +- Modify: `src-tauri/src/web/router.rs` +- Modify: `src/lib/api.ts` +- Modify: `src/lib/tauri.ts` +- Modify: `src/lib/types.ts` + +**Interfaces:** +- Consumes: the backend command from Task 1 +- Produces: `acpSetAgentSkillEnabled(params): Promise` + +- [ ] **Step 1: Add the typed frontend call before registration** + +```ts +export async function acpSetAgentSkillEnabled(params: { + agentType: AgentType + scope: AgentSkillScope + skillId: string + workspacePath?: string | null + enabled: boolean +}): Promise +``` + +- [ ] **Step 2: Run TypeScript checking and verify RED** + +Run `pnpm exec tsc --noEmit`. Expected: the transport call or the new DTO fields +are unavailable until all mirrors are updated. + +- [ ] **Step 3: Register both transport paths** + +Add the Tauri invoke handler, the Axum request DTO/handler, and +`/acp_set_agent_skill_enabled`; mirror `enabled` and `can_toggle` in TypeScript +and send camelCase request keys through the shared transport. + +- [ ] **Step 4: Run TypeScript and Rust checks** + +Run: + +```bash +pnpm exec tsc --noEmit +cd src-tauri && cargo check +cd src-tauri && cargo check --no-default-features --bin codeg-server +``` + +Expected: all commands exit 0. + +### Task 4: Skills Settings Switch And Autocomplete Filtering + +**Files:** +- Create: `src/components/settings/skills-settings.test.tsx` +- Modify: `src/components/settings/skills-settings.tsx` +- Create: `src/hooks/use-agent-skills.test.tsx` +- Modify: `src/hooks/use-agent-skills.ts` + +**Interfaces:** +- Consumes: `AgentSkillItem.enabled`, `AgentSkillItem.can_toggle`, and `acpSetAgentSkillEnabled` +- Produces: a row switch whose accessible name identifies the skill and selected agent + +- [ ] **Step 1: Write failing component and hook tests** + +Mock the existing API module, render one enabled skill, click its switch, and +assert the exact request plus authoritative reload: + +```ts +expect(acpSetAgentSkillEnabled).toHaveBeenCalledWith({ + agentType: "codex", + scope: "global", + skillId: "demo", + workspacePath: null, + enabled: false, +}) +expect(acpListAgentSkills).toHaveBeenCalledTimes(3) +``` + +Also assert that `useAgentSkills` excludes `{ enabled: false }` and that +read-only/non-toggleable switches are disabled. + +- [ ] **Step 2: Run focused Vitest and verify RED** + +Run: + +```bash +pnpm test -- src/components/settings/skills-settings.test.tsx src/hooks/use-agent-skills.test.tsx +``` + +Expected: switch queries and disabled filtering fail because neither behavior +exists. + +- [ ] **Step 3: Implement the switch behavior** + +Add a stable switch to each list row, stop its click from changing row +selection, track one in-flight skill ID, call the new API, invalidate the cache, +reload the authoritative list, and show localized success/error feedback. Filter +disabled items in `useAgentSkills` before caching. + +- [ ] **Step 4: Run focused Vitest and verify GREEN** + +Run the command from Step 2. Expected: all focused tests pass. + +### Task 5: Localization And Final Verification + +**Files:** +- Modify: `src/i18n/messages/ar.json` +- Modify: `src/i18n/messages/de.json` +- Modify: `src/i18n/messages/en.json` +- Modify: `src/i18n/messages/es.json` +- Modify: `src/i18n/messages/fr.json` +- Modify: `src/i18n/messages/ja.json` +- Modify: `src/i18n/messages/ko.json` +- Modify: `src/i18n/messages/pt.json` +- Modify: `src/i18n/messages/zh-CN.json` +- Modify: `src/i18n/messages/zh-TW.json` + +**Interfaces:** +- Produces: matching keys for switch labels, enabled/disabled success, failure, and unavailable hints in every locale + +- [ ] **Step 1: Add the same message-key shape to all locales** + +Add `availability.enabled`, `availability.disabled`, +`availability.toggleAria`, `availability.readOnly`, +`availability.cannotIsolate`, and the corresponding toggle toast keys. + +- [ ] **Step 2: Run final frontend verification** + +```bash +pnpm eslint . +pnpm test +pnpm build +``` + +Expected: each command exits 0 with no failing tests. + +- [ ] **Step 3: Run final shared-backend verification** + +```bash +cd src-tauri && cargo test --features test-utils +cd src-tauri && cargo check --no-default-features --bin codeg-server +cd src-tauri && cargo test --no-default-features --bin codeg-server --lib +``` + +Expected: each command exits 0 with no failing tests. + +- [ ] **Step 4: Review the diff and commit** + +Run `git diff --check`, inspect `git diff --stat` and `git status --short`, then +commit only the task files on the current feature branch without merging, +rebasing, or pushing `main`. + +### Task 6: Review Fixes For Incoming Links And Cursor Vaults + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` + +**Interfaces:** +- Produces: a preflight that enumerates every supported entry in peer active + roots and disabled vaults, follows each link chain through intermediate + targets, then rejects a canonical move if an alias or same-name entry points + into that canonical Skill +- Produces: a distinct deterministic vault only for Cursor's exact builtin + `~/.cursor/skills-cursor` root, while preserving the legacy vault for + `~/.cursor/skills` and custom roots that happen to use the same basename +- Preserves: `AgentSkillItem.can_toggle` and toggle execution report the same + feasibility + +- [x] **Step 1: Write three failing filesystem regressions** + +Add tests for a shared canonical Skill with an incoming link from a peer that +does not scan the shared root, a private canonical Skill with an incoming peer +link, and Cursor's global writable Skill root: + +```rust +assert!(result.is_err(), "the canonical move must be refused"); +assert!(canonical.join("SKILL.md").is_file()); +assert!(!listed.can_toggle); + +assert_eq!( + disabled_skill_root(&home.join(".cursor/skills")), + home.join(".cursor/.skills.codeg-disabled") +); +assert_ne!( + disabled_skill_root(&home.join(".cursor/skills")), + disabled_skill_root(&home.join(".cursor/skills-cursor")) +); +``` + +- [x] **Step 2: Run focused tests and verify RED** + +Run: + +```bash +cd src-tauri +cargo test --features test-utils incoming_peer_link -- --nocapture +cargo test --features test-utils cursor_global_skill_round_trip -- --nocapture +``` + +Expected: the incoming-link toggles mutate the canonical entry instead of +refusing, and Cursor advertises a toggle that execution rejects because the two +sibling roots resolve to one vault. + +- [x] **Step 3: Implement the minimal preflight and vault compatibility fix** + +Add an entry-level preflight that scans every peer's supported entries in both +active roots and disabled vaults. Index the lexical and resolved identities of +each intermediate and final link target, resolving relative targets from the +link's resolved physical parent and bounding traversal at 40 links. Reject +aliases, case variants, absolute backreferences through the active entry, and +any incomplete link scan before moving the canonical entry. Invoke the same +preflight from capability calculation and command execution. Preserve safe +relative links that remain inside the physical Skill bundle even when the outer +Skill entry is itself a symlink. + +Keep `.cursor/skills` and all unrelated custom roots on their existing +`.skills.codeg-disabled` vaults so disabled entries remain discoverable. Map +only the read-only `.cursor/skills-cursor` root to +`.cursor/.skills-cursor.codeg-disabled` so it no longer claims the writable +root's vault. + +- [x] **Step 4: Run focused tests and verify GREEN** + +Run: + +```bash +cd src-tauri +cargo test --features test-utils incoming_peer_link -- --nocapture +cargo test --features test-utils cursor_global_skill_round_trip -- --nocapture +cargo test --features test-utils skill_capability -- --nocapture +cargo test --features test-utils shared_skill -- --nocapture +``` + +Expected: all focused regressions and existing Skill isolation tests pass. + +- [x] **Step 5: Run the complete repository verification matrix and review** + +Run the frontend checks and every desktop, server, and `codeg-mcp` Rust command +listed in `AGENTS.md`, followed by `git diff --check`. Request an independent +read-only review of the final commit range, resolve all Critical or Important +findings, and commit the fixes on the current feature branch. + +The first review found alias/case-variant links outside the exact Skill ID and +custom roots named `skills-cursor` were still unsafe. Those cases now have +RED/GREEN regressions, and the follow-up review reported no remaining Critical +or Important findings. The final review also found a two-hop alias blind spot +and incorrect relative-target resolution below symlinked Skill directories; +both cases now have RED/GREEN regressions alongside an unsafe absolute +backreference control. diff --git a/docs/superpowers/plans/2026-09-09-agent-skill-switch-hardening.md b/docs/superpowers/plans/2026-09-09-agent-skill-switch-hardening.md new file mode 100644 index 0000000000..6f76dff490 --- /dev/null +++ b/docs/superpowers/plans/2026-09-09-agent-skill-switch-hardening.md @@ -0,0 +1,143 @@ +# Agent Skill Switch Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make per-Agent skill switches match each Agent's real availability state without moving shared or Codeg-managed skills. + +**Architecture:** Codex keeps a declarative `skills.config` implementation and gains complete user, project, plugin, and system discovery. Other Agents may move only entries from private roots into identity-scoped vaults; shared roots and central Codeg skill-pack links are classified but never mutated. Tauri and Axum pass the resolved Codeg data directory to common core functions so project vaults remain outside repositories. + +**Tech Stack:** Rust 2021, Tauri 2, Axum, TOML/toml_edit, Next.js 16, React 19, TypeScript, next-intl, Vitest. + +## Global Constraints + +- Do not merge into, rebase onto, or push `main`. +- Do not move canonical entries from roots consumed by more than one Agent or by external tools. +- Do not move links managed by Experts, Office, Science, or Custom Skills. +- Preserve unknown Codex TOML keys, comments, ordering, and symlinked config targets. +- Use rename-only private toggles and refuse cross-filesystem operations. +- Do not automatically migrate or delete legacy fan-out vaults because they have no ownership manifest. +- Keep the existing frontend request payload stable across Tauri and Axum. + +--- + +### Task 1: Encode Toggle Availability Reasons + +**Files:** +- Modify: `src-tauri/src/acp/types.rs` +- Modify: `src/lib/types.ts` +- Modify: `src/components/settings/skills-settings.tsx` +- Modify: `src/components/settings/skills-settings.test.tsx` +- Modify: `src/i18n/messages/*.json` + +**Interfaces:** +- Produces: `AgentSkillToggleReason` serialized as snake_case. +- Produces: `AgentSkillItem.toggle_reason: Option` and TypeScript mirror `toggle_reason: AgentSkillToggleReason | null`. + +- [x] **Step 1: Add failing Rust serialization and frontend reason-rendering tests.** +- [x] **Step 2: Run `cargo test --features test-utils agent_skill_toggle_reason -- --nocapture` and the focused Skills Settings Vitest; confirm failures are caused by the missing field and messages.** +- [x] **Step 3: Add the enum/field, map every reason to localized copy, and keep `can_toggle` for transport compatibility.** +- [x] **Step 4: Re-run both focused commands and confirm they pass.** + +### Task 2: Replace Fan-Out With Private Root Plans + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` + +**Interfaces:** +- Produces: `SkillRootTopology`, resolving all configured roots once per request. +- Produces: `SkillRootPlan { active, vault, legacy_vault, shared }` for one Agent and scope. +- Produces: private vault paths containing workspace, Agent, and root identity. + +- [x] **Step 1: Replace fan-out expectations with failing tests asserting `shared_root`, no filesystem mutation, Codeg-managed-link refusal, project vault placement outside the workspace, and distinct vaults for sibling/custom roots.** +- [x] **Step 2: Run the focused `shared_skill`, `managed_skill`, and `project_skill_vault` tests and confirm RED.** +- [x] **Step 3: Implement root plans, active-first listing, exact private moves, same-filesystem checks, collision/link preflight, and read-only legacy discovery. Remove fan-out/link/delete transaction helpers.** +- [x] **Step 4: Re-run focused private/shared tests and confirm GREEN.** + +### Task 3: Pass The Effective Data Directory Through Both Runtimes + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` +- Modify: `src-tauri/src/web/handlers/acp.rs` +- Modify: `src-tauri/src/lib.rs` +- Modify: `src-tauri/tests/api_integration.rs` + +**Interfaces:** +- Produces: `_core` list/toggle/read/save/delete functions accepting `data_dir: &Path`. +- Preserves: frontend JSON payloads and Tauri command names. + +- [x] **Step 1: Add failing core and Axum tests proving two data directories cannot see each other's project vaults.** +- [x] **Step 2: Run the focused Rust tests and confirm missing explicit data-directory plumbing is the failure.** +- [x] **Step 3: Add Tauri wrappers using `resolve_effective_data_dir`, Axum handlers using `AppState.data_dir`, and route all operations through the core functions.** +- [x] **Step 4: Run desktop and server `cargo check` plus the focused API test.** + +### Task 4: Match Codex Native Configuration Semantics + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` + +**Interfaces:** +- Consumes: valid selectors containing exactly one of `path` and `name`. +- Produces: path-only overrides for precise per-skill writes. +- Preserves: mixed/unknown entries and both array-of-tables and inline-array formatting. + +- [x] **Step 1: Add failing tests for mixed selectors, root inline `skills = { config = [...] }`, frontmatter names, and `skills.bundled.enabled = false`.** +- [x] **Step 2: Run the four named tests and verify each fails for its intended semantic mismatch.** +- [x] **Step 3: Implement exactly-one selector matching, path-only updates, root inline-table mutation, frontmatter-name extraction, and bundled-state overlay.** +- [x] **Step 4: Re-run all Codex skill configuration tests and confirm GREEN.** + +### Task 5: Discover Enabled Codex Plugin Skills + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` + +**Interfaces:** +- Reads: `[plugins."@"].enabled` and cached `.codex-plugin/plugin.json` manifests. +- Produces: read-only, natively toggleable Codex items named `:`. + +- [x] **Step 1: Add a failing temporary-CODEX_HOME test with enabled and disabled plugins, duplicate cache versions, and a namespaced `SKILL.md`.** +- [x] **Step 2: Run the plugin test and confirm no plugin skill is currently discovered.** +- [x] **Step 3: Resolve one active cached manifest per enabled plugin, validate its declared skills directory, namespace item identities, and protect plugin files from save/delete.** +- [x] **Step 4: Re-run plugin and native-toggle tests and confirm GREEN.** + +### Task 6: Remove Per-Skill Topology Rescans And Verify + +**Files:** +- Modify: `src-tauri/src/commands/acp.rs` +- Modify: `docs/superpowers/plans/2026-09-07-agent-skill-switches.md` + +**Interfaces:** +- Preserves: deterministic scope/name ordering and active-first duplicate handling. +- Removes: fan-out planning, peer link creation/deletion, and O(skills x peer-directory-scans) capability checks. + +- [x] **Step 1: Add a regression test that counts topology/root directory reads independently of skill count where practical, and otherwise assert classification consumes a prebuilt topology.** +- [x] **Step 2: Remove obsolete fan-out tests/helpers and mark the old plan's fan-out task as superseded by this plan.** +- [x] **Step 3: Run `cargo fmt --check`, all skill-related Rust tests, focused frontend tests, and `git diff --check`.** +- [x] **Step 4: Run the complete frontend and Rust verification matrix from `AGENTS.md`, inspect the final diff, and commit the remediation on the current feature branch.** + +Verification on 2026-09-09: + +- `cargo test --features test-utils skill -- --nocapture`: 75 Skill-related + tests passed, including the final two-hop alias, physical-parent relative + link, and absolute-backreference regressions. +- `pnpm test`: 428 files and 6132 tests passed. `pnpm build` passed. ESLint + exited successfully with one pre-existing warning in + `src/components/layout/status-bar-mcp.tsx`. +- Desktop `cargo check` and all-target Clippy passed. The complete desktop test + matrix passed with 3572 library tests plus all integration tests after + filtering one pre-existing platform-sensitive Git assertion. +- Server check, Clippy, and library tests passed; 3536 library tests passed + after filtering the same Git assertion. `codeg-mcp` check and Clippy passed. +- `git diff --check` passed. +- `cargo fmt --check` remains red because Rustfmt 1.93 reports repository-wide + formatting drift across untouched files. Running full-repository formatting + would create unrelated churn, so this change does not rewrite those files. +- The filtered test is + `commands::folders::tests::remove_worktree_deletes_a_branch_plain_delete_cannot`. + Apple Git 2.39.3 says `checked out at`, while the existing assertion accepts + only `used by worktree`; `src-tauri/src/commands/folders.rs` is unchanged by + this branch. Running that test alone reproduces the baseline failure. + +Residual platform coverage: Unix symbolic-link behavior is covered locally and +Windows link/reparse handling is covered statically and by Windows-gated tests, +but the final link-chain changes were not exercised on a live Windows volume. +Chains longer than 40 links are intentionally classified as `unsafe_link`. diff --git a/docs/superpowers/specs/2026-09-07-agent-skill-switches-design.md b/docs/superpowers/specs/2026-09-07-agent-skill-switches-design.md new file mode 100644 index 0000000000..32c671ac0d --- /dev/null +++ b/docs/superpowers/specs/2026-09-07-agent-skill-switches-design.md @@ -0,0 +1,197 @@ +# Per-Agent Skill Switches Design + +## Goal + +Add an availability switch to Settings > Skills so the same skill can be +enabled for one agent and disabled for another, matching the per-agent meaning +of MCP assignments. + +An off switch must change what the selected agent can discover. Hiding a skill +only from Codeg autocomplete is not sufficient. + +## Current Behavior And Safety Boundary + +Codeg discovers skills by scanning each agent's native global or project skill +directories. Some directories belong to one agent, such as +`~/.claude/skills`. Others, especially `.agents/skills`, are read by several +agents and by tools outside Codeg. + +Codeg may only move an entry when its active root belongs to exactly one agent +and scope. A shared root remains owned by the upstream installer: Codeg neither +moves its entries nor creates replacement links in peer roots. This is a +deliberate scope limit, not a transient error. + +## User Experience + +- Every skill row has an availability switch for the currently selected agent. +- An enabled skill remains discoverable by that agent. A disabled skill remains + visible in Settings so it can be previewed, edited, deleted, or re-enabled, + but it is omitted from Codeg skill autocomplete and from the agent's + effective native discovery result. +- A switch shows an in-progress state while the filesystem operation runs. +- On failure, the authoritative list is reloaded, the switch returns to its + prior state, and a localized error toast is shown. +- For non-Codex agents, CLI-owned read-only skills remain visible but cannot be + toggled. Codex system skills remain toggleable because Codeg changes only + Codex's official availability configuration, not the skill files. +- A skill whose shared installation cannot be separated without changing + another consumer is marked non-toggleable with a shared-root explanation. +- Skills linked from Codeg's central skill-pack store are marked as managed by + the Experts, Office, Science, or Custom Skills page. The generic switch never + moves those links. +- Every unavailable switch carries a machine-readable reason so the UI can + distinguish read-only content, a shared root, Codeg-managed content, a + storage collision, an unsafe link, a cross-filesystem move, legacy state, and + a Codex-wide configuration override. +- A new or reconnected agent session is required when an already-running agent + caches its skill inventory. + +## Agent-Specific Control Models + +No database flag is authoritative. Codeg changes the native state consumed by +each agent so its own discovery result remains the source of truth. + +### Codex Native Configuration + +Codeg does not move Codex skill files. It reads and atomically updates +`CODEX_HOME/config.toml` using Codex's official `skills.config` entries. Rules +are evaluated in file order and the last valid matching selector wins. A valid +entry has exactly one selector: `path` or `name`. Entries containing both (or +neither) are preserved but ignored when calculating state, matching current +Codex behavior. When a broader or later `name` selector would override the +requested state, Codeg appends a path-only rule. A repeated toggle updates only +an existing path-only rule instead of modifying a mixed selector or growing the +configuration indefinitely. + +The selector name comes from the `SKILL.md` frontmatter, including the plugin +namespace used by Codex. Enabled plugin manifests under the Codex plugin cache +are included in discovery. `skills.bundled.enabled = false` is authoritative: +bundled system skills are shown disabled and cannot be individually re-enabled +until that global setting is enabled. Project, user, plugin, and system files +remain in place. A new Codex session is required because an existing session +may have cached its skill inventory. + +### Filesystem Isolation For Other Agents + +Agents without a native availability configuration continue to use filesystem +isolation because their CLIs scan native skill roots directly. + +Each private global root uses a sibling Codeg vault with agent and root +identity. Each private project root uses Codeg's resolved data directory with +workspace, agent, and root identity: + +```text +~/.claude/skills/pdf/SKILL.md +~/.claude/.codeg-skill-vaults/v1/claude_code//pdf/SKILL.md + +/skill-vaults/v1////pdf/SKILL.md +``` + +Directory skills and flat Markdown skills retain their original entry name and +layout. Before advertising the switch, Codeg verifies that source and vault are +on the same filesystem; otherwise it returns `cross_filesystem`. Codeg never +falls back to a recursive copy because that would change symlink identity and +weaken crash behavior. + +### Private Root + +Disabling renames the entry from the native root into its exact vault. Enabling +renames it back. Destination collisions and symlinks that would change meaning +after the move are rejected before mutation. A private entry is also rejected +when another configured Agent root contains a directory, Markdown, or +`SKILL.md` link into that entry; moving it would otherwise leave the peer with +a dangling link. Preflight simulates every missing destination-root ancestor +that `create_dir_all` creates, so safe internal and stable absolute links remain +toggleable even before a multi-level vault exists. + +Incoming-link discovery records both lexical and resolved identities for every +link target and each intermediate target in a chain, with traversal bounded at +40 links. A relative target is resolved from the link's resolved physical +parent, not merely from the path used to enter a symlinked directory. This keeps +an internal relative `SKILL.md -> docs/body.md` link valid when the outer Skill +entry is itself a symlink. An absolute link that points back through the active +entry is rejected because moving that entry would break the reference. An +unreadable, dangling, cyclic, or overlong chain makes the topology scan +incomplete, and private moves are then conservatively unavailable. + +### Shared Root + +Shared roots such as global or project `.agents/skills` are listed normally, +but their generic per-agent switch is unavailable with reason `shared_root`. +Codeg leaves both the canonical entry and every peer root unchanged. An agent +that later gains a native declarative availability setting can opt into an +agent-specific implementation without changing this filesystem rule. + +Vaults produced by pre-release fan-out builds have no trustworthy ownership +manifest. Codeg may show those entries as `legacy_state` for recovery, but it +does not infer ownership, relink peers, delete them, or move them automatically. + +## API And Data Flow + +`AgentSkillItem` gains: + +```text +enabled: bool +can_toggle: bool +toggle_reason: read_only | shared_root | managed_elsewhere | + storage_conflict | unsafe_link | cross_filesystem | + legacy_state | bundled_disabled | config_error | null +``` + +For non-Codex agents, the list command scans each active root and its exact +private vault once. Active entries win when the same ID occurs more than once. +Root topology and incoming peer links are indexed once per request rather than +rescanned once per skill. An incomplete peer-link scan conservatively disables +private moves. For Codex, no peer-link scan is needed: the list command overlays +native configuration state on user, project, plugin, and system skills while +retaining each original path. + +A new command is available over both Tauri and Axum transports: + +```text +acp_set_agent_skill_enabled( + agent_type, + scope, + skill_id, + workspace_path, + enabled +) -> AgentSkillItem +``` + +The resolved Codeg data directory is passed explicitly by both Tauri and Axum +to list, toggle, read, save, and delete core functions. Read, save, and delete +resolve both active and private-vault entries so turning a skill off does not +make it unmanageable. Saving a new skill creates an enabled entry. Plugin, +system, shared legacy, and Codeg-managed content remains protected from writes. +The frontend invalidates its skill cache after every toggle, and the generic +`useAgentSkills` hook returns enabled entries only. + +## Consistency And Failure Handling + +- Skill mutations are serialized inside the backend. +- Validation, ownership, filesystem, destination, and link checks run before a + private move. +- Shared roots are never mutated by the availability command. +- Repeated enable or disable requests are idempotent. +- Symbolic links are moved as links, never followed and copied during private + disable operations. +- Built-in system paths retain the existing backend content-write protection. + Codex availability remains configurable because toggling writes only the + Codex config file. + +## Tests + +Rust tests cover Codex selector validity, frontmatter names, root inline tables, +plugin namespaces, bundled disablement, stable paths, atomic config writes, and +new-session behavior. They also cover private directory and flat-file toggles, +project vault placement, root identity, disabled discovery, idempotency, +shared-root refusal without mutation, Codeg-managed links, collision refusal, +incoming directory/content/Markdown links, fresh multi-level vault link +round-trips, cross-filesystem refusal, and non-Codex read-only rejection. +Existing skill storage tests continue to pin each agent's native roots. + +Frontend tests cover switch state, the exact toggle request, cache invalidation, +authoritative reload, disabled autocomplete filtering, read-only/non-toggleable +rows, and failure rollback. The final gate runs focused tests followed by the +repository's frontend lint/test/build and Rust desktop/server checks appropriate +to the touched shared backend. diff --git a/src-tauri/src/acp/types.rs b/src-tauri/src/acp/types.rs index 50cdbf9934..c106c2459d 100644 --- a/src-tauri/src/acp/types.rs +++ b/src-tauri/src/acp/types.rs @@ -1482,7 +1482,7 @@ pub struct AgentDiagnosticsReport { pub plain_text: String, } -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] #[serde(rename_all = "snake_case")] pub enum AgentSkillScope { Global, @@ -1496,6 +1496,20 @@ pub enum AgentSkillLayout { SkillDirectory, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum AgentSkillToggleReason { + ReadOnly, + SharedRoot, + ManagedElsewhere, + StorageConflict, + UnsafeLink, + CrossFilesystem, + LegacyState, + BundledDisabled, + ConfigError, +} + #[derive(Debug, Clone, Serialize)] pub struct AgentSkillLocation { pub scope: AgentSkillScope, @@ -1510,6 +1524,12 @@ pub struct AgentSkillItem { pub scope: AgentSkillScope, pub layout: AgentSkillLayout, pub path: String, + /// Whether the skill currently lives in an agent-visible skills root. + pub enabled: bool, + /// Whether Codeg can change this skill's availability for this agent. + pub can_toggle: bool, + /// Machine-readable explanation when `can_toggle` is false. + pub toggle_reason: Option, /// Best-effort `description:` extracted from the SKILL.md YAML /// frontmatter. `None` when there is no frontmatter or no key. pub description: Option, @@ -1564,6 +1584,18 @@ pub struct ForkResultInfo { mod envelope_tests { use super::*; + #[test] + fn agent_skill_toggle_reason_serializes_as_snake_case() { + assert_eq!( + serde_json::to_value(AgentSkillToggleReason::ManagedElsewhere).unwrap(), + serde_json::json!("managed_elsewhere") + ); + assert_eq!( + serde_json::to_value(AgentSkillToggleReason::CrossFilesystem).unwrap(), + serde_json::json!("cross_filesystem") + ); + } + #[test] fn event_envelope_serializes_with_flat_payload() { let env = EventEnvelope { diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index 1ed2d91ca2..6a2cbe397f 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -1,9 +1,11 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; +use std::io::Write; use std::path::{Path, PathBuf}; use std::time::Duration; use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; #[cfg(feature = "tauri-runtime")] use tauri::{Manager, State}; @@ -16,10 +18,10 @@ use crate::acp::preflight::{self, PreflightResult}; use crate::acp::registry; use crate::acp::types::{ AcpAgentInfo, AgentDiagnosticsReport, AgentSkillContent, AgentSkillItem, AgentSkillLayout, - AgentSkillLocation, AgentSkillScope, AgentSkillsListResult, CodexGranularApproval, - CodexSandboxSettings, CodexSandboxStructuredConfig, CodexWorkspaceWrite, ConfigStaleKind, - ConnectionStatus, DiagCheck, DiagLevel, DiagSection, DiagnosticsVerdict, GrokSettings, - GrokStructuredConfig, + AgentSkillLocation, AgentSkillScope, AgentSkillToggleReason, AgentSkillsListResult, + CodexGranularApproval, CodexSandboxSettings, CodexSandboxStructuredConfig, CodexWorkspaceWrite, + ConfigStaleKind, ConnectionStatus, DiagCheck, DiagLevel, DiagSection, DiagnosticsVerdict, + GrokSettings, GrokStructuredConfig, }; #[cfg(feature = "tauri-runtime")] use crate::acp::types::{ConnectionInfo, ForkResultInfo, PromptInputBlock}; @@ -2675,6 +2677,14 @@ fn codex_config_toml_path() -> PathBuf { codex_home_dir().join("config.toml") } +static CODEX_CONFIG_MUTATION_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +fn lock_codex_config_mutation() -> Result, AcpError> { + CODEX_CONFIG_MUTATION_LOCK + .lock() + .map_err(|_| AcpError::protocol("codex config mutation lock poisoned")) +} + fn codex_auth_json_path() -> PathBuf { codex_home_dir().join("auth.json") } @@ -3320,6 +3330,7 @@ fn persist_codex_local_config(config_patch_json: Option<&str>) -> Result<(), Acp model, env, } = runtime; + let _config_guard = lock_codex_config_mutation()?; let config_path = codex_config_toml_path(); let mut toml_value = if config_path.exists() { @@ -3412,15 +3423,11 @@ fn persist_codex_local_config(config_patch_json: Option<&str>) -> Result<(), Acp } } - let serialized_toml = toml::to_string_pretty(&toml_value) - .map_err(|e| AcpError::protocol(format!("serialize codex toml failed: {e}")))?; - if let Some(parent) = config_path.parent() { - fs::create_dir_all(parent).map_err(|e| { - AcpError::protocol(format!("create codex config directory failed: {e}")) - })?; - } - fs::write(&config_path, format!("{serialized_toml}\n")) - .map_err(|e| AcpError::protocol(format!("write codex config failed: {e}")))?; + let serialized_toml = format!( + "{}\n", + toml::to_string_pretty(&toml_value) + .map_err(|e| AcpError::protocol(format!("serialize codex toml failed: {e}")))? + ); let auth_path = codex_auth_json_path(); let mut auth_value = if auth_path.exists() { @@ -3448,34 +3455,58 @@ fn persist_codex_local_config(config_patch_json: Option<&str>) -> Result<(), Acp auth_obj.remove("OPENAI_API_KEY"); } } - let serialized_auth = serde_json::to_string_pretty(&auth_value) - .map_err(|e| AcpError::protocol(format!("serialize codex auth failed: {e}")))?; - if let Some(parent) = auth_path.parent() { - fs::create_dir_all(parent) - .map_err(|e| AcpError::protocol(format!("create codex auth directory failed: {e}")))?; + let serialized_auth = format!( + "{}\n", + serde_json::to_string_pretty(&auth_value) + .map_err(|e| AcpError::protocol(format!("serialize codex auth failed: {e}")))? + ); + + persist_codex_native_config_files_unlocked(Some(&serialized_auth), Some(&serialized_toml)) +} + +fn codex_atomic_write_target(path: &Path) -> std::io::Result { + match fs::symlink_metadata(path) { + Ok(metadata) if metadata.file_type().is_symlink() => fs::canonicalize(path), + Ok(_) => Ok(path.to_path_buf()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(path.to_path_buf()), + Err(error) => Err(error), + } +} + +fn write_codex_file_atomic(path: &Path, contents: &str) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; } - fs::write(&auth_path, format!("{serialized_auth}\n")) - .map_err(|e| AcpError::protocol(format!("write codex auth failed: {e}")))?; + let target = codex_atomic_write_target(path)?; + let parent = target.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("codex file has no parent: {}", target.display()), + ) + })?; + fs::create_dir_all(parent)?; + let permissions = fs::metadata(&target) + .ok() + .map(|metadata| metadata.permissions()); + let mut staged = tempfile::NamedTempFile::new_in(parent)?; + staged.write_all(contents.as_bytes())?; + if let Some(permissions) = permissions { + staged.as_file().set_permissions(permissions)?; + } + staged.as_file_mut().sync_all()?; + staged.persist(&target).map_err(|error| error.error)?; Ok(()) } -fn persist_codex_native_config_files( +fn persist_codex_native_config_files_unlocked( codex_auth_json: Option<&str>, codex_config_toml: Option<&str>, ) -> Result<(), AcpError> { if let Some(raw_toml) = codex_config_toml { toml::from_str::(raw_toml) .map_err(|e| AcpError::protocol(format!("invalid codex config.toml: {e}")))?; - let path = codex_config_toml_path(); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent) - .map_err(|e| AcpError::protocol(format!("create codex directory failed: {e}")))?; - } - fs::write(&path, raw_toml) - .map_err(|e| AcpError::protocol(format!("write codex config.toml failed: {e}")))?; } - if let Some(raw_auth) = codex_auth_json { let parsed = serde_json::from_str::(raw_auth) .map_err(|e| AcpError::protocol(format!("invalid codex auth.json: {e}")))?; @@ -3484,18 +3515,31 @@ fn persist_codex_native_config_files( "invalid codex auth.json: root must be a JSON object", )); } + } + + if let Some(raw_toml) = codex_config_toml { + let path = codex_config_toml_path(); + write_codex_file_atomic(&path, raw_toml) + .map_err(|e| AcpError::protocol(format!("write codex config.toml failed: {e}")))?; + } + + if let Some(raw_auth) = codex_auth_json { let path = codex_auth_json_path(); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent) - .map_err(|e| AcpError::protocol(format!("create codex directory failed: {e}")))?; - } - fs::write(&path, raw_auth) + write_codex_file_atomic(&path, raw_auth) .map_err(|e| AcpError::protocol(format!("write codex auth.json failed: {e}")))?; } Ok(()) } +fn persist_codex_native_config_files( + codex_auth_json: Option<&str>, + codex_config_toml: Option<&str>, +) -> Result<(), AcpError> { + let _config_guard = lock_codex_config_mutation()?; + persist_codex_native_config_files_unlocked(codex_auth_json, codex_config_toml) +} + /// Read `~/.codex/config.toml` as the base of a structured sandbox merge. /// A missing file is an empty base; a real read error fails loudly so a save can /// never silently drop the user's existing config. @@ -3672,14 +3716,20 @@ fn remove_codex_catalog_key( /// Drop codeg's own `model_catalog_json` reference from the config.toml on disk, /// if it carries one. Reads fresh so it also cleans up a key written by an /// earlier codeg version or by another window since the panel opened. -fn drop_codex_catalog_reference() -> Result<(), AcpError> { +fn drop_codex_catalog_reference_unlocked() -> Result<(), AcpError> { let base = read_codex_config_or_empty()?; if let Some(next) = remove_codex_catalog_key(&base, &codex_home_dir())? { - persist_codex_native_config_files(None, Some(&next))?; + persist_codex_native_config_files_unlocked(None, Some(&next))?; } Ok(()) } +#[cfg(test)] +fn drop_codex_catalog_reference() -> Result<(), AcpError> { + let _config_guard = lock_codex_config_mutation()?; + drop_codex_catalog_reference_unlocked() +} + /// Apply the Codex panel's sandbox / approval PATCH to the raw config.toml text, /// format-preservingly (comments and unmanaged keys are kept). Values are /// validated against the upstream vocabularies first, so a UI bug can never @@ -8269,6 +8319,32 @@ fn skill_name_from_id(id: &str) -> String { id.to_string() } +fn read_skill_frontmatter_name(content_path: &Path) -> Option { + use std::io::Read; + + let mut file = fs::File::open(content_path).ok()?; + let mut buf = [0u8; 4096]; + let n = file.read(&mut buf).ok()?; + let head = std::str::from_utf8(&buf[..n]).ok()?; + let mut lines = head.lines(); + if lines.next()?.trim() != "---" { + return None; + } + for line in lines { + let trimmed_end = line.trim_end(); + if trimmed_end == "---" || trimmed_end == "..." { + break; + } + if line.starts_with(|c: char| c.is_whitespace()) { + continue; + } + if let Some(rest) = line.strip_prefix("name:") { + return parse_frontmatter_scalar(rest); + } + } + None +} + /// Best-effort extraction of a one-line skill description from a markdown /// file's YAML frontmatter. Prefers `short-description` (commonly nested under /// a `metadata:` block) and falls back to a top-level `description`. Only the @@ -8343,14 +8419,21 @@ fn build_skill_item( scope: AgentSkillScope, layout: AgentSkillLayout, path: PathBuf, + enabled: bool, ) -> AgentSkillItem { - let description = read_skill_description(&skill_content_path(layout, &path)); + let content_path = skill_content_path(layout, &path); + let description = read_skill_description(&content_path); + let name = + read_skill_frontmatter_name(&content_path).unwrap_or_else(|| skill_name_from_id(&id)); AgentSkillItem { - name: skill_name_from_id(&id), + name, id, scope, layout, path: path.to_string_lossy().to_string(), + enabled, + can_toggle: true, + toggle_reason: None, description, read_only: false, } @@ -8372,11 +8455,22 @@ fn is_read_only_skill_path(agent_type: AgentType, skill_path: &Path) -> bool { AgentType::DeepSeek => crate::parsers::deepseek::resolve_dsh_home_dir() .join("skills") .join(".system"), + AgentType::Antigravity => { + crate::parsers::antigravity::resolve_antigravity_cli_dir().join("skills") + } _ => return false, }; skill_path.starts_with(&ro_root) } +fn apply_skill_capabilities(agent_type: AgentType, skill: &mut AgentSkillItem) { + if is_read_only_skill_path(agent_type, Path::new(&skill.path)) { + skill.read_only = true; + skill.can_toggle = false; + skill.toggle_reason = Some(AgentSkillToggleReason::ReadOnly); + } +} + fn skill_content_path(layout: AgentSkillLayout, skill_path: &Path) -> PathBuf { match layout { AgentSkillLayout::SkillDirectory => skill_path.join("SKILL.md"), @@ -8384,6 +8478,24 @@ fn skill_content_path(layout: AgentSkillLayout, skill_path: &Path) -> PathBuf { } } +fn skill_entry_layout(path: &Path, kind: SkillStorageKind) -> Option { + if matches!( + kind, + SkillStorageKind::SkillDirectoryOnly | SkillStorageKind::SkillDirectoryOrMarkdownFile + ) && path.is_dir() + && path.join("SKILL.md").is_file() + { + return Some(AgentSkillLayout::SkillDirectory); + } + if kind == SkillStorageKind::SkillDirectoryOrMarkdownFile + && path.is_file() + && is_markdown_file(path) + { + return Some(AgentSkillLayout::MarkdownFile); + } + None +} + /// Symlink-safe removal: if `path` is a symlink (to a file or directory), /// only the link itself is removed. Otherwise directories are removed /// recursively and files are unlinked. This prevents `remove_dir_all` from @@ -8438,6 +8550,35 @@ pub(crate) fn list_skills_from_dir( scope: AgentSkillScope, dir: &Path, kind: SkillStorageKind, +) -> Result, AcpError> { + list_skills_from_dir_with_state(scope, dir, kind, true) +} + +fn list_skills_from_dir_with_state( + scope: AgentSkillScope, + dir: &Path, + kind: SkillStorageKind, + enabled: bool, +) -> Result, AcpError> { + let mut by_id: BTreeMap = BTreeMap::new(); + for skill in scan_skills_from_dir_with_state(scope, dir, kind, enabled)? { + match skill.layout { + AgentSkillLayout::SkillDirectory => { + by_id.insert(skill.id.clone(), skill); + } + AgentSkillLayout::MarkdownFile => { + by_id.entry(skill.id.clone()).or_insert(skill); + } + } + } + Ok(by_id.into_values().collect()) +} + +fn scan_skills_from_dir_with_state( + scope: AgentSkillScope, + dir: &Path, + kind: SkillStorageKind, + enabled: bool, ) -> Result, AcpError> { if !dir.exists() { return Ok(Vec::new()); @@ -8446,7 +8587,7 @@ pub(crate) fn list_skills_from_dir( let entries = fs::read_dir(dir) .map_err(|e| AcpError::protocol(format!("failed to read skills directory: {e}")))?; - let mut by_id: BTreeMap = BTreeMap::new(); + let mut skills = Vec::new(); for entry in entries { let entry = match entry { Ok(value) => value, @@ -8456,44 +8597,129 @@ pub(crate) fn list_skills_from_dir( let file_name = entry.file_name(); let id = file_name.to_string_lossy().to_string(); - if path.is_dir() - && matches!( - kind, - SkillStorageKind::SkillDirectoryOnly - | SkillStorageKind::SkillDirectoryOrMarkdownFile - ) - { - let skill_doc = path.join("SKILL.md"); - if !skill_doc.is_file() { - continue; + match skill_entry_layout(&path, kind) { + Some(AgentSkillLayout::SkillDirectory) => { + skills.push(build_skill_item( + id, + scope, + AgentSkillLayout::SkillDirectory, + path, + enabled, + )); } - by_id.insert( - id.clone(), - build_skill_item(id, scope, AgentSkillLayout::SkillDirectory, path), - ); - continue; + Some(AgentSkillLayout::MarkdownFile) => { + let stem = path + .file_stem() + .and_then(|s| s.to_str()) + .map(str::to_string) + .unwrap_or_else(|| id.clone()); + skills.push(build_skill_item( + stem, + scope, + AgentSkillLayout::MarkdownFile, + path, + enabled, + )); + } + None => {} } + } + skills.sort_by(|left, right| left.path.cmp(&right.path)); + Ok(skills) +} - if path.is_file() - && matches!(kind, SkillStorageKind::SkillDirectoryOrMarkdownFile) - && is_markdown_file(&path) - { - let stem = path - .file_stem() - .and_then(|s| s.to_str()) - .map(str::to_string) - .unwrap_or_else(|| id.clone()); - if by_id.contains_key(&stem) { +pub(crate) fn disabled_skill_root(active_root: &Path) -> PathBuf { + let cursor_builtin_root = home_dir_or_default().join(".cursor").join("skills-cursor"); + let vault_name = if active_root == cursor_builtin_root { + ".skills-cursor.codeg-disabled" + } else { + // Keep the original location for existing disabled entries. + ".skills.codeg-disabled" + }; + active_root + .parent() + .unwrap_or_else(|| Path::new("")) + .join(vault_name) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SkillResidence { + Active, + PrivateVault, + LegacyVault, +} + +#[derive(Debug, Clone)] +struct PlannedSkill { + item: AgentSkillItem, + plan_index: usize, + residence: SkillResidence, +} + +#[derive(Debug, Default)] +struct SkillInventory { + listed: Vec, + active_by_id: BTreeMap>, +} + +fn list_skills_from_plans( + scope: AgentSkillScope, + plans: &[SkillRootPlan], + kind: SkillStorageKind, +) -> Result { + let mut by_id: BTreeMap = BTreeMap::new(); + let mut active_by_id: BTreeMap> = BTreeMap::new(); + + for residence in [ + SkillResidence::Active, + SkillResidence::PrivateVault, + SkillResidence::LegacyVault, + ] { + let mut seen_roots = HashSet::new(); + for (plan_index, plan) in plans.iter().enumerate() { + let (dir, enabled) = match residence { + SkillResidence::Active => (&plan.active, true), + SkillResidence::PrivateVault => (&plan.vault, false), + SkillResidence::LegacyVault => (&plan.legacy_vault, false), + }; + let resolved_dir = match residence { + SkillResidence::Active => plan.resolved_active.clone(), + SkillResidence::PrivateVault => plan.resolved_vault.clone(), + SkillResidence::LegacyVault => resolved_skill_root(&plan.legacy_vault)?, + }; + if !seen_roots.insert(resolved_dir) { continue; } - by_id.insert( - stem.clone(), - build_skill_item(stem, scope, AgentSkillLayout::MarkdownFile, path), - ); + for skill in scan_skills_from_dir_with_state(scope, dir, kind, enabled)? { + if residence == SkillResidence::Active { + active_by_id + .entry(skill.id.clone()) + .or_default() + .push(skill.clone()); + } + by_id.entry(skill.id.clone()).or_insert(PlannedSkill { + item: skill, + plan_index, + residence, + }); + } } } - Ok(by_id.into_values().collect()) + Ok(SkillInventory { + listed: by_id.into_values().collect(), + active_by_id, + }) +} + +fn locate_skill_in_inventory<'a>( + inventory: &'a SkillInventory, + skill_id: &str, +) -> Option<&'a PlannedSkill> { + inventory + .listed + .iter() + .find(|skill| skill.item.id == skill_id) } fn locate_existing_skill( @@ -8501,6 +8727,7 @@ fn locate_existing_skill( kind: SkillStorageKind, skill_id: &str, scope: AgentSkillScope, + enabled: bool, ) -> Option { if matches!( kind, @@ -8513,6 +8740,7 @@ fn locate_existing_skill( scope, AgentSkillLayout::SkillDirectory, skill_dir, + enabled, )); } } @@ -8525,6 +8753,7 @@ fn locate_existing_skill( scope, AgentSkillLayout::MarkdownFile, file_path, + enabled, )); } } @@ -8539,1447 +8768,2964 @@ pub(crate) fn locate_existing_skill_across_dirs( scope: AgentSkillScope, ) -> Option { for dir in dirs { - if let Some(found) = locate_existing_skill(dir, kind, skill_id, scope) { + if let Some(found) = locate_existing_skill(dir, kind, skill_id, scope, true) { + return Some(found); + } + } + for dir in dirs { + if let Some(found) = + locate_existing_skill(&disabled_skill_root(dir), kind, skill_id, scope, false) + { return Some(found); } } None } -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(rename_all = "camelCase")] -struct AgentRuntimeConfig { - #[serde(default, alias = "api_base_url")] - api_base_url: Option, - #[serde(default, alias = "api_key")] - api_key: Option, - #[serde(default)] - model: Option, - #[serde(default, deserialize_with = "deserialize_env_strings")] - env: BTreeMap, +#[derive(Debug)] +struct CodexSkillConfig { + entries: Vec, + bundled_enabled: bool, + enabled_plugins: Vec, } -/// Read an agent config's `env` map, skipping entries whose value is not a -/// string instead of failing the whole parse. -/// -/// Both readers of this struct discard the ENTIRE local config on a parse error -/// (`build_runtime_env_from_setting` and the agent-info env projection), so one -/// odd value would silently cost the launch env its base URL and key too. The -/// value seen in the wild is `null` — written by an older -/// [`merge_json_values`], see [`patch_addition`] — but a hand-edited number or -/// bool deserves the same containment. -fn deserialize_env_strings<'de, D>(deserializer: D) -> Result, D::Error> -where - D: serde::Deserializer<'de>, -{ - let raw = BTreeMap::::deserialize(deserializer)?; - Ok(raw - .into_iter() - .filter_map(|(key, value)| match value { - serde_json::Value::String(value) => Some((key, value)), - _ => None, - }) - .collect()) +#[derive(Debug)] +struct CodexSkillConfigEntry { + path: Option, + name: Option, + enabled: bool, } -fn trim_non_empty(value: Option) -> Option { - value.and_then(|raw| { - let trimmed = raw.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } - }) +#[derive(Debug, Clone, PartialEq, Eq)] +struct CodexPluginRef { + name: String, + marketplace: String, } -// --------------------------------------------------------------------------- -// Cursor settings panel (cli-config.json + auth / models probes) -// --------------------------------------------------------------------------- - -fn cursor_cli_config_path() -> PathBuf { - crate::parsers::cursor::resolve_cursor_config_dir().join("cli-config.json") +impl Default for CodexSkillConfig { + fn default() -> Self { + Self { + entries: Vec::new(), + bundled_enabled: true, + enabled_plugins: Vec::new(), + } + } } -fn load_cursor_cli_config_raw() -> Option { - fs::read_to_string(cursor_cli_config_path()).ok() +fn resolve_codex_skill_config_path(raw: &str, codex_home: &Path) -> PathBuf { + if raw == "~" { + return home_dir_or_default(); + } + if let Some(relative) = raw.strip_prefix("~/") { + return home_dir_or_default().join(relative); + } + let path = PathBuf::from(raw); + if path.is_absolute() { + path + } else { + codex_home.join(path) + } } -/// Project the structured controls out of a raw cli-config.json text. -/// Malformed JSON yields defaults (the panel shows the raw text separately). -pub(crate) fn parse_cursor_settings(raw: &str) -> crate::acp::types::CursorSettings { - let Ok(v) = serde_json::from_str::(raw) else { - return crate::acp::types::CursorSettings::default(); - }; - let string_list = |val: Option<&serde_json::Value>| -> Vec { - val.and_then(|v| v.as_array()) - .map(|items| { - items - .iter() - .filter_map(|i| i.as_str()) - .map(str::to_string) - .collect() +fn absolute_skill_content_path(skill: &AgentSkillItem) -> Result { + let path = skill_content_path(skill.layout, Path::new(&skill.path)); + if path.is_absolute() { + Ok(path) + } else { + std::env::current_dir() + .map(|cwd| cwd.join(path)) + .map_err(|error| { + AcpError::protocol(format!("failed to resolve current directory: {error}")) }) - .unwrap_or_default() - }; - crate::acp::types::CursorSettings { - sandbox_mode: v - .pointer("/sandbox/mode") - .and_then(serde_json::Value::as_str) - .map(str::to_string), - permissions_allow: string_list(v.pointer("/permissions/allow")), - permissions_deny: string_list(v.pointer("/permissions/deny")), } } -/// Merge the Cursor panel's structured controls into the raw cli-config.json -/// text. Only the managed keys are touched; every other key (editor prefs, -/// hints, network, …) is preserved verbatim. `None` fields leave their key -/// as-is; `Some` fields replace it (lists wholesale). -fn apply_cursor_structured_config( - base: &str, - patch: &crate::acp::types::CursorStructuredConfig, -) -> Result { - let mut root: serde_json::Value = if base.trim().is_empty() { - serde_json::json!({}) - } else { - serde_json::from_str(base) - .map_err(|e| AcpError::protocol(format!("invalid cursor cli-config.json: {e}")))? - }; - if !root.is_object() { - return Err(AcpError::protocol( - "invalid cursor cli-config.json: root must be a JSON object", - )); - } - let obj = root.as_object_mut().expect("checked object"); +fn same_skill_config_path(first: &Path, second: &Path) -> bool { + first == second + || fs::canonicalize(first) + .ok() + .zip(fs::canonicalize(second).ok()) + .is_some_and(|(first, second)| first == second) +} - // Drop the legacy `approvalMode` key an earlier panel version wrote: the - // CLI never reads it from cli-config.json (approval mode lives in each - // chat's store.db metadata, seeded by the `--force`/`--auto-review` - // launch flags), so leaving it around only misleads whoever inspects the - // file. - obj.remove("approvalMode"); - if let Some(mode) = &patch.sandbox_mode { - let sandbox = obj - .entry("sandbox") - .or_insert_with(|| serde_json::json!({})); - if let Some(sandbox_obj) = sandbox.as_object_mut() { - if mode.trim().is_empty() { - sandbox_obj.remove("mode"); - } else { - sandbox_obj.insert("mode".into(), serde_json::Value::String(mode.clone())); - } +fn parse_codex_skill_config( + raw_toml: &str, + codex_home: &Path, +) -> Result { + let root = raw_toml + .parse::() + .map_err(|error| AcpError::protocol(format!("invalid codex config.toml: {error}")))?; + let skills = root.get("skills").and_then(toml::Value::as_table); + let bundled_enabled = skills + .and_then(|skills| skills.get("bundled")) + .and_then(toml::Value::as_table) + .and_then(|bundled| bundled.get("enabled")) + .map(|enabled| { + enabled.as_bool().ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: skills.bundled.enabled must be a boolean", + ) + }) + }) + .transpose()? + .unwrap_or(true); + + let mut entries = Vec::new(); + if let Some(config) = skills.and_then(|skills| skills.get("config")) { + let config = config.as_array().ok_or_else(|| { + AcpError::protocol("invalid codex config.toml: skills.config must be an array") + })?; + entries.reserve(config.len()); + for entry in config { + let table = entry.as_table().ok_or_else(|| { + AcpError::protocol("invalid codex config.toml: skills.config entry must be a table") + })?; + let path = match table.get("path") { + Some(value) => Some( + value + .as_str() + .map(|path| resolve_codex_skill_config_path(path, codex_home)) + .ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: skills.config path must be a string", + ) + })?, + ), + None => None, + }; + let name = match table.get("name") { + Some(value) => Some(value.as_str().map(str::to_string).ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: skills.config name must be a string", + ) + })?), + None => None, + }; + let enabled = table + .get("enabled") + .ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: skills.config enabled is required", + ) + })? + .as_bool() + .ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: skills.config enabled must be a boolean", + ) + })?; + entries.push(CodexSkillConfigEntry { + path, + name, + enabled, + }); } } - let mut set_rules = |key: &str, rules: &Option>| { - if let Some(rules) = rules { - let permissions = obj - .entry("permissions") - .or_insert_with(|| serde_json::json!({})); - if let Some(perm_obj) = permissions.as_object_mut() { - let cleaned: Vec = rules - .iter() - .map(|r| r.trim()) - .filter(|r| !r.is_empty()) - .map(|r| serde_json::Value::String(r.to_string())) - .collect(); - perm_obj.insert(key.to_string(), serde_json::Value::Array(cleaned)); + + let mut enabled_plugins = Vec::new(); + if let Some(plugins) = root.get("plugins").and_then(toml::Value::as_table) { + for (qualified_name, value) in plugins { + let enabled = value + .as_table() + .and_then(|plugin| plugin.get("enabled")) + .and_then(toml::Value::as_bool) + .unwrap_or(false); + if !enabled { + continue; + } + let Some((name, marketplace)) = qualified_name.rsplit_once('@') else { + continue; + }; + if !name.is_empty() && !marketplace.is_empty() { + enabled_plugins.push(CodexPluginRef { + name: name.to_string(), + marketplace: marketplace.to_string(), + }); } } - }; - set_rules("allow", &patch.permissions_allow); - set_rules("deny", &patch.permissions_deny); + } + enabled_plugins.sort_by(|left, right| { + left.name + .cmp(&right.name) + .then_with(|| left.marketplace.cmp(&right.marketplace)) + }); + enabled_plugins.dedup(); - serde_json::to_string_pretty(&root) - .map_err(|e| AcpError::protocol(format!("serialize cursor cli-config failed: {e}"))) + Ok(CodexSkillConfig { + entries, + bundled_enabled, + enabled_plugins, + }) } -/// Validate + write cli-config.json (whole-document; the merge already -/// preserved unmanaged keys). -fn persist_cursor_cli_config(text: &str) -> Result<(), AcpError> { - let parsed = serde_json::from_str::(text) - .map_err(|e| AcpError::protocol(format!("invalid cursor cli-config.json: {e}")))?; - if !parsed.is_object() { - return Err(AcpError::protocol( - "invalid cursor cli-config.json: root must be a JSON object", - )); - } - let path = cursor_cli_config_path(); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent) - .map_err(|e| AcpError::protocol(format!("create cursor config dir failed: {e}")))?; +impl CodexSkillConfig { + fn skill_enabled(&self, path: &Path, name: &str) -> bool { + self.entries.iter().fold(true, |enabled, entry| { + if entry.path.is_some() == entry.name.is_some() { + return enabled; + } + let path_matches = entry + .path + .as_deref() + .is_some_and(|configured| same_skill_config_path(configured, path)); + let name_matches = entry.name.as_deref() == Some(name); + if path_matches || name_matches { + entry.enabled + } else { + enabled + } + }) } - fs::write(&path, format!("{text}\n")) - .map_err(|e| AcpError::protocol(format!("write cursor cli-config failed: {e}"))) } -// --------------------------------------------------------------------------- -// Qoder settings.json helpers -// --------------------------------------------------------------------------- - -/// Validate + write settings.json, whole-document. -/// -/// The text is whatever the panel's advanced editor holds, which is the file as -/// codeg last read it plus the user's edits — writing it verbatim is what lets -/// a key be DELETED, which the generic merge-persist path cannot do. -/// -/// Note this file has other writers (the Qoder CLI itself, and codeg's MCP -/// settings page, which owns the top-level `mcpServers`). A verbatim write -/// therefore reverts anything they wrote since the editor last loaded — the -/// same last-writer-wins contract every raw editor in this module has. -fn persist_qoder_settings(text: &str) -> Result<(), AcpError> { - let parsed = serde_json::from_str::(text) - .map_err(|e| AcpError::protocol(format!("invalid qoder settings.json: {e}")))?; - if !parsed.is_object() { - return Err(AcpError::protocol( - "invalid qoder settings.json: root must be a JSON object", - )); - } - let path = qoder_settings_json_path(); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent) - .map_err(|e| AcpError::protocol(format!("create qoder config dir failed: {e}")))?; +fn codex_skill_entries_enabled( + entries: &[AgentSkillItem], + config: &CodexSkillConfig, +) -> Result { + for skill in entries { + if config.skill_enabled(&absolute_skill_content_path(skill)?, &skill.name) { + return Ok(true); + } } - fs::write(&path, format!("{text}\n")) - .map_err(|e| AcpError::protocol(format!("write qoder settings failed: {e}"))) + Ok(false) } -/// The `qoder` binary codeg would launch: managed cache first, then the user's -/// own install (PATH / ~/.local/bin) — the same order as `build_agent`. -fn resolve_qoder_binary() -> Option { - if let Ok(Some((path, _))) = - binary_cache::find_best_cached_binary_for_agent(AgentType::Qoder, "qoder") - { - return Some(path); +fn update_codex_array_of_tables( + config: &mut toml_edit::ArrayOfTables, + codex_home: &Path, + skill_name: &str, + paths: &[PathBuf], + enabled: bool, +) { + for path in paths { + let mut last_matching_path = None; + for (index, entry) in config.iter().enumerate() { + let has_path = entry.get("path").is_some(); + let has_name = entry.get("name").is_some(); + if has_path == has_name { + continue; + } + let path_matches = entry + .get("path") + .and_then(toml_edit::Item::as_str) + .map(|configured| resolve_codex_skill_config_path(configured, codex_home)) + .is_some_and(|configured| same_skill_config_path(&configured, path)); + let name_matches = + entry.get("name").and_then(toml_edit::Item::as_str) == Some(skill_name); + if path_matches || name_matches { + last_matching_path = path_matches.then_some(index); + } + } + if let Some(index) = last_matching_path { + config + .get_mut(index) + .expect("matching Codex skill config entry must exist") + .insert("enabled", toml_edit::value(enabled)); + } else { + let mut entry = toml_edit::Table::new(); + entry.insert("path", toml_edit::value(path.to_string_lossy().as_ref())); + entry.insert("enabled", toml_edit::value(enabled)); + config.push(entry); + } } - resolve_system_agent_binary("qoder") } -/// The Qoder agent's effective probe env: the saved env with the settings -/// form's live personal access token applied on top, so `status` reports on the -/// credential that is on screen rather than a stale saved one. -/// -/// `PAT` is always materialized (empty when unset) so `run_qoder_probe` makes -/// an explicit set-or-remove decision — an inherited token from the user's dev -/// shell must not make the card claim an account that a launch would not use. -async fn qoder_probe_env(db: &AppDatabase, personal_access_token: Option<&str>) -> BTreeMap { - let mut env: BTreeMap = - agent_setting_service::get_by_agent_type(&db.conn, AgentType::Qoder) - .await - .ok() - .flatten() - .and_then(|m| m.env_json) - .and_then(|raw| serde_json::from_str::>(&raw).ok()) - .unwrap_or_default(); - if let Some(token) = personal_access_token { - env.insert( - "QODER_PERSONAL_ACCESS_TOKEN".to_string(), - token.trim().to_string(), - ); +fn update_codex_inline_array( + config: &mut toml_edit::Array, + codex_home: &Path, + skill_name: &str, + paths: &[PathBuf], + enabled: bool, +) -> Result<(), AcpError> { + for path in paths { + let mut last_matching_path = None; + for (index, value) in config.iter().enumerate() { + let entry = value.as_inline_table().ok_or_else(|| { + AcpError::protocol("invalid codex config.toml: skills.config entry must be a table") + })?; + let has_path = entry.get("path").is_some(); + let has_name = entry.get("name").is_some(); + if has_path == has_name { + continue; + } + let path_matches = entry + .get("path") + .and_then(toml_edit::Value::as_str) + .map(|configured| resolve_codex_skill_config_path(configured, codex_home)) + .is_some_and(|configured| same_skill_config_path(&configured, path)); + let name_matches = + entry.get("name").and_then(toml_edit::Value::as_str) == Some(skill_name); + if path_matches || name_matches { + last_matching_path = path_matches.then_some(index); + } + } + if let Some(index) = last_matching_path { + config + .get_mut(index) + .and_then(toml_edit::Value::as_inline_table_mut) + .expect("matching Codex skill config entry must be an inline table") + .insert("enabled", toml_edit::Value::from(enabled)); + } else { + let mut entry = toml_edit::InlineTable::new(); + entry.insert( + "path", + toml_edit::Value::from(path.to_string_lossy().as_ref()), + ); + entry.insert("enabled", toml_edit::Value::from(enabled)); + config.push(toml_edit::Value::InlineTable(entry)); + } } - env.entry("QODER_PERSONAL_ACCESS_TOKEN".to_string()) - .or_default(); - env + Ok(()) } -/// Run a `qoder` subcommand with a timeout, capturing stdout. -async fn run_qoder_probe( - args: &[&str], - timeout_secs: u64, - extra_env: &BTreeMap, -) -> Result { - let bin = resolve_qoder_binary().ok_or_else(|| "qoder is not installed".to_string())?; - let mut cmd = crate::process::tokio_command(&bin); - cmd.args(args); - for (key, value) in extra_env { - if value.trim().is_empty() { - // This process's env is inherited by the child; an empty value means - // "ensure absent" so a stale inherited token can't leak in. - cmd.env_remove(key); - } else { - cmd.env(key, value); +fn apply_codex_skill_enabled_config( + base_toml: &str, + codex_home: &Path, + skill_name: &str, + paths: &[PathBuf], + enabled: bool, +) -> Result { + parse_codex_skill_config(base_toml, codex_home)?; + let mut doc = base_toml + .parse::() + .map_err(|error| AcpError::protocol(format!("invalid codex config.toml: {error}")))?; + // Match Codex's native `skills/config/write`: persist the resolved + // SKILL.md target so symlink aliases collapse to one stable rule. + let mut unique_paths = paths + .iter() + .map(|path| fs::canonicalize(path).unwrap_or_else(|_| path.clone())) + .collect::>(); + unique_paths.sort(); + unique_paths.dedup(); + + if doc.get("skills").is_none() { + doc["skills"] = toml_edit::Item::Table(toml_edit::Table::new()); + } + let skills = doc + .get_mut("skills") + .ok_or_else(|| AcpError::protocol("invalid codex config.toml: missing skills"))?; + match skills { + toml_edit::Item::Table(skills) => { + if skills.get("config").is_none() { + skills.insert( + "config", + toml_edit::Item::ArrayOfTables(toml_edit::ArrayOfTables::new()), + ); + } + match skills + .get_mut("config") + .expect("skills.config inserted above") + { + toml_edit::Item::ArrayOfTables(config) => update_codex_array_of_tables( + config, + codex_home, + skill_name, + &unique_paths, + enabled, + ), + toml_edit::Item::Value(toml_edit::Value::Array(config)) => { + update_codex_inline_array( + config, + codex_home, + skill_name, + &unique_paths, + enabled, + )?; + } + _ => { + return Err(AcpError::protocol( + "invalid codex config.toml: skills.config must be an array of tables", + )) + } + } + } + toml_edit::Item::Value(toml_edit::Value::InlineTable(skills)) => { + if skills.get("config").is_none() { + skills.insert("config", toml_edit::Value::Array(toml_edit::Array::new())); + } + let config = skills + .get_mut("config") + .and_then(toml_edit::Value::as_array_mut) + .ok_or_else(|| { + AcpError::protocol( + "invalid codex config.toml: inline skills.config must be an array", + ) + })?; + update_codex_inline_array(config, codex_home, skill_name, &unique_paths, enabled)?; + } + _ => { + return Err(AcpError::protocol( + "invalid codex config.toml: skills must be a table", + )) } } - let output = tokio::time::timeout(std::time::Duration::from_secs(timeout_secs), cmd.output()) - .await - .map_err(|_| format!("qoder {} timed out", args.join(" ")))? - .map_err(|e| format!("failed to run qoder: {e}"))?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - if !output.status.success() && stdout.trim().is_empty() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(format!("qoder {} failed: {}", args.join(" "), stderr.trim())); - } - Ok(stdout) + Ok(doc.to_string()) } -/// Probe `qoder status -o json` for the Qoder settings panel's auth card. -/// -/// The CLI prints one flat object — `{logged_in, version, allow_byok, username, -/// email, avatar_url, user_type}` — so unlike the Cursor probe there is no -/// nested `userInfo` to unwrap. A parse failure is reported as `error` with -/// `logged_in: false`; the panel renders that as "could not check", not as -/// "signed out", so a CLI output change never reads as a lost session. -pub(crate) async fn acp_qoder_auth_status_core( - db: &AppDatabase, - personal_access_token: Option, -) -> crate::acp::types::QoderAuthStatus { - let binary_path = resolve_qoder_binary().map(|p| p.to_string_lossy().to_string()); - if binary_path.is_none() { - return crate::acp::types::QoderAuthStatus { - installed: false, - logged_in: false, - username: None, - email: None, - user_type: None, - version: None, - allow_byok: None, - error: None, - binary_path: None, - }; - } - let extra_env = qoder_probe_env(db, personal_access_token.as_deref()).await; - let failed = |error: Option| crate::acp::types::QoderAuthStatus { - installed: true, - logged_in: false, - username: None, - email: None, - user_type: None, - version: None, - allow_byok: None, - error, - binary_path: binary_path.clone(), +fn remove_codex_disabled_skill_path_rules( + base_toml: &str, + codex_home: &Path, + paths: &[PathBuf], +) -> Result, AcpError> { + parse_codex_skill_config(base_toml, codex_home)?; + let mut doc = base_toml + .parse::() + .map_err(|error| AcpError::protocol(format!("invalid codex config.toml: {error}")))?; + let matches_path = |raw: &str| { + let configured = resolve_codex_skill_config_path(raw, codex_home); + paths + .iter() + .any(|path| same_skill_config_path(&configured, path)) }; - match run_qoder_probe(&["status", "-o", "json"], 30, &extra_env).await { - Ok(stdout) => { - // Scan to the first `{` so a leading log/update-notice line can't - // break parsing. - let json_start = stdout.find('{').unwrap_or(0); - match serde_json::from_str::(stdout[json_start..].trim()) { - Ok(v) => { - let get_str = |key: &str| { - v.get(key) - .and_then(serde_json::Value::as_str) - .filter(|s| !s.is_empty()) - .map(str::to_string) - }; - crate::acp::types::QoderAuthStatus { - installed: true, - logged_in: v - .get("logged_in") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false), - username: get_str("username"), - email: get_str("email"), - user_type: get_str("user_type"), - version: get_str("version"), - // The CLI emits 0/1 here rather than a JSON boolean. - allow_byok: v - .get("allow_byok") - .and_then(|b| { - b.as_bool().or_else(|| b.as_i64().map(|n| n != 0)) - }), - error: None, - binary_path: binary_path.clone(), - } + let Some(skills) = doc.get_mut("skills") else { + return Ok(None); + }; + let mut removed = false; + + match skills { + toml_edit::Item::Table(skills) => { + let Some(config) = skills.get_mut("config") else { + return Ok(None); + }; + match config { + toml_edit::Item::ArrayOfTables(config) => config.retain(|entry| { + let remove = entry.get("name").is_none() + && entry + .get("path") + .and_then(toml_edit::Item::as_str) + .is_some_and(&matches_path) + && entry.get("enabled").and_then(toml_edit::Item::as_bool) == Some(false); + removed |= remove; + !remove + }), + toml_edit::Item::Value(toml_edit::Value::Array(config)) => { + config.retain(|value| { + let remove = value.as_inline_table().is_some_and(|entry| { + entry.get("name").is_none() + && entry + .get("path") + .and_then(toml_edit::Value::as_str) + .is_some_and(&matches_path) + && entry.get("enabled").and_then(toml_edit::Value::as_bool) + == Some(false) + }); + removed |= remove; + !remove + }); + } + _ => { + return Err(AcpError::protocol( + "invalid codex config.toml: skills.config must be an array of tables", + )) } - Err(e) => crate::acp::types::QoderAuthStatus { - error: Some(format!( - "unexpected status output: {e}: {}", - truncate_probe_output(&stdout) - )), - ..failed(None) - }, } } - Err(err) => failed(Some(err)), - } -} - -/// The cursor-agent binary codeg would launch: managed cache first, then the -/// user's own install (PATH / ~/.local/bin) — the same order as `build_agent`. -fn resolve_cursor_binary() -> Option { - if let Ok(Some((path, _))) = - binary_cache::find_best_cached_binary_for_agent(AgentType::Cursor, "cursor-agent") - { - return Some(path); + toml_edit::Item::Value(toml_edit::Value::InlineTable(skills)) => { + let Some(config) = skills + .get_mut("config") + .and_then(toml_edit::Value::as_array_mut) + else { + return Ok(None); + }; + config.retain(|value| { + let remove = value.as_inline_table().is_some_and(|entry| { + entry.get("name").is_none() + && entry + .get("path") + .and_then(toml_edit::Value::as_str) + .is_some_and(&matches_path) + && entry.get("enabled").and_then(toml_edit::Value::as_bool) == Some(false) + }); + removed |= remove; + !remove + }); + } + _ => { + return Err(AcpError::protocol( + "invalid codex config.toml: skills must be a table", + )) + } } - resolve_system_agent_binary("cursor-agent") -} -/// The Cursor agent's effective probe env: the saved env (env_json) with the -/// settings form's live API key applied on top, so `status` / `models` test -/// exactly the credential on screen rather than a stale saved value. -/// -/// `api_key` is the form value — `Some(key)` in API-key mode, `Some("")` in -/// subscription mode to force (and verify) the browser-login credential. -/// `CURSOR_API_KEY` is always materialized (empty when unset) so -/// `run_cursor_probe` makes an explicit set-or-remove decision and a stale -/// inherited key can never leak in and produce a bogus "invalid API key". -/// `CURSOR_API_BASE_URL` is always cleared — the CLI has no custom-endpoint -/// support, so a base URL is never a valid probe input. -async fn cursor_probe_env(db: &AppDatabase, api_key: Option<&str>) -> BTreeMap { - let mut env: BTreeMap = - agent_setting_service::get_by_agent_type(&db.conn, AgentType::Cursor) - .await - .ok() - .flatten() - .and_then(|m| m.env_json) - .and_then(|raw| serde_json::from_str::>(&raw).ok()) - .unwrap_or_default(); - if let Some(key) = api_key { - env.insert("CURSOR_API_KEY".to_string(), key.trim().to_string()); - } - // Materialize the key so an unset one becomes an explicit empty ⇒ removed. - env.entry("CURSOR_API_KEY".to_string()).or_default(); - // Scrub any stale base URL (legacy env_json row or inherited dev-shell - // export): empty ⇒ removed by run_cursor_probe. - env.insert("CURSOR_API_BASE_URL".to_string(), String::new()); - env + Ok(removed.then(|| doc.to_string())) } -/// Run a cursor-agent subcommand with a timeout, capturing stdout. -async fn run_cursor_probe( - args: &[&str], - timeout_secs: u64, - extra_env: &BTreeMap, -) -> Result { - let bin = resolve_cursor_binary().ok_or_else(|| "cursor-agent is not installed".to_string())?; - let mut cmd = crate::process::tokio_command(&bin); - cmd.args(args); - for (key, value) in extra_env { - if value.trim().is_empty() { - // This process's env is inherited by the child; an empty value means - // "ensure absent" so a stale inherited CURSOR_API_KEY can't leak in. - cmd.env_remove(key); - } else { - cmd.env(key, value); - } +fn set_codex_skill_enabled_native( + mut listed: AgentSkillItem, + active: &[AgentSkillItem], + enabled: bool, +) -> Result { + let _config_guard = lock_codex_config_mutation()?; + let codex_home = codex_home_dir(); + let base = read_codex_config_or_empty()?; + let current = parse_codex_skill_config(&base, &codex_home)?; + listed.enabled = codex_skill_entries_enabled(active, ¤t)?; + listed.can_toggle = true; + if listed.enabled == enabled { + return Ok(listed); } - let output = tokio::time::timeout( - std::time::Duration::from_secs(timeout_secs), - cmd.output(), - ) - .await - .map_err(|_| format!("cursor-agent {} timed out", args.join(" ")))? - .map_err(|e| format!("failed to run cursor-agent: {e}"))?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - if !output.status.success() && stdout.trim().is_empty() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(format!( - "cursor-agent {} failed: {}", - args.join(" "), - stderr.trim() + let paths = active + .iter() + .map(absolute_skill_content_path) + .collect::, _>>()?; + let next = apply_codex_skill_enabled_config(&base, &codex_home, &listed.name, &paths, enabled)?; + let updated = parse_codex_skill_config(&next, &codex_home)?; + if codex_skill_entries_enabled(active, &updated)? != enabled { + return Err(AcpError::protocol( + "requested Codex skill state was not reached", )); } - Ok(stdout) + persist_codex_native_config_files_unlocked(None, Some(&next))?; + listed.enabled = enabled; + Ok(listed) } -pub(crate) async fn acp_cursor_auth_status_core( - db: &AppDatabase, - api_key: Option, -) -> crate::acp::types::CursorAuthStatus { - let binary_path = resolve_cursor_binary().map(|p| p.to_string_lossy().to_string()); - if binary_path.is_none() { - return crate::acp::types::CursorAuthStatus { - installed: false, - is_authenticated: false, - raw_status: None, - email: None, - membership: None, - error: None, - binary_path: None, - }; - } - let extra_env = cursor_probe_env(db, api_key.as_deref()).await; - match run_cursor_probe(&["status", "--format", "json"], 20, &extra_env).await { - Ok(stdout) => { - // The CLI prints one JSON object; scan to the first `{` so a - // leading log line can't break parsing. - let json_start = stdout.find('{').unwrap_or(0); - match serde_json::from_str::(stdout[json_start..].trim()) { - Ok(v) => { - let get_str = |keys: &[&str]| { - keys.iter().find_map(|k| { - v.get(*k) - .and_then(serde_json::Value::as_str) - .filter(|s| !s.is_empty()) - .map(str::to_string) - }) - }; - // Email is nested under `userInfo` in current CLI output; - // fall back to a top-level field for forward-compatibility. - let email = v - .get("userInfo") - .and_then(|u| u.get("email")) - .and_then(serde_json::Value::as_str) - .filter(|s| !s.is_empty()) - .map(str::to_string) - .or_else(|| get_str(&["email", "userEmail", "user_email"])); - crate::acp::types::CursorAuthStatus { - installed: true, - is_authenticated: v - .get("isAuthenticated") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false), - raw_status: get_str(&["status", "message"]), - email, - membership: get_str(&["membershipType", "membership", "plan"]), - error: None, - binary_path: binary_path.clone(), - } +// All settings mutations share this lock so lookup, preflight and mutation +// observe one state. No guard is held across an await. +static SKILL_MUTATION_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +fn resolved_skill_root(root: &Path) -> Result { + let absolute = if root.is_absolute() { + root.to_path_buf() + } else { + std::env::current_dir() + .map_err(|e| AcpError::protocol(format!("failed to resolve current directory: {e}")))? + .join(root) + }; + let mut ancestor = absolute.as_path(); + let mut suffix = Vec::new(); + loop { + match fs::canonicalize(ancestor) { + Ok(mut resolved) => { + for component in suffix.into_iter().rev() { + resolved.push(component); } - Err(e) => crate::acp::types::CursorAuthStatus { - installed: true, - is_authenticated: false, - raw_status: Some(truncate_probe_output(&stdout)), - email: None, - membership: None, - error: Some(format!("unexpected status output: {e}")), - binary_path: binary_path.clone(), - }, + return Ok(resolved); + } + Err(error) + if error.kind() == std::io::ErrorKind::NotFound + && fs::symlink_metadata(ancestor).is_err() => + { + let Some(name) = ancestor.file_name() else { + return Err(AcpError::protocol(format!( + "failed to resolve skill root '{}': {error}", + root.display() + ))); + }; + suffix.push(name.to_os_string()); + ancestor = ancestor + .parent() + .ok_or_else(|| AcpError::protocol("skill root has no parent"))?; + } + Err(error) => { + return Err(AcpError::protocol(format!( + "failed to resolve skill root '{}': {error}", + root.display() + ))) } } - Err(err) => crate::acp::types::CursorAuthStatus { - installed: true, - is_authenticated: false, - raw_status: None, - email: None, - membership: None, - error: Some(err), - binary_path, - }, } } -pub(crate) async fn acp_cursor_list_models_core( - db: &AppDatabase, - api_key: Option, -) -> crate::acp::types::CursorModelsResult { - let extra_env = cursor_probe_env(db, api_key.as_deref()).await; - match run_cursor_probe(&["models"], 30, &extra_env).await { - Ok(stdout) => { - let (models, default_model) = parse_cursor_models(&stdout); - crate::acp::types::CursorModelsResult { - models, - default_model, - error: None, +/// Resolve a path against the filesystem as it would look after one rename. +/// Destination entries are inspected at their source paths, while symlink +/// targets are interpreted from their future parents. The old entry is absent. +fn resolve_skill_path_after_move( + path: &Path, + source: &Path, + destination: &Path, + directory: bool, + followed_links: usize, +) -> std::io::Result { + if followed_links > 40 { + return Err(std::io::Error::other("too many skill symlinks")); + } + let mut resolved = PathBuf::new(); + let mut components = path.components(); + while let Some(component) = components.next() { + match component { + std::path::Component::CurDir => continue, + std::path::Component::ParentDir => { + resolved.pop(); + continue; } + _ => resolved.push(component.as_os_str()), } - Err(err) => crate::acp::types::CursorModelsResult { - models: Vec::new(), - default_model: None, - error: Some(err), - }, - } -} - -/// Parse `cursor-agent models` output. Each model line is -/// ` -