78 lines
3.1 KiB
Markdown
78 lines
3.1 KiB
Markdown
# Fix: Trigger Manager Bypasses Skill Adoption List
|
|
|
|
## Problem
|
|
|
|
`trigger_manager_load_from_skills()` and `trigger_manager_reconcile_from_relays()`
|
|
activate cron/webhook/subscription triggers for **every** self-authored skill event
|
|
(kind 31123/31124) that has trigger metadata, without checking whether the skill is
|
|
on the kind 10123 adoption list.
|
|
|
|
This means a skill that was published but never adopted (or was un-adopted) will
|
|
still have its cron trigger firing.
|
|
|
|
Contrast: `agent.c:refresh_adopted_skills_cache_if_needed()` properly fetches the
|
|
kind 10123 list, parses `"a"` tags, and only loads adopted skills — and also checks
|
|
the `enabled` tag. The trigger manager was written as a parallel system that skips
|
|
both gates.
|
|
|
|
## Root Cause
|
|
|
|
- `trigger_manager.c:939` — `trigger_manager_load_from_skills()`: no reference to
|
|
kind 10123 or adoption anywhere in the function. Calls `trigger_manager_add()`
|
|
unconditionally.
|
|
- `trigger_manager.c:592` — `trigger_manager_reconcile_from_relays()`: same bug.
|
|
Currently disabled (`enable_periodic_reconcile = 0` at line 1575) but should be
|
|
fixed before re-enabling.
|
|
- Neither function checks the `enabled` tag either, unlike
|
|
`trigger_manager_load_from_startup_events()` (line 1078) which does check it.
|
|
|
|
## Fix
|
|
|
|
### 1. Add a static helper: `load_adopted_d_tags()`
|
|
|
|
Add a static function in `trigger_manager.c` that:
|
|
- Fetches kind 10123 events via `nostr_handler_get_self_events_by_kind_json(10123)`
|
|
- Selects the latest event by `created_at` (same logic as agent.c)
|
|
- Parses `"a"` tags using a local `parse_skill_address_tag_local()` equivalent
|
|
- Populates a fixed-size array of adopted d_tag strings
|
|
- Returns the count
|
|
|
|
Since both call sites only deal with self-authored skills, matching on d_tag
|
|
alone is sufficient (the pubkey in the "a" tag will be the agent's own).
|
|
|
|
### 2. Add `enabled` tag check
|
|
|
|
Both functions should read the `enabled` tag (defaulting to `"true"`) and skip
|
|
skills where `enabled` is `false` or `0`, matching the pattern already used in
|
|
`trigger_manager_load_from_startup_events()`.
|
|
|
|
### 3. Gate `trigger_manager_load_from_skills()`
|
|
|
|
Before calling `trigger_manager_add()`, check:
|
|
1. The skill's d_tag is in the adopted set
|
|
2. The skill's `enabled` tag is not false/0
|
|
|
|
If the adoption list is empty or missing, log a warning and load nothing (do not
|
|
fall back to loading all skills — that's the bug).
|
|
|
|
### 4. Gate `trigger_manager_reconcile_from_relays()`
|
|
|
|
Apply the same adoption + enabled checks. This ensures the code is correct when
|
|
`enable_periodic_reconcile` is eventually flipped back to 1.
|
|
|
|
### 5. Pass `is_enabled` to `trigger_manager_add()`
|
|
|
|
Currently both functions pass `1` (hardcoded enabled) as the `enabled` parameter.
|
|
Change to pass the actual `is_enabled` value derived from the `enabled` tag.
|
|
|
|
## Files Changed
|
|
|
|
- `src/trigger_manager.c` — add helper, fix both functions
|
|
|
|
## Risk
|
|
|
|
Low. The fix adds a filter before `trigger_manager_add()`. If the adoption list
|
|
is present and correct, behavior is unchanged for adopted skills. If the adoption
|
|
list is missing, triggers simply won't load (which is the correct behavior —
|
|
nothing should fire if nothing is adopted).
|