feat(skills): activation feedback pipeline + install idempotence (#2530)

* feat(events): SkillActivated carries activation feedback notes

Add an optional `feedback: Vec<String>` field to the SkillActivated
event so the engine and selector can surface human-readable activation
notes (chain-load reasons, marker exclusions, scoring summaries) to the
UI. Wire the field through the StatusUpdate, the SSE bridge, and the
gateway's activity timeline; serialize-skip empty vectors so the wire
format stays backwards compatible.

* fix(skills): skill_install never prompts when skill is already loaded

When the LLM force-activates a persona via `/ceo-setup` it sometimes
follows up with a redundant `skill_install("ceo-setup")` call. The
`execute` path was already idempotent (returns `already_installed`
without touching the catalog), but `requires_approval` still gated
the call behind a confirmation prompt — pure friction on a guaranteed
no-op.

Mirror the idempotent shortcut in `requires_approval`: when a skill
with the requested name is already loaded (bundled, user, workspace,
or previously installed), return `ApprovalRequirement::Never`. The
shortcut wins even when `install_dependencies=true` because the
top-level execute is still a no-op (companions get reconciled by their
own activation paths). Regression test covers all three cases.

* fix(skills): preserve approval for dependency installs

* fix(events): include feedback in AppEvent::SkillActivated all-variants list

The variant-enumeration constructor in event.rs:501 was missed when
the new `feedback` field was added to AppEvent::SkillActivated, breaking
the build with E0063. All three Clippy CI jobs failed on this.

Regression: covered by `cargo build --all-features`, which fails to
compile if any variant in this list is constructed with missing fields.

* feat(skills): wire up v1 feedback producer for SkillActivated

The `SkillActivated` event carried an empty `feedback` field because
nothing populated it. This adds the producer end of the pipeline.

**Selector:**
- `prefilter_skills` now returns `SelectionOutcome { selected, notes }`.
- `try_select` returns a reason enum (`Selected`, `BudgetFull`,
  `CandidateLimit`, `MarkerSatisfied`, `AlreadySelected`) so callers
  can render distinct notes instead of opaque "skipped".
- Notes generated for:
  - `<companion>: chain-loaded from <parent>`
  - `<companion>: chain-load skipped (budget full)`
  - `<companion>: chain-load skipped (max active skills reached)`
  - `<companion>: chain-load skipped (setup already complete)`
  - `<skill>: skipped (skill context budget exhausted)` for parents
    that scored but didn't fit.

**Agent loop:**
- `select_active_skills` returns the notes alongside selected skills
  and prepends a `<skill>: force-activated via /mention` note for each
  explicit mention.

**Dispatcher:**
- Emits `StatusUpdate::SkillActivated { skill_names, feedback }` via
  `channels.send_status` whenever something activated or notes exist
  (so "nothing loaded because budget exhausted" surfaces too).
- Silent when nothing activated and no notes — no UI noise.

**Stale comment:**
- Router's v2-bridge comment no longer claims v1 callers populate
  feedback "directly on `StatusUpdate`"; the v1 dispatcher now emits
  its own event, and v2 remains empty until the Python orchestrator
  is updated.

Regression: existing selector test `test_chain_load_respects_budget`,
`test_chain_load_skips_companion_with_satisfied_marker`, and
`test_chain_load_is_non_transitive` now also assert that the
corresponding note is in `outcome.notes`. The 42 selector tests and
503 agent-module tests all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
firat.sertgoz
2026-04-19 16:19:32 +03:00
committed by GitHub
parent 43d6fc16b0
commit 08693aa3cc
16 changed files with 488 additions and 81 deletions

View File

@@ -162,7 +162,7 @@ mod github_dev_workflow_test {
eprintln!("active skills: {:?}", harness.rig().active_skill_names());
for event in harness.rig().captured_status_events() {
match event {
StatusUpdate::SkillActivated { skill_names } => {
StatusUpdate::SkillActivated { skill_names, .. } => {
eprintln!(" ◆ skills activated: {}", skill_names.join(", "));
}
StatusUpdate::ToolStarted { name, detail, .. } => {