diff --git a/apps/memos-local-plugin/core/config/defaults.ts b/apps/memos-local-plugin/core/config/defaults.ts index 2575a8405..0743c07b7 100644 --- a/apps/memos-local-plugin/core/config/defaults.ts +++ b/apps/memos-local-plugin/core/config/defaults.ts @@ -259,6 +259,14 @@ export const DEFAULT_CONFIG: ResolvedConfig = { archiveEta: 0.1, minEtaForRetrieval: 0.1, idleArchiveMs: 30 * 24 * 60 * 60 * 1000, + // Persistent per-policy back-off for skill crystallization retries + // (issue #2319). Defaults picked to keep transient failures snappy + // (5 min → 10 min → 20 min ...) while permanent-failure policies get + // quarantined after ~10 h of cumulative exponential wait — a >300× + // reduction on the observed 2,640-failures-in-25-days workload. + crystallizationBackoffBaseMs: 5 * 60 * 1000, + crystallizationBackoffMaxMs: 24 * 60 * 60 * 1000, + crystallizationMaxAttempts: 8, }, feedback: { failureThreshold: 3, diff --git a/apps/memos-local-plugin/core/config/index.ts b/apps/memos-local-plugin/core/config/index.ts index ae3991017..42119333f 100644 --- a/apps/memos-local-plugin/core/config/index.ts +++ b/apps/memos-local-plugin/core/config/index.ts @@ -145,6 +145,23 @@ export function resolveConfig(raw: unknown, warnings?: string[], agent?: string) throw new MemosError("config_invalid", `invalid logging.timezone: ${completed.logging.timezone}`); } + // Cross-field: baseMs must not exceed maxMs, otherwise the exponential + // back-off's very first `wait = min(base * 2^0, max)` clamps to `max` + // and the retry curve collapses to a flat delay. The individual field + // ranges overlap (base up to 24h, max down to 1min) so the schema step + // above cannot catch this on its own — issue #2319 PR #2325 review. + { + const base = completed.algorithm.skill.crystallizationBackoffBaseMs; + const max = completed.algorithm.skill.crystallizationBackoffMaxMs; + if (base > max) { + throw new MemosError( + "config_invalid", + `algorithm.skill.crystallizationBackoffBaseMs (${base}) must be <= crystallizationBackoffMaxMs (${max})`, + { base, max }, + ); + } + } + return Object.freeze(completed) as ResolvedConfig; } diff --git a/apps/memos-local-plugin/core/config/schema.ts b/apps/memos-local-plugin/core/config/schema.ts index 9cd79b612..42c39958b 100644 --- a/apps/memos-local-plugin/core/config/schema.ts +++ b/apps/memos-local-plugin/core/config/schema.ts @@ -377,6 +377,26 @@ const AlgorithmSchema = Type.Object({ 60 * 60 * 1000, 365 * 24 * 60 * 60 * 1000, ), + /** + * Initial back-off before retrying a failed crystallization for the + * same policy. Doubles per attempt, capped at `crystallizationBackoffMaxMs`. + * Must not exceed `crystallizationBackoffMaxMs` — cross-checked at + * config-load time (`resolveConfig`) because the individual field ranges + * overlap. See issue #2319 for the retry-storm this replaces. + */ + crystallizationBackoffBaseMs: NumberInRange(5 * 60 * 1000, 1000, 24 * 60 * 60 * 1000), + /** + * Upper bound of the exponential back-off between crystallization retries. + * Must be >= `crystallizationBackoffBaseMs` (see cross-field check in + * `resolveConfig`). + */ + crystallizationBackoffMaxMs: NumberInRange( + 24 * 60 * 60 * 1000, + 60 * 1000, + 30 * 24 * 60 * 60 * 1000, + ), + /** Consecutive failures before a policy is quarantined from crystallization. */ + crystallizationMaxAttempts: NumberInRange(8, 1, 100), }, { default: {} }), feedback: Type.Object({ /** Raise a burst after this many failures of the same tool in-window. */ diff --git a/apps/memos-local-plugin/core/skill/ALGORITHMS.md b/apps/memos-local-plugin/core/skill/ALGORITHMS.md index e1560d926..0b655db19 100644 --- a/apps/memos-local-plugin/core/skill/ALGORITHMS.md +++ b/apps/memos-local-plugin/core/skill/ALGORITHMS.md @@ -299,3 +299,33 @@ A skill run never throws because one policy in a batch misbehaves. Run-level cancellation is never used — the orchestrator instead returns `rejected` / `warnings` counts in `RunSkillResult` so the caller can assert on them. + +### Persistent back-off (issue #2319) + +`runSkill` also persists per-policy failure state on the `policies` +row (migration 019: `crystallization_attempts`, +`crystallization_backoff_until`, `crystallization_last_attempt_at`, +`crystallization_last_failure_reason`). Every failure branch — +`evidence` (no-evidence), `crystallize` (LLM refusal / parse / disabled), +`verify` (coverage or resonance rejection) — calls +`policies.recordCrystallizationFailure` which: + +* increments `attempts` +* schedules `backoffUntil = now + min(baseMs * 2^(attempts-1), maxMs)` +* stamps `lastAttemptAt = now` and the failure reason + +At `attempts >= crystallizationMaxAttempts` the policy is quarantined: +`backoffUntil` is cleared, and eligibility keeps skipping until either +the policy is updated (bumping `updatedAt` past `lastAttemptAt`) or a +successful crystallization calls +`policies.resetCrystallizationFailure`. + +Eligibility auto-invalidates the back-off when a fresh policy update +arrives: if `policy.updatedAt > lastAttemptAt` the guard is ignored and +the retry proceeds. This is the natural rehab path — new evidence +means the previous failure may no longer apply. + +Defaults: `baseMs=5min`, `maxMs=24h`, `maxAttempts=8`. Under the +observed 25-day workload from #2319 this turns 2,640 retries into 8 +retries then quarantine — a >300× budget reduction. + diff --git a/apps/memos-local-plugin/core/skill/README.md b/apps/memos-local-plugin/core/skill/README.md index 04a2de8bf..619ef1710 100644 --- a/apps/memos-local-plugin/core/skill/README.md +++ b/apps/memos-local-plugin/core/skill/README.md @@ -201,7 +201,10 @@ See `algorithm.skill` in | `minSupport` | `2` | Min distinct-episode support to crystallize. | | `minGain` | `0.02` | Min policy gain required (paired with the new shrinkage-anchored gain in `core/memory/l2/gain.ts`). | | `candidateTrials` | `3` | Trials required to transition out of `candidate`. NOTE: legacy docs called this `probationaryTrials`; the schema field is `candidateTrials`. | -| `cooldownMs` | `60000` | Debounce between runs triggered by the same policy. | +| `cooldownMs` | `60000` | Debounce between runs triggered by the same policy. **Documented for legacy YAML files — currently a no-op; see `crystallizationBackoffBaseMs` for the wired-up mechanism.** | +| `crystallizationBackoffBaseMs` | `300000` | Initial wait before retrying a failed crystallization (doubles per attempt, issue #2319). | +| `crystallizationBackoffMaxMs` | `86400000` | Upper bound of the exponential back-off between crystallization retries. | +| `crystallizationMaxAttempts` | `8` | Consecutive failures before a policy is quarantined from crystallization. | | `traceCharCap` | `600` | Char cap per evidence trace in the crystallize prompt.| | `evidenceLimit` | `4` | Max evidence traces per crystallize call. | | `useLlm` | `true` | Toggle the LLM off (tests / degraded mode). | diff --git a/apps/memos-local-plugin/core/skill/eligibility.ts b/apps/memos-local-plugin/core/skill/eligibility.ts index 4e54ba5db..9fd506caa 100644 --- a/apps/memos-local-plugin/core/skill/eligibility.ts +++ b/apps/memos-local-plugin/core/skill/eligibility.ts @@ -10,12 +10,16 @@ * anchor before they can crystallize into a Skill. * 5. It is not already represented by a non-archived skill, OR the existing * skill was built before the policy's latest `updatedAt` (→ rebuild). + * 6. The per-policy crystallization back-off (issue #2319) has not been + * tripped. The check ignores stale state, so a fresh policy update + * (`policy.updatedAt > lastAttemptAt`) always lets a retry through. * * The check returns a structured verdict per policy so the orchestrator can * emit a single rollup event. We never mutate anything here — this module is * read-only on purpose to make it trivially unit-testable. */ +import { now as nowMs } from "../time.js"; import type { PolicyRow, SkillRow } from "../types.js"; import type { SkillConfig } from "./types.js"; @@ -34,6 +38,12 @@ export interface EligibilityInput { * Callers collect this via `skillsRepo.list()` once per run. */ skillsByPolicy: Map; + /** + * Wall-clock time used for back-off comparisons. Optional so existing + * unit tests keep working — production callers thread the same + * `nowMs()` they use elsewhere in `runSkill` for consistent timings. + */ + now?: number; } export interface EligibilityResult { @@ -49,10 +59,11 @@ export function evaluateEligibility( const decisions: EligibilityDecision[] = []; let eligibleCount = 0; let skippedCount = 0; + const now = input.now ?? nowMs(); for (const policy of input.policies) { const existing = input.skillsByPolicy.get(policy.id) ?? null; - const decision = decide(policy, existing, config); + const decision = decide(policy, existing, config, now); decisions.push(decision); if (decision.action === "skip") skippedCount += 1; else eligibleCount += 1; @@ -65,7 +76,33 @@ function decide( policy: PolicyRow, existing: SkillRow | null, cfg: SkillConfig, + now: number, ): EligibilityDecision { + // Back-off / quarantine gate — check first so a permanently-failing + // policy is skipped before we spend cycles on the other gates. The + // guard is skipped entirely when the state is "stale" (i.e., the + // policy itself has been updated since we last tried it), because a + // fresh update likely reflects new evidence and warrants a retry. + // + // Ordering caveat: because this runs before the `policy.status` gate, + // a policy that is *both* quarantined and inactive/archived reports + // the back-off reason, hiding the underlying status. Operators who + // clear a quarantine (via a policy edit that bumps `updatedAt` past + // `lastAttemptAt`) may then see a status-based skip they did not + // expect. That is intentional — quarantine is a "do not retry" signal + // that must dominate any downstream reasoning — so operators should + // consult `policy.status` independently rather than rely on eligibility + // skip reasons alone. + const backoffSkip = evaluateBackoff(policy, cfg, now); + if (backoffSkip !== null) { + return { + policy, + existingSkill: existing, + action: "skip", + reason: backoffSkip, + }; + } + if (policy.status !== "active") { return { policy, @@ -124,6 +161,37 @@ function decide( }; } +/** + * Returns a skip reason string when the policy is currently blocked by + * the crystallization back-off / quarantine gate, or `null` when there + * is no active back-off (either it never failed, or the state is stale). + */ +function evaluateBackoff( + policy: PolicyRow, + cfg: SkillConfig, + now: number, +): string | null { + const bo = policy.crystallizationBackoff; + if (!bo || bo.attempts <= 0) return null; + + // A policy update after the last attempt invalidates the back-off — + // new evidence has arrived, retry immediately. + if (bo.lastAttemptAt !== null && policy.updatedAt > bo.lastAttemptAt) { + return null; + } + + if (bo.attempts >= cfg.crystallizationMaxAttempts) { + const reason = bo.lastFailureReason ?? "unknown"; + return `crystallization-quarantined attempts=${bo.attempts} reason=${reason}`; + } + + if (bo.backoffUntil !== null && now < bo.backoffUntil) { + return `crystallization-backoff attempts=${bo.attempts} until=${bo.backoffUntil}`; + } + + return null; +} + function hasSuccessAnchor(policy: PolicyRow): boolean { if (policy.skillEligible === false) return false; const type = policy.experienceType ?? "success_pattern"; diff --git a/apps/memos-local-plugin/core/skill/skill.ts b/apps/memos-local-plugin/core/skill/skill.ts index d0cdc48ae..15dab35e5 100644 --- a/apps/memos-local-plugin/core/skill/skill.ts +++ b/apps/memos-local-plugin/core/skill/skill.ts @@ -77,7 +77,10 @@ export async function runSkill( const warnings: RunSkillResult["warnings"] = []; const tEligibility = nowMs(); - const eligibility = evaluateEligibility({ policies, skillsByPolicy }, config); + const eligibility = evaluateEligibility( + { policies, skillsByPolicy, now: tEligibility }, + config, + ); timings.eligibility = nowMs() - tEligibility; bus.emit({ @@ -105,6 +108,7 @@ export async function runSkill( }); if (evidence.traces.length === 0) { warnings.push({ policyId: decision.policy.id, reason: "no-evidence" }); + recordCrystallizationFailure(decision.policy.id, "no-evidence", deps); bus.emit({ kind: "skill.failed", at: nowMs(), @@ -146,6 +150,11 @@ export async function runSkill( policyId: decision.policy.id, reason: crystResult.skippedReason, }); + recordCrystallizationFailure( + decision.policy.id, + crystResult.skippedReason, + deps, + ); bus.emit({ kind: "skill.failed", at: nowMs(), @@ -166,10 +175,12 @@ export async function runSkill( if (!verdict.ok) { rejected += 1; + const verifyReason = `verify:${verdict.reason ?? "verify-failed"}`; warnings.push({ policyId: decision.policy.id, - reason: verdict.reason ?? "verify-failed", + reason: verifyReason, }); + recordCrystallizationFailure(decision.policy.id, verifyReason, deps); bus.emit({ kind: "skill.verification.failed", at: nowMs(), @@ -209,6 +220,36 @@ export async function runSkill( } repos.skills.upsert(row); + // Success clears any prior back-off — the next natural policy update + // or reward tick can retry without artificial delay. A failure here + // leaves the policy stuck in whatever backoff / quarantine state it + // was in, so we surface it as both a warning and a `skill.failed` + // bus event (stage=persist) — otherwise operators would only see + // `resetCrystallizationFailure` errors in the log stream, and the + // quarantine would look permanent even after a successful upsert. + try { + repos.policies.resetCrystallizationFailure(decision.policy.id); + } catch (err) { + const errMsg = err instanceof Error ? err.message : String(err); + log.warn("skill.backoff.reset_failed", { + policyId: decision.policy.id, + skillId: row.id, + err: errMsg, + }); + warnings.push({ + policyId: decision.policy.id, + skillId: row.id, + reason: `backoff-reset-failed: ${errMsg}`, + }); + bus.emit({ + kind: "skill.failed", + at: nowMs(), + policyId: decision.policy.id, + skillId: row.id, + stage: "persist", + reason: `backoff-reset-failed: ${errMsg}`, + }); + } if (!row.vec && deps.embedder) { repos.embeddingRetryQueue.enqueue({ id: `er_${ids.span()}`, @@ -338,6 +379,48 @@ export function applySkillFeedback( // ─── Helpers ────────────────────────────────────────────────────────────── +/** + * Persist a per-policy failure into the `policies` row so eligibility can + * skip the policy on the next tick (issue #2319). Isolated from `runSkill` + * so a repo-side error can never leak past the orchestrator and abort + * remaining policies in the batch. + */ +function recordCrystallizationFailure( + policyId: PolicyRow["id"], + reason: string, + deps: RunSkillDeps, +): void { + try { + const result = deps.repos.policies.recordCrystallizationFailure(policyId, { + reason, + baseMs: deps.config.crystallizationBackoffBaseMs, + maxMs: deps.config.crystallizationBackoffMaxMs, + maxAttempts: deps.config.crystallizationMaxAttempts, + now: nowMs(), + }); + if (result.quarantined) { + deps.log.warn("skill.backoff.quarantined", { + policyId, + attempts: result.attempts, + reason, + }); + } else { + deps.log.debug("skill.backoff.recorded", { + policyId, + attempts: result.attempts, + backoffUntil: result.backoffUntil, + reason, + }); + } + } catch (err) { + deps.log.warn("skill.backoff.record_failed", { + policyId, + reason, + err: err instanceof Error ? err.message : String(err), + }); + } +} + function gatherPolicies(input: RunSkillInput, repos: Repos): PolicyRow[] { if (input.policyId) { const single = repos.policies.getById(input.policyId); diff --git a/apps/memos-local-plugin/core/skill/types.ts b/apps/memos-local-plugin/core/skill/types.ts index b04e3bc35..a628d6460 100644 --- a/apps/memos-local-plugin/core/skill/types.ts +++ b/apps/memos-local-plugin/core/skill/types.ts @@ -120,6 +120,26 @@ export interface SkillConfig { minEtaForRetrieval: number; /** Archive a low-η active skill after it has not been retrieved for this long. */ idleArchiveMs: number; + /** + * V7 §2.5 addendum — first back-off between crystallization retries after + * a per-policy failure (`crystallize`, `verify`, or `no-evidence`). + * Doubles per attempt via `min(baseMs * 2^(attempts-1), maxMs)`. Failure + * state is stored on the `policies` row so it survives daemon restarts; + * see issue #2319 for the retry-storm the field was designed to prevent. + */ + crystallizationBackoffBaseMs: number; + /** + * Cap for exponential back-off between crystallization retries. + * Must be >= `crystallizationBackoffBaseMs`. Enforced at config-load + * time in `resolveConfig`. + */ + crystallizationBackoffMaxMs: number; + /** + * After this many consecutive failures the policy is quarantined and + * never retried until its `updatedAt` moves past `lastAttemptAt` (i.e. + * new evidence arrived) or the failure state is explicitly cleared. + */ + crystallizationMaxAttempts: number; } /** diff --git a/apps/memos-local-plugin/core/storage/migrations/019-policy-crystallization-backoff.sql b/apps/memos-local-plugin/core/storage/migrations/019-policy-crystallization-backoff.sql new file mode 100644 index 000000000..f9e99cc65 --- /dev/null +++ b/apps/memos-local-plugin/core/storage/migrations/019-policy-crystallization-backoff.sql @@ -0,0 +1,43 @@ +-- Persistent per-policy back-off state for skill crystallization retries. +-- +-- Before this migration `runSkill` retried failing policies on every trigger +-- (l2.policy.induced / l2.policy.updated / reward.updated) with no state +-- ever written back, and the documented `skill.cooldownMs` config was never +-- wired into the subscriber. A single permanently-failing policy could +-- accumulate thousands of repeated LLM refusals / verifier mismatches over +-- weeks, burning budget and log volume forever (issue #2319: 2,640 repeated +-- failures observed in a 25-day audit of a local deployment). +-- +-- Adding four small columns lets `evaluateEligibility` skip a policy that +-- recently failed (exponential back-off, capped at 24h) or quarantine it +-- entirely after `crystallizationMaxAttempts` (default 8). Auto-invalidation +-- is implicit: whenever `policy.updated_at` moves past +-- `crystallization_last_attempt_at`, the eligibility check treats the +-- back-off state as stale and lets the retry through. +-- +-- All four columns are additive with safe defaults, so pre-migration rows +-- read as "never attempted" and existing installs pick up back-off gating +-- only for policies that fail after this deploy. +-- +-- Runtime note: at plugin boot the migrator (`storage/migrator.ts`) does +-- NOT execute this file via `db.exec`; it dispatches version 19 to +-- `ensurePolicyCrystallizationBackoffColumns`, which uses `ensureColumn` +-- so partial-schema test harnesses (no `policies` table) and re-runs on +-- an already-migrated DB stay idempotent. This file is kept as the +-- authoritative on-disk schema-of-record for tooling that reads the +-- migrations directory directly (dry-run diff, manual recovery, future +-- non-SQLite backend ports); both paths produce the same four columns. +-- If you edit either side, update the other to match. + +ALTER TABLE policies ADD COLUMN crystallization_attempts INTEGER NOT NULL DEFAULT 0; +ALTER TABLE policies ADD COLUMN crystallization_backoff_until INTEGER; +ALTER TABLE policies ADD COLUMN crystallization_last_attempt_at INTEGER; +ALTER TABLE policies ADD COLUMN crystallization_last_failure_reason TEXT; + +-- Partial index for bulk lookups of policies currently inside the back-off +-- window. Not needed by `evaluateEligibility` (which reads the state from +-- the already-loaded `PolicyRow`), but keeps future health-check dashboards, +-- bulk-reset commands, and audit queries off a full table scan. +CREATE INDEX IF NOT EXISTS idx_policies_crystallization_backoff + ON policies(crystallization_backoff_until) + WHERE crystallization_backoff_until IS NOT NULL; diff --git a/apps/memos-local-plugin/core/storage/migrator.ts b/apps/memos-local-plugin/core/storage/migrator.ts index b858f7aa3..0bb9caade 100644 --- a/apps/memos-local-plugin/core/storage/migrator.ts +++ b/apps/memos-local-plugin/core/storage/migrator.ts @@ -210,6 +210,16 @@ function applyMigration(db: StorageDb, file: MigrationFile): void { } return; } + if (file.version === 19 && file.name === "policy-crystallization-backoff") { + // Additive columns via ensureColumn so partial-schema test harnesses + // (no `policies` table) do not blow up on boot, and re-running the + // migrator on a DB that already has the columns is a no-op. The SQL + // file (migrations/019-policy-crystallization-backoff.sql) documents + // the same schema for tooling that reads the migrations dir directly; + // keep both sides in sync. + ensurePolicyCrystallizationBackoffColumns(db); + return; + } db.exec(fs.readFileSync(file.fullPath, "utf8")); } @@ -357,6 +367,25 @@ function ensureFeedbackExperienceMetadataColumns(db: StorageDb): void { db.exec(`CREATE INDEX IF NOT EXISTS idx_policies_skill_eligible ON policies(skill_eligible, status, updated_at DESC)`); } +function ensurePolicyCrystallizationBackoffColumns(db: StorageDb): void { + if (!tableExists(db, "policies")) return; + ensureColumn( + db, + "policies", + "crystallization_attempts", + "INTEGER NOT NULL DEFAULT 0", + ); + ensureColumn(db, "policies", "crystallization_backoff_until", "INTEGER"); + ensureColumn(db, "policies", "crystallization_last_attempt_at", "INTEGER"); + ensureColumn(db, "policies", "crystallization_last_failure_reason", "TEXT"); + // Partial index — keep in sync with 019-policy-crystallization-backoff.sql. + db.exec( + `CREATE INDEX IF NOT EXISTS idx_policies_crystallization_backoff + ON policies(crystallization_backoff_until) + WHERE crystallization_backoff_until IS NOT NULL`, + ); +} + function ensureHubSharingSearchColumns(db: StorageDb): void { if (!tableExists(db, "hub_shared_memories")) return; ensureColumn(db, "hub_shared_memories", "embedding", "BLOB"); diff --git a/apps/memos-local-plugin/core/storage/repos/policies.ts b/apps/memos-local-plugin/core/storage/repos/policies.ts index 29920f60a..8b0df6bd5 100644 --- a/apps/memos-local-plugin/core/storage/repos/policies.ts +++ b/apps/memos-local-plugin/core/storage/repos/policies.ts @@ -1,4 +1,4 @@ -import type { EmbeddingVector, PolicyId, PolicyRow, ShareScope } from "../../types.js"; +import type { EmbeddingVector, EpochMs, PolicyId, PolicyRow, ShareScope } from "../../types.js"; import type { PolicyListFilter, StorageDb } from "../types.js"; import { buildInsert, buildUpdate } from "../tx.js"; import { scanAndTopK, type VectorHit } from "../vector.js"; @@ -46,6 +46,10 @@ const COLUMNS = [ "share_target", "shared_at", "edited_at", + "crystallization_attempts", + "crystallization_backoff_until", + "crystallization_last_attempt_at", + "crystallization_last_failure_reason", ]; export interface PolicySearchMeta { @@ -386,6 +390,102 @@ export function makePoliciesRepo(db: StorageDb) { ).run({ id, vec: toBlob(vec)!, updated_at: Date.now() }); return res.changes > 0; }, + + /** + * Record a failed crystallization attempt and schedule the next retry + * via exponential back-off. Returns the resulting state so the caller + * can log / emit metrics without re-reading the row. + * + * At `attempts >= maxAttempts` the policy enters "quarantine": we clear + * `backoff_until` (there is no scheduled retry) but keep the attempt + * counter, so `evaluateEligibility` continues to skip until the policy + * itself is updated (bumping `updated_at` past `last_attempt_at`) or + * `resetCrystallizationFailure` is called explicitly. + * + * See openspec/changes/2026-09-02-2319-pre-submission-checklist/design.md. + */ + recordCrystallizationFailure( + id: PolicyId, + opts: { + reason: string; + baseMs: number; + maxMs: number; + maxAttempts: number; + now: number; + }, + ): { attempts: number; backoffUntil: number | null; quarantined: boolean } { + // Read-modify-write must be atomic — two concurrent writers reading + // the same `attempts` would both write `previous+1`, under-counting + // failures and letting a permanently-failing policy retry past the + // quarantine threshold. Wrap in a transaction so SQLite serializes + // the pair. We also throw when the policy row is missing so a caller + // typo or a race with `deleteById` surfaces as a logged warning + // (`skill.backoff.record_failed`) instead of silently reporting a + // successful record for a row that never got updated. + return db.tx(() => { + const row = db.prepare< + { id: string }, + { attempts: number | null } + >( + `SELECT crystallization_attempts AS attempts FROM policies WHERE id=@id`, + ).get({ id }); + if (!row) { + throw new Error( + `recordCrystallizationFailure: policy ${id} not found`, + ); + } + const previous = row.attempts ?? 0; + const attempts = previous + 1; + const quarantined = attempts >= opts.maxAttempts; + let backoffUntil: number | null; + if (quarantined) { + backoffUntil = null; + } else { + // 2^(attempts-1) grows fast; clamp the shift so it can't overflow + // JS safe-integer arithmetic for very large maxAttempts settings. + const exp = Math.max(0, Math.min(attempts - 1, 30)); + const wait = Math.min(opts.baseMs * 2 ** exp, opts.maxMs); + backoffUntil = opts.now + wait; + } + db.prepare<{ + id: string; + attempts: number; + backoff_until: number | null; + last_attempt_at: number; + last_failure_reason: string; + }>( + `UPDATE policies + SET crystallization_attempts = @attempts, + crystallization_backoff_until = @backoff_until, + crystallization_last_attempt_at = @last_attempt_at, + crystallization_last_failure_reason = @last_failure_reason + WHERE id = @id`, + ).run({ + id, + attempts, + backoff_until: backoffUntil, + last_attempt_at: opts.now, + last_failure_reason: opts.reason, + }); + return { attempts, backoffUntil, quarantined }; + }); + }, + + /** + * Clear the per-policy crystallization back-off state. Called after a + * successful crystallization / rebuild so the next natural policy + * update or reward tick can retry without artificial delay. + */ + resetCrystallizationFailure(id: PolicyId): void { + db.prepare<{ id: string }>( + `UPDATE policies + SET crystallization_attempts = 0, + crystallization_backoff_until = NULL, + crystallization_last_attempt_at = NULL, + crystallization_last_failure_reason = NULL + WHERE id = @id`, + ).run({ id }); + }, }; } @@ -420,6 +520,10 @@ interface RawPolicyRow { share_target: string | null; shared_at: number | null; edited_at: number | null; + crystallization_attempts: number | null; + crystallization_backoff_until: number | null; + crystallization_last_attempt_at: number | null; + crystallization_last_failure_reason: string | null; } type RawPolicySearchRow = Pick< @@ -444,6 +548,7 @@ const EMPTY_GUIDANCE: PolicyRow["decisionGuidance"] = Object.freeze({ }); function rowToParams(row: PolicyRow): Record { + const bo = row.crystallizationBackoff ?? null; return { id: row.id, ...ownerParamsFromRow(row), @@ -476,6 +581,10 @@ function rowToParams(row: PolicyRow): Record { share_target: row.share?.target ?? null, shared_at: row.share?.sharedAt ?? null, edited_at: row.editedAt ?? null, + crystallization_attempts: bo?.attempts ?? 0, + crystallization_backoff_until: bo?.backoffUntil ?? null, + crystallization_last_attempt_at: bo?.lastAttemptAt ?? null, + crystallization_last_failure_reason: bo?.lastFailureReason ?? null, }; } @@ -517,6 +626,19 @@ function mapRow(r: RawPolicyRow): PolicyRow { } : null, editedAt: r.edited_at, + crystallizationBackoff: + r.crystallization_attempts != null && r.crystallization_attempts > 0 + ? { + attempts: r.crystallization_attempts, + backoffUntil: (r.crystallization_backoff_until ?? null) as + | EpochMs + | null, + lastAttemptAt: (r.crystallization_last_attempt_at ?? null) as + | EpochMs + | null, + lastFailureReason: r.crystallization_last_failure_reason ?? null, + } + : null, }; } diff --git a/apps/memos-local-plugin/core/types.ts b/apps/memos-local-plugin/core/types.ts index 0e4f87f52..31fb6c662 100644 --- a/apps/memos-local-plugin/core/types.ts +++ b/apps/memos-local-plugin/core/types.ts @@ -203,6 +203,24 @@ export interface PolicyRow extends OwnedRow { } | null; /** Last user edit through the viewer's edit modal (migration 009). */ editedAt?: EpochMs | null; + /** + * V7 §2.5 addendum — persistent back-off state for skill crystallization + * retries (migration 019). Absent / `null` = never attempted or cleanly + * reset (`resetCrystallizationFailure` writes 0 to the DB, which `mapRow` + * hides from callers). When present, `attempts` is always >= 1. When + * `updatedAt > lastAttemptAt` the state is considered stale and + * eligibility ignores it (letting a fresh policy update through + * immediately). See + * `openspec/changes/2026-09-02-2319-pre-submission-checklist/design.md` + * for the full rationale. + */ + crystallizationBackoff?: { + /** Always >= 1 when this object is present. */ + attempts: number; + backoffUntil: EpochMs | null; + lastAttemptAt: EpochMs | null; + lastFailureReason: string | null; + } | null; } /** diff --git a/apps/memos-local-plugin/tests/unit/config/load.test.ts b/apps/memos-local-plugin/tests/unit/config/load.test.ts index 2d9f22f81..cd89f2b29 100644 --- a/apps/memos-local-plugin/tests/unit/config/load.test.ts +++ b/apps/memos-local-plugin/tests/unit/config/load.test.ts @@ -68,6 +68,45 @@ describe("config/loadConfig", () => { } }); + it("rejects crystallization back-off base > max with config_invalid (issue #2319)", () => { + // Both fields individually valid, but the pair inverts the exponential + // curve into a flat retry — schema ranges overlap so only the + // cross-field guard in resolveConfig catches this. + const raw = { + algorithm: { + skill: { + crystallizationBackoffBaseMs: 10 * 60 * 60 * 1000, // 10h + crystallizationBackoffMaxMs: 60 * 1000, // 1min + }, + }, + }; + expect(() => resolveConfig(raw)).toThrow(MemosError); + try { + resolveConfig(raw); + throw new Error("expected resolveConfig to reject inverted back-off"); + } catch (err) { + expect(MemosError.is(err)).toBe(true); + expect((err as MemosError).code).toBe("config_invalid"); + expect((err as MemosError).message).toMatch( + /crystallizationBackoffBaseMs.*must be <=.*crystallizationBackoffMaxMs/, + ); + } + }); + + it("accepts crystallization back-off base == max", () => { + const raw = { + algorithm: { + skill: { + crystallizationBackoffBaseMs: 60 * 60 * 1000, + crystallizationBackoffMaxMs: 60 * 60 * 1000, + }, + }, + }; + const cfg = resolveConfig(raw); + expect(cfg.algorithm.skill.crystallizationBackoffBaseMs).toBe(60 * 60 * 1000); + expect(cfg.algorithm.skill.crystallizationBackoffMaxMs).toBe(60 * 60 * 1000); + }); + it("defaults OpenRouter provider routing lists to empty arrays", () => { const cfg = resolveConfig({}); expect(cfg.llm.providerIgnore).toEqual([]); diff --git a/apps/memos-local-plugin/tests/unit/skill/_helpers.ts b/apps/memos-local-plugin/tests/unit/skill/_helpers.ts index 9cef40506..96270fed1 100644 --- a/apps/memos-local-plugin/tests/unit/skill/_helpers.ts +++ b/apps/memos-local-plugin/tests/unit/skill/_helpers.ts @@ -41,6 +41,9 @@ export function makeSkillConfig(partial: Partial = {}): SkillConfig archiveEta: 0.1, minEtaForRetrieval: 0.1, idleArchiveMs: 30 * 24 * 60 * 60 * 1000, + crystallizationBackoffBaseMs: 5 * 60 * 1000, + crystallizationBackoffMaxMs: 24 * 60 * 60 * 1000, + crystallizationMaxAttempts: 8, ...partial, }; } diff --git a/apps/memos-local-plugin/tests/unit/skill/eligibility.test.ts b/apps/memos-local-plugin/tests/unit/skill/eligibility.test.ts index ba1a4875d..034b52fba 100644 --- a/apps/memos-local-plugin/tests/unit/skill/eligibility.test.ts +++ b/apps/memos-local-plugin/tests/unit/skill/eligibility.test.ts @@ -36,6 +36,7 @@ function mkPolicy(partial: Partial): PolicyRow { vec: partial.vec ?? null, createdAt: (partial.createdAt ?? BASE_POLICY.createdAt) as PolicyRow["createdAt"], updatedAt: (partial.updatedAt ?? BASE_POLICY.updatedAt) as PolicyRow["updatedAt"], + crystallizationBackoff: partial.crystallizationBackoff, }; } @@ -146,4 +147,96 @@ describe("skill/eligibility", () => { expect(r.decisions[0]!.action).toBe("crystallize"); expect(r.decisions[0]!.existingSkill?.status).toBe("archived"); }); + + // ─── Crystallization back-off gate (issue #2319) ───────────────────── + + it("skips a policy whose back-off window has not yet elapsed", () => { + const cfg = makeSkillConfig({ + crystallizationBackoffBaseMs: 60_000, + crystallizationBackoffMaxMs: 3_600_000, + crystallizationMaxAttempts: 8, + }); + const policy = mkPolicy({ + support: 5, + gain: 0.4, + updatedAt: 1_000 as PolicyRow["updatedAt"], + crystallizationBackoff: { + attempts: 1, + backoffUntil: 60_000 as PolicyRow["updatedAt"], + lastAttemptAt: 5_000 as PolicyRow["updatedAt"], + lastFailureReason: "llm-refusal", + }, + }); + const r = evaluateEligibility( + { policies: [policy], skillsByPolicy: new Map(), now: 30_000 }, + cfg, + ); + expect(r.decisions[0]!.action).toBe("skip"); + expect(r.decisions[0]!.reason).toMatch(/^crystallization-backoff attempts=1 until=60000$/); + }); + + it("proceeds once the back-off window has elapsed", () => { + const cfg = makeSkillConfig({ + crystallizationBackoffBaseMs: 60_000, + crystallizationMaxAttempts: 8, + }); + const policy = mkPolicy({ + support: 5, + gain: 0.4, + updatedAt: 1_000 as PolicyRow["updatedAt"], + crystallizationBackoff: { + attempts: 1, + backoffUntil: 60_000 as PolicyRow["updatedAt"], + lastAttemptAt: 5_000 as PolicyRow["updatedAt"], + lastFailureReason: "llm-refusal", + }, + }); + const r = evaluateEligibility( + { policies: [policy], skillsByPolicy: new Map(), now: 60_001 }, + cfg, + ); + expect(r.decisions[0]!.action).toBe("crystallize"); + }); + + it("quarantines a policy once max attempts is reached", () => { + const cfg = makeSkillConfig({ crystallizationMaxAttempts: 3 }); + const policy = mkPolicy({ + support: 5, + gain: 0.4, + updatedAt: 100 as PolicyRow["updatedAt"], + crystallizationBackoff: { + attempts: 3, + backoffUntil: null, + lastAttemptAt: 1_000 as PolicyRow["updatedAt"], + lastFailureReason: "verify:coverage-below-threshold", + }, + }); + const r = evaluateEligibility( + { policies: [policy], skillsByPolicy: new Map(), now: 999_999_999 }, + cfg, + ); + expect(r.decisions[0]!.action).toBe("skip"); + expect(r.decisions[0]!.reason).toContain("crystallization-quarantined attempts=3"); + expect(r.decisions[0]!.reason).toContain("reason=verify:coverage-below-threshold"); + }); + + it("ignores back-off when the policy has been updated since the last attempt", () => { + const cfg = makeSkillConfig({ crystallizationMaxAttempts: 8 }); + const policy = mkPolicy({ + support: 5, + gain: 0.4, + updatedAt: 10_000 as PolicyRow["updatedAt"], + crystallizationBackoff: { + attempts: 5, + backoffUntil: 999_999_999 as PolicyRow["updatedAt"], + lastAttemptAt: 5_000 as PolicyRow["updatedAt"], + lastFailureReason: "verify:resonance-below-threshold", + }, + }); + const r = evaluateEligibility( + { policies: [policy], skillsByPolicy: new Map(), now: 6_000 }, + cfg, + ); + expect(r.decisions[0]!.action).toBe("crystallize"); + }); }); diff --git a/apps/memos-local-plugin/tests/unit/skill/skill.integration.test.ts b/apps/memos-local-plugin/tests/unit/skill/skill.integration.test.ts index 476134abc..49b4d185f 100644 --- a/apps/memos-local-plugin/tests/unit/skill/skill.integration.test.ts +++ b/apps/memos-local-plugin/tests/unit/skill/skill.integration.test.ts @@ -323,4 +323,105 @@ describe("skill/runSkill (integration)", () => { expect(r.warnings[0]?.reason).toBe("no-evidence"); expect(events.some((e) => e.kind === "skill.failed")).toBe(true); }); + + // ─── Crystallization back-off regression (issue #2319) ─────────────── + + it("does not re-invoke the LLM while the back-off window is active", async () => { + const h = open(); + const { policyId } = seedFullCandidate(h); + const llm = fakeLlm({ + completeJson: { + "skill.crystallize": makeDraft({ + summary: "I am Claude, made by Anthropic. I cannot process this request.", + }), + }, + }); + + const { deps } = makeDeps(h, { + llm, + config: makeSkillConfig({ + crystallizationBackoffBaseMs: 60_000, + crystallizationBackoffMaxMs: 3_600_000, + crystallizationMaxAttempts: 8, + }), + }); + + const first = await runSkill({ trigger: "manual", policyId }, deps); + expect(first.rejected).toBe(1); + const requestsAfterFirst = llm.stats().requests; + expect(requestsAfterFirst).toBe(1); + + // Second run under the same wall-clock landed inside the back-off + // window: the LLM must not be invoked again. + const second = await runSkill({ trigger: "manual", policyId }, deps); + expect(second.evaluated).toBe(0); + expect(second.rejected).toBe(0); + expect(llm.stats().requests).toBe(requestsAfterFirst); + + const stored = h.repos.policies.getById(policyId)!; + expect(stored.crystallizationBackoff?.attempts).toBe(1); + expect(stored.crystallizationBackoff?.lastFailureReason).toBe("llm-refusal"); + }); + + it("clears back-off state after a successful crystallization", async () => { + const h = open(); + const { policyId } = seedFullCandidate(h); + // Pre-seed a failure state on the row so we can confirm success wipes it. + h.repos.policies.recordCrystallizationFailure(policyId, { + reason: "llm-refusal", + baseMs: 60_000, + maxMs: 3_600_000, + maxAttempts: 8, + now: 1_000, + }); + // Move the policy timestamp past lastAttemptAt so the back-off is + // "stale" and the run proceeds without waiting. + const p = h.repos.policies.getById(policyId)!; + h.repos.policies.upsert({ + ...p, + updatedAt: 999_999_999 as typeof p.updatedAt, + }); + + const { deps } = makeDeps(h); + const r = await runSkill({ trigger: "manual", policyId }, deps); + expect(r.crystallized).toBe(1); + + const after = h.repos.policies.getById(policyId)!; + expect(after.crystallizationBackoff).toBeNull(); + }); + + it("quarantines a policy after crystallizationMaxAttempts consecutive failures", async () => { + const h = open(); + const { policyId } = seedFullCandidate(h); + const { deps } = makeDeps(h, { + llm: fakeLlm({ + completeJson: { + "skill.crystallize": makeDraft({ + summary: "I am Claude, made by Anthropic. I cannot process this request.", + }), + }, + }), + config: makeSkillConfig({ + crystallizationBackoffBaseMs: 1, + crystallizationBackoffMaxMs: 5, + crystallizationMaxAttempts: 2, + }), + }); + + // Attempt 1: records failure, schedules a back-off ≤ 5 ms. + await runSkill({ trigger: "manual", policyId }, deps); + // Wait the tiny back-off out. + await new Promise((r) => setTimeout(r, 15)); + // Attempt 2: records failure, hits maxAttempts, quarantines. + await runSkill({ trigger: "manual", policyId }, deps); + + const after = h.repos.policies.getById(policyId)!; + expect(after.crystallizationBackoff?.attempts).toBe(2); + expect(after.crystallizationBackoff?.backoffUntil).toBeNull(); + + // A third run must skip the policy entirely — no attempted crystallize. + const third = await runSkill({ trigger: "manual", policyId }, deps); + expect(third.evaluated).toBe(0); + expect(third.rejected).toBe(0); + }); }); diff --git a/apps/memos-local-plugin/tests/unit/storage/repos.test.ts b/apps/memos-local-plugin/tests/unit/storage/repos.test.ts index ea1904901..382f3d680 100644 --- a/apps/memos-local-plugin/tests/unit/storage/repos.test.ts +++ b/apps/memos-local-plugin/tests/unit/storage/repos.test.ts @@ -261,6 +261,87 @@ describe("storage/repos — happy paths", () => { } }); + it("policies: recordCrystallizationFailure throws on missing policy (issue #2319)", () => { + // A silent success would let a caller typo or race with deleteById log + // a fake back-off record — the throw surfaces the anomaly instead. + const { repos, cleanup } = makeTmpDb(); + try { + expect(() => + repos.policies.recordCrystallizationFailure("p_does_not_exist", { + reason: "unit-test", + baseMs: 1_000, + maxMs: 60_000, + maxAttempts: 8, + now: 1, + }), + ).toThrow(/policy p_does_not_exist not found/); + } finally { + cleanup(); + } + }); + + it("policies: recordCrystallizationFailure increments monotonically across calls", () => { + // Atomic RMW inside a transaction — sequential calls always advance + // `attempts` by 1 with the correct back-off. + const { repos, cleanup } = makeTmpDb(); + try { + repos.policies.insert({ + id: "p_bo", + title: "bo", + trigger: "", + procedure: "", + verification: "", + boundary: "", + support: 1, + gain: 0, + status: "active", + sourceEpisodeIds: [], + inducedBy: "proto", + decisionGuidance: { preference: [], antiPattern: [] }, + vec: vec([1, 0]), + createdAt: 1, + updatedAt: 1, + }); + const r1 = repos.policies.recordCrystallizationFailure("p_bo", { + reason: "verify:coverage-below-threshold", + baseMs: 1_000, + maxMs: 60_000, + maxAttempts: 3, + now: 100, + }); + expect(r1.attempts).toBe(1); + expect(r1.quarantined).toBe(false); + expect(r1.backoffUntil).toBe(1_100); + const r2 = repos.policies.recordCrystallizationFailure("p_bo", { + reason: "verify:coverage-below-threshold", + baseMs: 1_000, + maxMs: 60_000, + maxAttempts: 3, + now: 200, + }); + expect(r2.attempts).toBe(2); + expect(r2.quarantined).toBe(false); + const r3 = repos.policies.recordCrystallizationFailure("p_bo", { + reason: "verify:coverage-below-threshold", + baseMs: 1_000, + maxMs: 60_000, + maxAttempts: 3, + now: 300, + }); + expect(r3.attempts).toBe(3); + expect(r3.quarantined).toBe(true); + expect(r3.backoffUntil).toBeNull(); + const stored = repos.policies.getById("p_bo")!; + expect(stored.crystallizationBackoff?.attempts).toBe(3); + expect(stored.crystallizationBackoff?.backoffUntil).toBeNull(); + expect(stored.crystallizationBackoff?.lastFailureReason).toBe( + "verify:coverage-below-threshold", + ); + } finally { + cleanup(); + } + }); + it("skills: insert + bumpTrial + unique name constraint", () => { const { repos, cleanup } = makeTmpDb(); try {