You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds a global capability-preference abstraction, initially used only for skills. This keeps mutable user preferences out of Skill.Info and leaves permissions, runtime health, and visibility as separate concerns.
A capability is identified by a typed reference. The only supported kind in this PR is:
{kind: "skill",key: ["effect"]}
Explicit enabled and disabled preferences are stored through the shared KV service under capability:preferences. inherit removes the explicit preference and restores the skill default, which is derived from autoinvoke.
The capability API returns the default state, explicit preference, and effective state separately. The Skills dialog uses that API, while SkillInstructions applies the effective state when building automatic skill guidance. Explicit @skill references remain available even when automatic discovery is disabled.
To add another capability kind later:
Extend Capability.Kind and add a canonical reference constructor such as Capability.tool(name).
Add that domain's inventory to the capability API projection.
Call capability.resolve(ref, defaultState) at the domain's real availability boundary.
Build a domain-specific dialog over the shared preference API.
The preference service only resolves desired activation. Each domain still owns what disabling means.
How did you verify your code works?
Ran Core, Server, Client, and TUI typechecks.
Ran capability persistence and skill-instruction tests.
Ran the existing skill prompt and session tests.
Regenerated the Effect and Promise clients.
Screenshots / recordings
Not included.
Checklist
I have tested my changes locally
I have not included unrelated changes in this PR
If you do not follow this template your PR will be automatically rejected.
AI code review — automated review for reference; please use your judgment.
packages/core/src/capability.ts:41 — load() deletes the whole preferences key whenever decoding fails (kv.remove(Key)) — a schema evolution or partially-written value then silently wipes every user preference on first read, irreversibly — don't destroy data you can't parse: log + ignore, or store a schema version alongside so migrations are possible.
packages/core/src/capability.ts:60 — set() does load → filter → write with no locking or serialization; two concurrent updates (two TUI clients, or rapid toggles from different surfaces than this dialog) lose one preference — wrap the read-modify-write in an Effect semaphore/single-flight like other config writers, since KV isn't transactional here.
packages/core/src/session/runner/to-llm-message.ts:227 + compaction.ts:141 — dropping skills[].text from message lowering and compaction means pre-existing sessions recorded with inline skill guidance lose that context when resumed or compacted after upgrade — fine as a forward-looking design, but worth a release note ("old sessions degrade to mention-only") and ideally a compaction fallback that re-resolves attachments by id if still installed.
packages/server/src/handlers/capability.ts:14 — capability.list calls capability.get(ref) per skill, and each get re-reads and re-decodes the entire preferences array from KV (O(N) reads per request) — load once and match locally, mirroring what the service already exposes via list().
packages/core/test/capability.test.ts:8 — coverage of the happy path and inherit semantics is good, but the risky branches are untested: the corrupted-payload removal from item 1, concurrent set calls (item 2), and the new HTTP handler itself (list/update round-trip) — please pin down at least the first two before merge.
Nit — packages/tui/src/component/dialog-skill.tsx:47 — toggle derives the wire state as preference ?? "inherit" while optimistically rendering state; if another client flips defaultState between render and update, "inherit" lands on a different effective value than the checkbox showed — acceptable for a single-user dialog, just be aware the optimistic row can disagree until refresh.
Overall: nice abstraction — the Ref/Preference/Info split, event emission, and the move to on-demand skill loading via the tool are clean, and the TUI dialog rewrite (optimistic toggle + revert + search) is a clear UX win. Items 1–2 are the ones I'd fix pre-merge. Thanks!
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
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.
Issue for this PR
Closes #
Type of change
What does this PR do?
Adds a global capability-preference abstraction, initially used only for skills. This keeps mutable user preferences out of
Skill.Infoand leaves permissions, runtime health, and visibility as separate concerns.A capability is identified by a typed reference. The only supported kind in this PR is:
Explicit
enabledanddisabledpreferences are stored through the shared KV service undercapability:preferences.inheritremoves the explicit preference and restores the skill default, which is derived fromautoinvoke.The capability API returns the default state, explicit preference, and effective state separately. The Skills dialog uses that API, while
SkillInstructionsapplies the effective state when building automatic skill guidance. Explicit@skillreferences remain available even when automatic discovery is disabled.To add another capability kind later:
Capability.Kindand add a canonical reference constructor such asCapability.tool(name).capability.resolve(ref, defaultState)at the domain's real availability boundary.The preference service only resolves desired activation. Each domain still owns what disabling means.
How did you verify your code works?
Screenshots / recordings
Not included.
Checklist
If you do not follow this template your PR will be automatically rejected.