fix(skill): back off crystallization retries after repeated failures

[smoryan]

Summary

A crystallize or verify failure leaves the policy untouched, so every skill
tick retried the same policy forever. On one long-running production install,
the 2026-08-28 audit counted 2,640 repeated crystallize failures over
25 days — the bulk of wasted skill invocations. This PR bounds that retry
cost: after 3 consecutive failures a policy is taken off the crystallization
path; at any earlier point a successful crystallization clears the counter
and failures start counting from zero again.

Problem

runSkill treats both failure modes (crystallizer skip and verifier
rejection) as advisory: it logs a warning and moves on, but the policy's
skill_eligible flag stays true. The next trigger re-selects the same
policy and the same failure repeats — indefinitely for policies whose failure
cause is permanent (e.g. evidence that can never satisfy the verifier).

Change

• core/skill/skill.ts — count consecutive failures per policy in kv
  (skill.failCount:<id>); both failure paths call bumpFailureBackoff()
  before continue, and a successful crystallization clears the counter.
  After SKILL_FAILURE_BACKOFF_LIMIT (3) consecutive failures the policy's
  skill_eligible flag is turned off. The one deliberate exception is
  "llm-disabled": that skip reason is a global configuration state, not a
  failure of the policy, so it never counts (otherwise switching the LLM off
  would trip the whole candidate pool with no recovery path).
• core/skill/eligibility.ts — decide() now reports skill_eligible=false
  as its own skip reason ("policy.skillEligible=false (backoff or manual)"),
  so a tripped backoff stays distinguishable from a manual toggle; the hidden
  check inside hasSuccessAnchor() is removed so the two sources don't blur.
• core/storage/repos/policies.ts — new setSkillEligible() repo method. It
  deliberately leaves updated_at untouched: bumping it would flip the
  rebuild heuristic for an existing skill as a side effect.
• tests/unit/skill/backoff.test.ts — new regression suite (5 tests): trip
  after the 3rd consecutive failure and not before, tripped policies are
  skipped by the eligibility gate with the counter cleared on trip, a
  successful crystallization clears the counter, a manually re-enabled
  policy gets a fresh backoff window, and llm-disabled ticks never count.

The counter is cleared when the backoff trips, so re-enabling the
skill_eligible flag gives the policy a fresh 3-failure window.

Tests

• npx vitest run tests/unit/skill → 74 passed (12 files) (69
  pre-existing + 5 new)
• npx tsc --noEmit → clean (exit 0)

Related

• Complements fix(skill): auto-generate crystallizer summary/steps instead of rejecting #2253: that PR removed one large source of crystallizer
  failures (validator rejections of valid-but-incomplete drafts); this PR
  bounds the retry cost of the failures that remain (LLM refusal, verifier
  mismatches, permanently unqualifying evidence). Fixes fix: skill crystallization retries the same failing policies forever #2319.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (does not change functionality, e.g. code style improvements, lint)
• Documentation update

How Has This Been Tested?

• Unit tests (npx vitest run tests/unit/skill — 74 passed)
• Type-check (tsc --noEmit clean)

Checklist

• I have performed a self-review of my own code
• I have commented my code in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works
• I have created related documentation issue/PR in MemOS-Docs (if applicable)
• I have linked the issue to this PR (if applicable)
• I have mentioned the person who will review this PR

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2326
Task: 8dbd50af769761aa
Base: main
Head: fix/skill-crystallization-backoff

🔍 OpenCodeReview found 1 issue(s) in this PR.

---

1. apps/memos-local-plugin/core/skill/skill.ts (L364-L374)

The failure counter is written to KV before checking whether the trip threshold is reached, but both operations are not atomic. If setSkillEligible throws (e.g. a DB error), the counter will have been incremented but the policy will not be marked ineligible. On the next call the counter starts from 1 again because kv.del is only called inside the if block, so the window effectively resets silently on every error. More critically, if kv.set succeeds but the process crashes before kv.del runs after tripping, the counter is already deleted by the next invocation path, but the policy flag was never set — the backoff never trips.

Consider restructuring so the KV write and the policy update happen together, or at minimum ensure kv.del is called in a finally-style guard so partial state is avoided:

if (count >= SKILL_FAILURE_BACKOFF_LIMIT) {
  deps.repos.kv.del(failCountKey(policyId)); // clear first to avoid stale state on error
  deps.repos.policies.setSkillEligible(policyId, false);
  deps.log.warn("skill.backoff.exhausted", { policyId, count });
}

Clearing the counter before the policy write means a crash between the two leaves the counter cleared and the policy still eligible — a safe fallback that lets it retry cleanly, rather than leaving the counter stuck at the trip value.

💡 Suggested Change

Before:

function bumpFailureBackoff(deps: RunSkillDeps, policyId: PolicyId): void {
  const count = deps.repos.kv.get<number>(failCountKey(policyId), 0) + 1;
  deps.repos.kv.set(failCountKey(policyId), count);
  if (count >= SKILL_FAILURE_BACKOFF_LIMIT) {
    deps.repos.policies.setSkillEligible(policyId, false);
    // Clear the counter on trip: a later manual re-enable must get a fresh
    // window, not instant re-trip on the next single failure.
    deps.repos.kv.del(failCountKey(policyId));
    deps.log.warn("skill.backoff.exhausted", { policyId, count });
  }
}

After:

function bumpFailureBackoff(deps: RunSkillDeps, policyId: PolicyId): void {
  const count = deps.repos.kv.get<number>(failCountKey(policyId), 0) + 1;
  deps.repos.kv.set(failCountKey(policyId), count);
  if (count >= SKILL_FAILURE_BACKOFF_LIMIT) {
    // Clear before writing the policy flag: if setSkillEligible throws,
    // the counter is already gone so the next run starts fresh rather
    // than re-tripping instantly or leaving stale state.
    deps.repos.kv.del(failCountKey(policyId));
    deps.repos.policies.setSkillEligible(policyId, false);
    deps.log.warn("skill.backoff.exhausted", { policyId, count });
  }
}

Generated by cloud-assistant via Open Code Review.

[smoryan]

Thanks — confirmed and fixed in 3d2fe48: the counter is now cleared when the backoff trips, so a manual re-enable starts a fresh window instead of instantly re-tripping off the stale count. Locked by a new test (gives a manually re-enabled policy a fresh backoff window) plus two assertion updates reflecting the cleared-on-trip state. skill domain: 74 passed (12 files), tsc clean.

[Memtensor-AI]

⚠️ Automated Test Results: NO TEST SCOPE

Automated tests were not run because the changed files do not map to an executable test scope.

Details: No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: (none detected)
Manual review or env.yaml source_mapping/execution coverage is required before merge.

Branch: fix/skill-crystallization-backoff