…ousResearch#29912)

Closes NousResearch#29912.

The curator could archive a skill that an enabled cron job still depended on,
causing the next scheduled run to silently fail.  Three coordinated layers
close the gap:

1. `cron/jobs.py` — add `get_active_skill_refs()`: calls `list_jobs(include_disabled=False)`
   (single source of truth) and returns the set of skill names referenced by any
   enabled job.

2. `agent/curator.py` — add `_load_cron_protected()`: wraps the import + call behind
   a two-tier error boundary.  `ModuleNotFoundError` → cron not installed, return `set()`.
   Any other exception → cron present but broken, return `None` (fail-closed signal).
   `apply_automatic_transitions()` and `run_curator_review()` snapshot cron-protected
   refs once and propagate the `None` signal: transitions are skipped entirely when the
   cron store is unreadable.  `_render_candidate_list()` emits a visible LLM warning
   and adds a "Cron-protected skills" section to the prompt so the model won't archive
   a live dependency.

3. `tools/skill_manager_tool.py` — harden `_delete_skill()` as a last-resort guard:
   inside a curator background-review fork (`is_background_review()` ContextVar), reject
   any delete of a cron-referenced skill with a structured error before touching disk.
   Same two-tier import boundary; fail-closed on cron-store errors.

All three layers share `get_active_skill_refs()` as their single source of truth.

The handler uses `except Exception` (to catch broken-import errors like
RuntimeError during module init), but the inline comment still said
"Other ImportError". Align the comment with the actual code.

…otected kwarg

The stub in test_real_run_takes_pre_snapshot used `lambda now=None: {...}`
which did not accept the new `cron_protected` keyword argument added by the
fix for NousResearch#29912. When run_curator_review called
`apply_automatic_transitions(now=start, cron_protected=cron_protected)` the
stub raised TypeError. Update to `lambda now=None, cron_protected=None: {...}`.

When cron.jobs is installed but get_active_skill_refs is absent (e.g.
partial upgrade / old module version), Python raises plain ImportError
with e.name == "cron.jobs" — indistinguishable from a missing module
if we only check e.name.  Both _load_cron_protected() and the
_delete_skill() cron guard were returning "not installed" (empty
set / pass) in this case, silently disabling the protection.

Fix: catch ModuleNotFoundError and ImportError separately.  Only
ModuleNotFoundError with e.name in ("cron", "cron.jobs") means the
subsystem is absent; plain ImportError always means fail-closed.

Also fix trailing \n inside a string appended to a join-joined list
in _render_candidate_list (produces double blank line).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- curator.py: use BACKGROUND_REVIEW constant (from tools.skill_provenance)
  instead of the bare string literal when setting _memory_write_origin,
  so it stays in sync with is_background_review() automatically.
- curator.py: add exc_info=True to all WARNING calls in _load_cron_protected
  so import/runtime errors include a traceback for easier diagnosis.
- skill_manager_tool.py: collapse three identical except branches in the
  _delete_skill cron guard into one except Exception as e with an isinstance
  check, eliminating the duplicated log call and error-dict literal.
- tests/tools/test_skill_manager_tool.py: replace manual sys.modules
  save/restore with monkeypatch.setitem for correct teardown semantics.
- cron/jobs.py: log a WARNING when _normalize_skill_list swallows TypeError
  so operators know to repair a malformed job record.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…chain

Add a comment clarifying that conversation_loop.py reads _memory_write_origin
and calls set_current_write_origin() at the start of run_conversation(),
which is what sets the ContextVar that is_background_review() checks in
tool handlers such as _delete_skill(). The mechanism is correct; the
comment makes the indirect path visible to reviewers.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ules

monkeypatch.setitem only restores sys.modules["cron.jobs"]; if the import
machinery ever sets cron.jobs on the parent package, the attribute leaks
into later tests. Patch both together so monkeypatch teardown restores
the complete import state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

When a job record has a malformed non-iterable 'skills' field but a
valid legacy 'skill' field, the TypeError handler was dropping the
skill entirely, causing get_active_skill_refs() to miss the dependency
and potentially allowing curator to archive an in-use skill.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…skill_list

Asserts that a job with a non-iterable 'skills' field (e.g., an integer
from corrupted JSON) still contributes its legacy 'skill' value to
get_active_skill_refs(), and that a WARNING is logged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… docstring

- test_curator.py: also patch cron.jobs on the parent package so
  monkeypatch teardown restores the full import state (same fix already
  applied to test_skill_manager_tool.py)
- _load_cron_protected() docstring: empty set means "no protected skills"
  (cron absent OR installed with no enabled skill refs), not just "absent"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>