Core cleanup: remove vestigial enabled flag and enable_skill/disable_skill from the skill model

[aroff]

---

issue: 184
branch: feature/184-core-cleanup-remove-vestigial-enabled-flag-and-enable-skill
board_item_id: PVTI_lADODpSNOs4BRC0yzgu-Uvc
source: github-project

Core cleanup: remove the vestigial enabled flag and enable_skill/disable_skill from the skill model

---

1. Problem statement

The fastskill-core crate's skill model carries an enabled: bool field on SkillDefinition, an enabled: Option<bool> field on both SkillUpdate and the event-bus-local SkillUpdate struct, an enabled: Option<bool> filter on SkillFilters, enable_skill and disable_skill methods on the SkillManagementService trait and its SkillManager implementation, corresponding SkillEnabled/SkillDisabled event variants in SkillEvent, and a mirrored enabled: bool field on SkillMetadata. The disable CLI command that drove this concept was retired as part of the CLI command-surface redesign (PR #185 / issue #183); the enable command was intentionally never surfaced. With no callers remaining in the CLI or any other crate, these symbols are dead weight that misrepresents the data model — a future developer reading the trait will incorrectly infer that a "disabled-but-installed" skill lifecycle exists. This issue removes all of that dead code in one cohesive change. Out of scope: removal of HotReloadConfig.enabled (an unrelated feature flag), any changes to the lock-file format, and any UI or server-side changes beyond removing the "enabled" key from the skill_metadata_json helper.

---

2. Goals

1. enable_skill and disable_skill MUST be removed from the SkillManagementService trait and from the SkillManager implementation.
2. SkillDefinition.enabled: bool MUST be removed from the struct definition and its new() constructor.
3. SkillUpdate.enabled: Option<bool> MUST be removed from both the skill_manager module's SkillUpdate and the event-bus module's SkillUpdate.
4. SkillFilters.enabled: Option<bool> MUST be removed, and the enabled-based retain filter in list_skills MUST be removed.
5. SkillMetadata.enabled: bool MUST be removed, including its population in the From<&SkillDefinition> conversion.
6. SkillEvent::SkillEnabled and SkillEvent::SkillDisabled variants MUST be removed from the enum and from all match arms that cover it (notify_handlers, LoggingEventHandler, MetricsEventHandler).
7. EventBus::publish_skill_enabled and EventBus::publish_skill_disabled convenience methods MUST be removed.
8. The "enabled" key MUST be removed from the skill_metadata_json helper in the HTTP handler.
9. A workspace-wide search for enable_skill, disable_skill, SkillEnabled, SkillDisabled, and \.enabled (scoped to skill contexts) MUST return no hits after the change.
10. The workspace MUST compile with zero dead-code warnings attributable to this removal, and all existing tests MUST pass.
11. Loading a skill whose SkillFrontmatter YAML contains an unrecognised enabled key MUST NOT produce an error (backward-compatible parsing, already satisfied by #[serde(flatten)] on SkillFrontmatter.extra; this MUST be verified by a test fixture).

---

3. Current behavior

Component	Role	Mutable at runtime?
SkillDefinition.enabled: bool	Runtime flag indicating whether a skill is considered active; defaults to true	Yes — via update_skill with SkillUpdate { enabled: Some(…) }
SkillManagementService::enable_skill	Trait method; sets enabled = true via update_skill	N/A (method)
SkillManagementService::disable_skill	Trait method; sets enabled = false via update_skill	N/A (method)
SkillFilters.enabled: Option<bool>	Optional predicate applied in list_skills; filters out skills whose enabled doesn't match	No
SkillUpdate.enabled: Option<bool> (skill_manager)	Patch field used to flip SkillDefinition.enabled	No
SkillUpdate.enabled: Option<bool> (event_bus)	Field on the event-bus-local SkillUpdate used in SkillEvent::SkillUpdated payloads	No
SkillMetadata.enabled: bool	Mirror of SkillDefinition.enabled; populated via From<&SkillDefinition>	No
SkillEvent::SkillEnabled / SkillDisabled	Event variants for skill lifecycle state changes; have matching handlers in LoggingEventHandler and MetricsEventHandler	No
EventBus::publish_skill_enabled / publish_skill_disabled	Convenience publishers for the above events	No
skill_metadata_json helper	Serialises "enabled": skill.enabled into the JSON blob embedded in SkillResponse.metadata	No

Current build and resume flow: SkillManager stores all skills in an in-memory Arc<RwLock<HashMap<SkillId, SkillDefinition>>>. There is no on-disk serialisation of SkillDefinition; the on-disk artefacts are skill-project.toml and skills.lock, neither of which carries an enabled field. Skills are reloaded into memory at startup from those manifests. Because SkillFrontmatter (parsed from SKILL.md) uses #[serde(flatten)] pub extra: HashMap<String, serde_yaml::Value>, any unknown YAML keys (including a legacy enabled: true) already go into the extra map without error — no additional tolerance work is needed.

---

4. Design

4.1 Removal strategy (dependency order)

Symbols MUST be removed in bottom-up dependency order to keep the workspace compilable at every step:

1. Remove enable_skill / disable_skill implementations from SkillManager.
2. Remove those methods from the SkillManagementService trait declaration.
3. Remove the enabled field from SkillUpdate (skill_manager module); remove the corresponding if let Some(enabled) branch in update_skill.
4. Remove SkillFilters.enabled and the retain block in list_skills. Because SkillFilters has no remaining fields after this removal, it MUST be removed entirely (Option B — user-confirmed design decision). The list_skills trait method signature changes from list_skills(&self, filters: Option<SkillFilters>) -> Result<Vec<SkillDefinition>, ServiceError> to list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError>. All call sites (9+) that previously passed None must drop the argument; any code importing SkillFilters must remove that import. See §4.3 for rationale.
5. Remove SkillDefinition.enabled and the enabled: true initializer in SkillDefinition::new.
6. Remove SkillMetadata.enabled and enabled: skill.enabled in the From impl.
7. Remove SkillEvent::SkillEnabled, SkillEvent::SkillDisabled variants. Remove the corresponding arms in notify_handlers, LoggingEventHandler::handle_event, MetricsEventHandler::handle_event. Remove the SkillUpdate.enabled field in the event-bus module's own SkillUpdate. Remove EventBus::publish_skill_enabled and publish_skill_disabled.
8. Remove "enabled": skill.enabled from skill_metadata_json in http/handlers/skills.rs.

4.2 Concurrency / locking

No concurrency changes. All mutations to SkillManager continue to go through the existing Arc<RwLock<…>> write lock in update_skill. The removed methods were thin wrappers around update_skill; eliminating them reduces the write-lock surface.

4.3 SkillFilters invariant

After removal, SkillFilters contains zero fields. Two options were considered:

• Option A: Retain the empty struct as a placeholder, keeping list_skills(filters: Option<SkillFilters>) signature unchanged. Avoids a breaking change; leaves an extension point for future filters.
• Option B (chosen): Remove SkillFilters entirely, changing the signature to list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError>. Produces the simplest possible API: a parameter that currently carries no data is eliminated entirely.

Decision: Option B. All 9+ active call sites already pass None, so the only required change at each call site is dropping the argument — a mechanical update with no semantic impact. An empty struct with no fields is API noise; removing it makes the trait immediately understandable. Future filter predicates, if ever needed, can be reintroduced as a new method overload or a builder pattern at that time, rather than preserving an empty placeholder indefinitely. This is a trait-level breaking change and is documented in §11.

4.4 HTTP API surface

The "enabled" key is removed from the JSON object returned in SkillResponse.metadata. This is a breaking change to the HTTP API contract. Callers that key on metadata.enabled will receive undefined / absent instead of true. Since disabled-skill support is retired, there is no longer any meaningful boolean to convey; removing the key is correct.

4.5 Backward-compatible deserialization (SKILL.md frontmatter)

SkillFrontmatter already uses #[serde(flatten)] pub extra: HashMap<String, serde_yaml::Value> at line 48 of metadata.rs. Any SKILL.md file whose YAML frontmatter contains enabled: true or enabled: false will silently absorb the unknown key into extra. No code changes are needed for this. A regression test MUST verify this behaviour (see §7, criterion 11).

---

5. Schema and API

5.1 Structs after removal

// crates/fastskill-core/src/core/skill_manager.rs

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SkillDefinition {
    pub id: SkillId,
    pub name: String,
    pub description: String,
    pub version: String,
    pub author: Option<String>,
    // `enabled: bool` REMOVED
    pub created_at: DateTime<Utc>,
    pub updated_at: DateTime<Utc>,
    pub skill_file: std::path::PathBuf,
    pub reference_files: Option<Vec<std::path::PathBuf>>,
    pub script_files: Option<Vec<std::path::PathBuf>>,
    pub asset_files: Option<Vec<std::path::PathBuf>>,
    pub execution_environment: Option<String>,
    pub dependencies: Option<Vec<String>>,
    pub timeout: Option<u64>,
    pub source_url: Option<String>,
    pub source_type: Option<SourceType>,
    pub source_branch: Option<String>,
    pub source_tag: Option<String>,
    pub source_subdir: Option<PathBuf>,
    pub installed_from: Option<String>,
    pub commit_hash: Option<String>,
    pub fetched_at: Option<DateTime<Utc>>,
    pub editable: bool,
}

// `SkillFilters` struct REMOVED ENTIRELY (Option B — see §4.3)

#[derive(Debug, Clone, Default)]
pub struct SkillUpdate {
    pub name: Option<String>,
    pub description: Option<String>,
    pub version: Option<String>,
    pub author: Option<String>,
    // `enabled: Option<bool>` REMOVED
    pub source_url: Option<String>,
    pub source_type: Option<SourceType>,
    pub source_branch: Option<String>,
    pub source_tag: Option<String>,
    pub source_subdir: Option<PathBuf>,
    pub installed_from: Option<String>,
    pub commit_hash: Option<String>,
    pub fetched_at: Option<DateTime<Utc>>,
    pub editable: Option<bool>,
}

5.2 Trait after removal

// crates/fastskill-core/src/core/skill_manager.rs

#[async_trait]
pub trait SkillManagementService: Send + Sync {
    async fn register_skill(&self, skill: SkillDefinition) -> Result<SkillId, ServiceError>;
    async fn force_register_skill(&self, skill: SkillDefinition) -> Result<SkillId, ServiceError>;
    async fn get_skill(&self, skill_id: &SkillId) -> Result<Option<SkillDefinition>, ServiceError>;
    async fn update_skill(
        &self,
        skill_id: &SkillId,
        updates: SkillUpdate,
    ) -> Result<(), ServiceError>;
    async fn unregister_skill(&self, skill_id: &SkillId) -> Result<(), ServiceError>;
    async fn list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError>;
    // `enable_skill` REMOVED
    // `disable_skill` REMOVED
    // `filters: Option<SkillFilters>` parameter REMOVED — SkillFilters struct deleted (see §4.3)
}

5.3 SkillMetadata after removal

// crates/fastskill-core/src/core/metadata.rs

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SkillMetadata {
    pub id: SkillId,
    pub name: String,
    pub description: String,
    pub version: String,
    pub author: Option<String>,
    // `enabled: bool` REMOVED
    pub token_estimate: usize,
    pub last_updated: chrono::DateTime<chrono::Utc>,
}

5.4 SkillEvent after removal

// crates/fastskill-core/src/events/event_bus.rs

#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum SkillEvent {
    SkillRegistered { skill_id: String, skill: Box<SkillDefinition> },
    SkillUpdated { skill_id: String, changes: SkillUpdate },
    SkillUnregistered { skill_id: String },
    SkillReloaded { skill_id: String, success: bool, error_message: Option<String> },
    SkillValidationFailed { skill_id: String, errors: Vec<String> },
    HotReloadEnabled { config: HotReloadConfig },
    HotReloadDisabled,
    // `SkillEnabled` REMOVED
    // `SkillDisabled` REMOVED
    Custom { event_type: String, data: serde_json::Value },
}

// event-bus-local SkillUpdate:
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SkillUpdate {
    pub name: Option<String>,
    pub description: Option<String>,
    pub version: Option<String>,
    // `enabled: Option<bool>` REMOVED
}

5.5 Removed EventBus methods

// REMOVED from EventBus impl:
// pub async fn publish_skill_enabled(&self, skill_id: String) -> Result<usize, ServiceError>
// pub async fn publish_skill_disabled(&self, skill_id: String) -> Result<usize, ServiceError>

---

6. Error codes

This change introduces no new error codes. It removes two code paths that previously returned ServiceError::SkillNotFound — specifically the paths through enable_skill and disable_skill (which delegated to update_skill). No existing error code semantics change.

Code	Category	Trigger	Change
ServiceError::SkillNotFound	Not found	Returned by update_skill when skill_id is absent from the in-memory store	No change to the code itself; two previously reachable paths via enable_skill/disable_skill are eliminated

---

7. Acceptance criteria

1. SkillManagementService trait MUST NOT declare enable_skill or disable_skill.
2. SkillManager MUST NOT implement enable_skill or disable_skill.
3. SkillDefinition MUST NOT contain an enabled field.
4. SkillUpdate (skill_manager module) MUST NOT contain an enabled field.
5. SkillFilters MUST be removed entirely from the codebase (struct definition, public export, and all imports). list_skills MUST NOT apply any enabled-based retain filter, and its signature MUST be list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError> with no filters parameter.
6. SkillMetadata MUST NOT contain an enabled field.
7. SkillEvent MUST NOT declare SkillEnabled or SkillDisabled variants.
8. EventBus MUST NOT expose publish_skill_enabled or publish_skill_disabled methods.
9. The event-bus-local SkillUpdate struct MUST NOT contain an enabled field.
10. The skill_metadata_json helper in http/handlers/skills.rs MUST NOT include an "enabled" key in its output.
11. A test fixture MUST demonstrate that deserialising a SkillFrontmatter from YAML containing an enabled: true key succeeds without error and the extra map contains the key.
12. A workspace-wide grep -rn "enable_skill\|disable_skill\|SkillEnabled\|SkillDisabled\|SkillFilters" across crates/ MUST return zero hits.
13. cargo build --workspace MUST succeed with zero dead-code warnings related to the removed symbols.
14. cargo test --workspace MUST pass with no regressions.
15. All call sites of list_skills MUST be updated to drop the filters argument (previously None everywhere): fastskill-cli/src/commands/list.rs:181, fastskill-cli/src/commands/read.rs:179,374, http/handlers/skills.rs:45,76,177, http/handlers/status.rs:64,241, http/handlers/search.rs:113, http/handlers/registry.rs:202,309, core/metadata.rs:232, lib.rs:130. Each call site MUST compile after removing the argument.
16. The SkillFilters struct MUST NOT exist anywhere in the workspace after this change (Option B from §4.3). The list_skills trait method signature MUST be list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError> with no filters parameter. A workspace-wide grep -rn "SkillFilters" across crates/ MUST return zero hits.

---

8. Functionality comparison (before vs after)

Capability	Before	After
Install a skill	Skill registered with enabled: true by default	Skill registered; no enabled field
List skills	list_skills(Some(SkillFilters { enabled: Some(true) })) returns only enabled skills	list_skills() returns all installed skills; SkillFilters type removed; no filtering by enabled state
Disable a skill (CLI)	disable command existed (since retired in PR #185)	No concept; not representable
Enable/disable via service API	service.skill_manager().enable_skill(&id) / disable_skill(&id)	Methods do not exist; call sites must be removed
HTTP skill metadata	GET /api/skills response includes metadata.enabled: true/false	metadata.enabled key absent from response
Event bus	SkillEnabled and SkillDisabled events publishable	Events do not exist; publish_skill_enabled/disabled removed
Skill metadata struct	SkillMetadata.enabled field present	Field removed

---

9. Benefit of implementing this functionality

Removing the dead code eliminates a model-level fiction: the trait, struct fields, and event variants collectively imply that a "disabled-but-installed" lifecycle exists when it does not. Deleting them reduces cognitive overhead for contributors reading the code, lowers the surface area that must be kept in sync during future refactors (e.g. if SkillDefinition gains new fields), and ensures any future consumer of the public API (CLI, HTTP, embedding pipeline) does not mistakenly build a feature on top of an absent concept. It also produces a cleaner, smaller binary by eliminating unreachable code paths, and avoids the possibility of a new test or integration accidentally calling enable_skill/disable_skill and inferring they are meaningful.

---

10. Stages

Stage 1 — Trait and impl removal (blocking for all subsequent stages)

Deliverable: crates/fastskill-core/src/core/skill_manager.rs compiles with enable_skill and disable_skill removed from both the trait and SkillManager impl.

Scope:

• Remove async fn enable_skill(&self, skill_id: &SkillId) -> Result<(), ServiceError>; from SkillManagementService trait (line 106).
• Remove async fn disable_skill(&self, skill_id: &SkillId) -> Result<(), ServiceError>; from SkillManagementService trait (line 107).
• Remove the enable_skill implementation body from SkillManager (lines 264–273).
• Remove the disable_skill implementation body from SkillManager (lines 275–284).

Dependency: None (first stage).

---

Stage 2 — Remove enabled from SkillUpdate, remove SkillFilters entirely, update all list_skills call sites (depends on Stage 1)

Deliverable: SkillUpdate no longer carries enabled; SkillFilters is deleted entirely; list_skills signature is list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError>; all 9+ call sites compile with the argument dropped.

Scope:

• Remove pub enabled: Option<bool> from SkillUpdate in skill_manager.rs (line 140).
• Remove the if let Some(enabled) = updates.enabled { skill.enabled = enabled; } branch from update_skill in skill_manager.rs (lines 200–202).
• Remove the entire SkillFilters struct from skill_manager.rs (line 129–132) — not just the enabled field, but the struct declaration itself.
• Remove the if let Some(enabled) = filters.enabled { filtered_skills.retain(|skill| skill.enabled == enabled); } block from list_skills in skill_manager.rs (lines 254–259).
• Update the list_skills signature in the SkillManagementService trait and SkillManager impl to list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError> (drop filters: Option<SkillFilters> parameter).
• Remove SkillFilters from the public re-export in crates/fastskill-core/src/core/mod.rs (line 112).
• Update all call sites to drop the filters argument:
  • fastskill-cli/src/commands/list.rs:181
  • fastskill-cli/src/commands/read.rs:179, 374
  • crates/fastskill-core/src/http/handlers/skills.rs:45, 76, 177
  • crates/fastskill-core/src/http/handlers/status.rs:64, 241
  • crates/fastskill-core/src/http/handlers/search.rs:113
  • crates/fastskill-core/src/http/handlers/registry.rs:202, 309
  • crates/fastskill-core/src/core/metadata.rs:232
  • crates/fastskill-core/src/lib.rs:130

Dependency: Stage 1 must be complete (the enabled-update path was called only by enable_skill/disable_skill, now absent).

---

Stage 3 — Remove enabled from SkillDefinition (depends on Stage 2)

Deliverable: SkillDefinition struct and constructor no longer carry or initialise enabled.

Scope:

• Remove pub enabled: bool field from SkillDefinition struct (line 30 of skill_manager.rs).
• Remove enabled: true, from SkillDefinition::new() (line 66 of skill_manager.rs).

Dependency: Stage 2 must be complete; the enabled field is no longer read or written after Stage 2 removals.

---

Stage 4 — Remove enabled from SkillMetadata (depends on Stage 3)

Deliverable: SkillMetadata struct and its From<&SkillDefinition> impl no longer carry enabled.

Scope:

• Remove pub enabled: bool from SkillMetadata in metadata.rs (line 16).
• Remove enabled: skill.enabled, from the From<&SkillDefinition> for SkillMetadata impl in metadata.rs (line 29).

Dependency: Stage 3 must be complete; skill.enabled would be a compile error once the field is removed from SkillDefinition.

---

Stage 5 — Remove event-bus symbols (depends on Stage 3)

Deliverable: SkillEvent, EventBus, and the event-bus-local SkillUpdate no longer carry enabled-related symbols; all match arms are exhaustive.

Scope:

• Remove SkillEnabled { skill_id: String } variant from SkillEvent enum in event_bus.rs (line 53).
• Remove SkillDisabled { skill_id: String } variant from SkillEvent enum in event_bus.rs (line 56).
• Remove pub enabled: Option<bool> from the event-bus-local SkillUpdate struct in event_bus.rs (line 71).
• Remove the SkillEvent::SkillEnabled { .. } => "skill:enabled" arm from notify_handlers in event_bus.rs (line 207).
• Remove the SkillEvent::SkillDisabled { .. } => "skill:disabled" arm from notify_handlers in event_bus.rs (line 208).
• Remove the SkillEvent::SkillEnabled { skill_id } arm from LoggingEventHandler::handle_event in event_bus.rs (lines 303–305).
• Remove the SkillEvent::SkillDisabled { skill_id } arm from LoggingEventHandler::handle_event in event_bus.rs (lines 306–308).
• Remove the SkillEvent::SkillEnabled { .. } => "skill:enabled".to_string() arm from MetricsEventHandler::handle_event in event_bus.rs (line 352).
• Remove the SkillEvent::SkillDisabled { .. } => "skill:disabled".to_string() arm from MetricsEventHandler::handle_event in event_bus.rs (line 353).
• Remove EventBus::publish_skill_enabled method in event_bus.rs (lines 438–442).
• Remove EventBus::publish_skill_disabled method in event_bus.rs (lines 444–448).

Dependency: Stage 3 must be complete (event variants referenced SkillDefinition indirectly through publish helpers; no compile blocker, but logical dependency on model being clean first).

---

Stage 6 — Remove enabled from HTTP handler (depends on Stage 3)

Deliverable: skill_metadata_json helper no longer serialises "enabled" into the JSON response.

Scope:

• Remove "enabled": skill.enabled, from the serde_json::json! block in skill_metadata_json in http/handlers/skills.rs (line 29).

Dependency: Stage 3 must be complete; skill.enabled is a compile error once the field is removed.

---

Stage 7 — Add backward-compat test and final verification (depends on Stages 1–6)

Deliverable: A test fixture confirms that YAML frontmatter containing a legacy enabled field is parsed without error.

Scope:

• Add a unit test in crates/fastskill-core/src/core/metadata.rs (or a new #[cfg(test)] block in that file) that calls parse_yaml_frontmatter with YAML content that includes enabled: true in the frontmatter and asserts the call succeeds and the returned SkillFrontmatter.extra contains the "enabled" key.
• Run cargo build --workspace and confirm zero dead-code warnings related to removed symbols.
• Run cargo test --workspace and confirm all tests pass.
• Run grep -rn "enable_skill\|disable_skill\|SkillEnabled\|SkillDisabled\|SkillFilters" crates/ and confirm zero hits.

Dependency: All of Stages 1–6.

---

11. Breaking changes

Change	Affected surface	Impact
enable_skill removed from SkillManagementService trait	Any downstream crate that implements the trait or calls the method	Callers must remove their call sites; trait implementations must remove the method; compile error if not updated
disable_skill removed from SkillManagementService trait	Same as above	Same as above
SkillDefinition.enabled removed	Any code reading skill.enabled	Compile error if not updated
SkillUpdate.enabled removed (skill_manager module)	Any code constructing SkillUpdate { enabled: Some(…) }	Compile error if not updated
SkillUpdate.enabled removed (event_bus module)	Any code constructing the event-bus SkillUpdate { enabled: Some(…) }	Compile error if not updated
SkillFilters removed entirely; list_skills signature simplified	Any code importing or constructing SkillFilters, and all 9+ list_skills call sites	Compile error: type not found; call sites must drop the filters argument
SkillMetadata.enabled removed	Any code reading metadata.enabled	Compile error if not updated
SkillEvent::SkillEnabled / SkillDisabled removed	Any match on SkillEvent that names these variants	Compile error if not updated; exhaustive match patterns must be updated
EventBus::publish_skill_enabled / publish_skill_disabled removed	Any caller of these methods	Compile error if not updated
GET /api/skills and GET /api/skills/{id} HTTP responses	Clients consuming metadata.enabled	Runtime break: key absent from JSON; no compile-time signal

---

12. Modified files and summary

File / path	Summary of modifications
crates/fastskill-core/src/core/skill_manager.rs	Remove enabled: bool from SkillDefinition; remove enabled: true from SkillDefinition::new(); remove enabled: Option<bool> from SkillUpdate; remove if let Some(enabled) = updates.enabled branch in update_skill; remove SkillFilters struct entirely; remove enabled-based retain block in list_skills; change list_skills signature to list_skills(&self) -> Result<Vec<SkillDefinition>, ServiceError>; remove enable_skill and disable_skill from SkillManagementService trait and SkillManager impl
crates/fastskill-core/src/core/metadata.rs	Remove enabled: bool from SkillMetadata; remove enabled: skill.enabled from From<&SkillDefinition>; update list_skills() call at line 232 (drop argument); add #[cfg(test)] block with backward-compat frontmatter parse test
crates/fastskill-core/src/events/event_bus.rs	Remove SkillEnabled and SkillDisabled variants from SkillEvent; remove enabled: Option<bool> from event-bus-local SkillUpdate; remove match arms for SkillEnabled/SkillDisabled in notify_handlers, LoggingEventHandler, MetricsEventHandler; remove publish_skill_enabled and publish_skill_disabled from EventBus
crates/fastskill-core/src/http/handlers/skills.rs	Remove "enabled": skill.enabled, from skill_metadata_json; update list_skills() calls at lines 45, 76, 177 (drop argument)
crates/fastskill-core/src/http/handlers/status.rs	Update list_skills() calls at lines 64, 241 (drop argument)
crates/fastskill-core/src/http/handlers/search.rs	Update list_skills() call at line 113 (drop argument)
crates/fastskill-core/src/http/handlers/registry.rs	Update list_skills() calls at lines 202, 309 (drop argument)
crates/fastskill-core/src/core/mod.rs	Remove SkillFilters from the public re-export (line 112); other re-exports automatically reflect struct changes
crates/fastskill-core/src/lib.rs	Update list_skills() call at line 130 (drop argument); re-exports of SkillDefinition, SkillManagementService automatically reflect changes
crates/fastskill-cli/src/commands/list.rs	Update list_skills() call at line 181 (drop argument)
crates/fastskill-cli/src/commands/read.rs	Update list_skills() calls at lines 179, 374 (drop argument)

No existing test files in crates/fastskill-cli/tests/ (lock_migration_test.rs, lock_determinism_test.rs, lock_scope_routing_test.rs) reference skill-enabled symbols; they operate on lock-file logic and do not require modification.

---

13. Dependencies

Dependency	Type	Blocking?	Gates which stage?	If skipped
PR #185 / issue #183 (CLI command-surface redesign, disable command removal)	Internal — prior PR	Blocking	Stage 1 (all stages)	If this change lands before #183 merges, disable command would fail to compile because its call to disable_skill would be unresolvable
async-trait crate	External library	Non-blocking	N/A — already in use	N/A
serde / serde_yaml crates	External library	Non-blocking	Stage 7 (test)	N/A — already in use
No team dependencies	—	—	—	—

---

14. Proposed folder structure

This is not a new project. The files listed in §12 are modified. No new files are created except optionally a test fixture added inline via #[cfg(test)] in metadata.rs. No folder-structure changes are required.

crates/fastskill-core/src/
├── core/
│   ├── metadata.rs          ← modified (remove field + update list_skills call + add test)
│   ├── skill_manager.rs     ← modified (primary cleanup site; SkillFilters removed entirely)
│   └── mod.rs               ← modified (remove SkillFilters re-export)
├── events/
│   └── event_bus.rs         ← modified (remove variants + methods)
├── http/
│   └── handlers/
│       ├── skills.rs        ← modified (remove "enabled" key; update list_skills calls)
│       ├── status.rs        ← modified (update list_skills calls)
│       ├── search.rs        ← modified (update list_skills call)
│       └── registry.rs      ← modified (update list_skills calls)
└── lib.rs                   ← modified (update list_skills call)

crates/fastskill-cli/src/commands/
├── list.rs                  ← modified (update list_skills call)
└── read.rs                  ← modified (update list_skills calls)

---

15. Design decisions

#	Question	Decision	Rationale
1	Should SkillFilters be removed entirely or kept as an empty struct?	Remove entirely (Option B — user-confirmed)	A struct carrying zero fields is API noise. All 9+ call sites already pass None, so dropping the argument is a mechanical one-line change at each site. The simplest possible API (list_skills(&self)) is immediately understandable, and future filter predicates can be introduced as a new method or builder at that time rather than indefinitely preserving an empty placeholder
2	Should the SkillEvent::SkillUpdated variant's changes: SkillUpdate payload be kept (minus enabled)?	Yes, keep the variant and payload	SkillUpdated is a meaningful general-purpose event; removing it would be a separate, broader change beyond this issue's scope
3	Should a changelog entry be added?	No	The spec explicitly states no changelog entry is needed beyond what the CLI removal (PR #185) already covered
4	Should SkillDefinition remain Serialize + Deserialize?	Yes	Other code paths (e.g. HTTP response helpers, test serialisation) may use these derives; removing them is not part of this issue
5	Should the backward-compat YAML test use a file fixture or an inline string?	Inline string in #[cfg(test)]	Simpler, no file-system dependency in unit tests; the frontmatter parsing code (parse_yaml_frontmatter) operates on &str
6	Should HotReloadConfig.enabled be removed as part of this change?	No	HotReloadConfig.enabled is an unrelated feature flag for the hot-reload subsystem and is actively used in service.rs (line 357) and http/handlers/status.rs (line 251); explicitly out of scope

---

16. Out of scope (explicit)

• Removal of HotReloadConfig.enabled — this is an active feature flag for the hot-reload subsystem, not related to the skill-enabled concept.
• Any changes to the lock file format (skills.lock) or the manifest format (skill-project.toml).
• Adding a migration step or tooling to update existing installations.
• Any UI changes to the server dashboard or web interface.
• Removal of SkillEvent::SkillUpdated or the event-bus-local SkillUpdate struct (beyond the enabled field).
• Changes to SkillFrontmatter (the extra map already handles unknown keys gracefully).
• Adding new filter predicates to SkillFilters (that is a separate feature).
• Changes to crates/fastskill-evals — a grep confirms no references to enabled-skill symbols there.
• Documentation (README, changelog) updates beyond removing the disable-skill references already removed by PR feat(cli): CLI command-surface redesign — remove redundant/vestigial commands (#183) #185.

---

17. References

Implementers SHOULD start from these exact locations:

Symbol	File	Lines
SkillDefinition struct	crates/fastskill-core/src/core/skill_manager.rs	23–55
SkillDefinition::new() constructor	crates/fastskill-core/src/core/skill_manager.rs	58–86
SkillManagementService trait	crates/fastskill-core/src/core/skill_manager.rs	91–108
SkillFilters struct (to be deleted entirely)	crates/fastskill-core/src/core/skill_manager.rs	129–132
SkillUpdate struct (skill_manager)	crates/fastskill-core/src/core/skill_manager.rs	134–150
update_skill impl (enabled branch)	crates/fastskill-core/src/core/skill_manager.rs	200–202
list_skills impl (enabled filter)	crates/fastskill-core/src/core/skill_manager.rs	254–259
enable_skill impl	crates/fastskill-core/src/core/skill_manager.rs	264–273
disable_skill impl	crates/fastskill-core/src/core/skill_manager.rs	275–284
SkillMetadata struct	crates/fastskill-core/src/core/metadata.rs	9–19
From<&SkillDefinition> for SkillMetadata	crates/fastskill-core/src/core/metadata.rs	21–34
SkillFrontmatter.extra (serde flatten)	crates/fastskill-core/src/core/metadata.rs	48–49
SkillEvent enum	crates/fastskill-core/src/events/event_bus.rs	16–63
Event-bus-local SkillUpdate struct	crates/fastskill-core/src/events/event_bus.rs	66–72
notify_handlers match	crates/fastskill-core/src/events/event_bus.rs	199–211
LoggingEventHandler::handle_event	crates/fastskill-core/src/events/event_bus.rs	261–316
MetricsEventHandler::handle_event	crates/fastskill-core/src/events/event_bus.rs	343–363
publish_skill_enabled / publish_skill_disabled	crates/fastskill-core/src/events/event_bus.rs	438–448
skill_metadata_json helper	crates/fastskill-core/src/http/handlers/skills.rs	18–39
Public re-exports (auto-updated)	crates/fastskill-core/src/core/mod.rs	111–113
Crate-level re-exports (auto-updated)	crates/fastskill-core/src/lib.rs	74

[aroff]

Depends on #183 (CLI command-surface redesign), which removes the disable command. Do this cleanup after that lands, or core won't compile.