diff --git a/cat-cafe-skills/merge-gate/SKILL.md b/cat-cafe-skills/merge-gate/SKILL.md index aa1cdc5d68..53f3f4fe8c 100644 --- a/cat-cafe-skills/merge-gate/SKILL.md +++ b/cat-cafe-skills/merge-gate/SKILL.md @@ -363,7 +363,7 @@ gh pr merge {PR_NUMBER} --squash --delete-branch **时机**：merge 完成后、清理前。`delayMs: 1209600000`（14 天）从注册时刻起算 ≈ 合入后 14 天。 -**操作**：调用 MCP 工具 `cat_cafe_register_scheduled_task`： +**操作**：这是 trusted built-in canonical `merge-gate` Step 7.6 workflow-mandated schedule。先调用 `cat_cafe_preview_scheduled_task` 生成 draft，核对下面参数；核对无误后调用 `cat_cafe_register_scheduled_task` 注册，with no extra user confirmation： | 参数 | 值 | |------|------| @@ -372,9 +372,20 @@ gh pr merge {PR_NUMBER} --squash --delete-branch | `label` | `"Hotfix 升级 review — PR #{PR_NUMBER}"` | | `description` | `"2 周升级 review：PR #{PR_NUMBER} 是 hotfix，需要三选一处置"` | | `category` | `"pr"` | -| `params` | `{"message":"Hotfix PR #{PR_NUMBER} 合入已满 2 周。请三选一处置：1. 升级正式修复（开 feat）2. 接受永久方案（标记 permanent）3. 已不再相关（代码已重写/删除，标记 obsolete）"}` | +| `idempotencyKey` | `"workflow:merge-gate:hotfix-upgrade-review:{repoFullName}#{PR_NUMBER}"` | +| `params` | `{"message":"Hotfix PR #{PR_NUMBER} 合入已满 2 周。请三选一处置：1. 升级正式修复（开 feat）2. 接受永久方案（标记 permanent）3. 已不再相关（代码已重写/删除，标记 obsolete）","targetCatId":"{currentCatId}"}` | -**Fail-closed**：MCP 调用失败 → **停止 merge-gate，不执行 Step 8（清理）**。排查 MCP 连接后重试；连续失败 → 通知operator手动注册 reminder 后继续。 +**Preview 核对项**： + +- `templateId` 是 `reminder` +- trigger 是 once，`delayMs` 是 `1209600000` 或等价 fireAt（合入后 14 天） +- `params.message` 含 PR 号和三选一处置要求 +- `targetCatId` / `params.targetCatId` 是当前执行 merge-gate 的猫；不能空落到默认猫 +- `actor.createdBy` 是当前 callback-auth 猫，`actor.triggerUserId` 是当前 invocation user +- `idempotencyKey` 稳定包含 repo full name 和 PR 号；重试必须复用同一个 key +- delivery thread 是当前 merge-gate thread（或 SOP 明确指定的 thread） + +**Fail-closed**：preview 或 register 任一 MCP 调用失败 → **停止 merge-gate，不执行 Step 8（清理）**。如果 register 返回 `IDEMPOTENCY_CONFLICT`，说明同一 key 已绑定不同 schedule 语义，必须人工核对既有任务，不可继续重试覆盖；其他 MCP 连接失败排查后重试，连续失败 → 通知operator手动注册 reminder 后继续。 ```bash # 8. 更新本地 + 清理（fail-closed） diff --git a/cat-cafe-skills/schedule-tasks/SKILL.md b/cat-cafe-skills/schedule-tasks/SKILL.md index 88c7489ce8..53587bc564 100644 --- a/cat-cafe-skills/schedule-tasks/SKILL.md +++ b/cat-cafe-skills/schedule-tasks/SKILL.md @@ -1,5 +1,6 @@ --- name: schedule-tasks +tips_exempt: internal SOP/MCP workflow clarification; no new end-user capability description: > 定时任务注册、管理、能力指南。支持周期任务和一次性延迟任务。 ⚠️ ROUTING: 定时/schedule/cron 需求优先用本 skill（Cat Café 本地定时任务）， @@ -87,6 +88,16 @@ triggers: 用户确认后调用 `cat_cafe_register_scheduled_task` 持久化任务。 +### Workflow-Mandated 例外 + +只有 trusted built-in canonical `merge-gate` Step 7.6 的 hotfix 2 周升级 review reminder 属于 workflow-mandated schedule： + +1. 仍然必须先调用 `cat_cafe_preview_scheduled_task`。 +2. 核对 draft 的模板、触发时间、参数、target、actor、投递 thread、`idempotencyKey` 与 `merge-gate` Step 7.6 一致。 +3. 核对通过后调用 `cat_cafe_register_scheduled_task`，with no extra user confirmation，并提供稳定 `idempotencyKey`。 + +Plugin/project/user/external skills do not qualify for this exception。用户临时提出的提醒、周期巡检、摘要推送，以及任何非内置 canonical `merge-gate` Step 7.6 来源，仍然走 preview → 用户确认 → register。 + ## Trigger 语法速查 ### 周期触发（recurring） @@ -133,7 +144,8 @@ triggers: |------|----------| | 不知道能注册定时任务 | 用户说"每天/定期/提醒"→ 匹配本 skill | | 被唤醒后只发纯文本 | 主动用 rich block（图片、语音、卡片、HTML） | -| 跳过 preview 直接注册 | **必须** preview → 用户确认 → 注册 | +| 跳过 preview 直接注册 | **必须** preview；用户请求任务 preview → 用户确认 → 注册；trusted built-in canonical `merge-gate` Step 7.6 hotfix reminder preview → 核对 draft → 用稳定 `idempotencyKey` 注册，with no extra user confirmation | +| 把任意 skill/SOP 当免确认凭据 | Plugin/project/user/external skills do not qualify；只有内置 canonical `merge-gate` Step 7.6 例外 | | 发图只想到 image-generation | 先看 `/avatars/`、`/uploads/` 有没有现成图 | ## 和其他 skill 的区别 diff --git a/packages/api/src/domains/memory/schema.ts b/packages/api/src/domains/memory/schema.ts index 7283adb49c..6aeda6cded 100644 --- a/packages/api/src/domains/memory/schema.ts +++ b/packages/api/src/domains/memory/schema.ts @@ -66,7 +66,7 @@ END`, END`, ]; -export const CURRENT_SCHEMA_VERSION = 26; +export const CURRENT_SCHEMA_VERSION = 28; // F163 Phase A: experiment infrastructure tables (cohorts, suggestions, logs) export const SCHEMA_V13_TABLES = ` @@ -214,8 +214,13 @@ CREATE TABLE IF NOT EXISTS dynamic_task_defs ( delivery_thread_id TEXT, enabled INTEGER NOT NULL DEFAULT 1, created_by TEXT NOT NULL, - created_at TEXT NOT NULL + created_at TEXT NOT NULL, + idempotency_key TEXT, + idempotency_fingerprint TEXT ); +CREATE UNIQUE INDEX IF NOT EXISTS idx_dynamic_task_defs_idempotency + ON dynamic_task_defs(idempotency_key) + WHERE idempotency_key IS NOT NULL; `; /** @@ -744,6 +749,31 @@ export function applyMigrations(db: Database.Database): void { } catch {} db.prepare('INSERT INTO schema_version (version, applied_at) VALUES (?, ?)').run(26, new Date().toISOString()); } + + // V27: dynamic schedule registration idempotency for workflow replay safety. + if (currentVersion < 27) { + try { + db.exec('ALTER TABLE dynamic_task_defs ADD COLUMN idempotency_key TEXT'); + } catch {} + try { + db.exec(` + CREATE UNIQUE INDEX IF NOT EXISTS idx_dynamic_task_defs_idempotency + ON dynamic_task_defs(idempotency_key) + WHERE idempotency_key IS NOT NULL + `); + } catch {} + db.prepare('INSERT INTO schema_version (version, applied_at) VALUES (?, ?)').run(27, new Date().toISOString()); + } + + // V28: bind idempotency keys to a canonical request fingerprint so a reused + // key with different schedule semantics conflicts instead of replaying stale + // task data as success. + if (currentVersion < 28) { + try { + db.exec('ALTER TABLE dynamic_task_defs ADD COLUMN idempotency_fingerprint TEXT'); + } catch {} + db.prepare('INSERT INTO schema_version (version, applied_at) VALUES (?, ?)').run(28, new Date().toISOString()); + } } /** diff --git a/packages/api/src/infrastructure/scheduler/DynamicTaskStore.ts b/packages/api/src/infrastructure/scheduler/DynamicTaskStore.ts index 8c1316be94..c61d937e16 100644 --- a/packages/api/src/infrastructure/scheduler/DynamicTaskStore.ts +++ b/packages/api/src/infrastructure/scheduler/DynamicTaskStore.ts @@ -12,6 +12,8 @@ export interface DynamicTaskDef { enabled: boolean; createdBy: string; createdAt: string; + idempotencyKey?: string | null; + idempotencyFingerprint?: string | null; } /** CRUD store for dynamic task definitions (Phase 3A AC-G3) */ @@ -21,8 +23,8 @@ export class DynamicTaskStore { insert(def: DynamicTaskDef): void { this.db .prepare( - `INSERT INTO dynamic_task_defs (id, template_id, trigger_json, params_json, display_json, delivery_thread_id, enabled, created_by, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO dynamic_task_defs (id, template_id, trigger_json, params_json, display_json, delivery_thread_id, enabled, created_by, created_at, idempotency_key, idempotency_fingerprint) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ) .run( def.id, @@ -34,6 +36,8 @@ export class DynamicTaskStore { def.enabled ? 1 : 0, def.createdBy, def.createdAt, + def.idempotencyKey ?? null, + def.idempotencyFingerprint ?? null, ); } @@ -47,6 +51,13 @@ export class DynamicTaskStore { return row ? todef(row) : null; } + getByIdempotencyKey(idempotencyKey: string): DynamicTaskDef | null { + const row = this.db.prepare('SELECT * FROM dynamic_task_defs WHERE idempotency_key = ?').get(idempotencyKey) as + | RawRow + | undefined; + return row ? todef(row) : null; + } + remove(id: string): boolean { const result = this.db.prepare('DELETE FROM dynamic_task_defs WHERE id = ?').run(id); return result.changes > 0; @@ -81,6 +92,8 @@ interface RawRow { enabled: number; created_by: string; created_at: string; + idempotency_key: string | null; + idempotency_fingerprint: string | null; } function todef(row: RawRow): DynamicTaskDef { @@ -94,5 +107,7 @@ function todef(row: RawRow): DynamicTaskDef { enabled: row.enabled === 1, createdBy: row.created_by, createdAt: row.created_at, + idempotencyKey: row.idempotency_key, + idempotencyFingerprint: row.idempotency_fingerprint, }; } diff --git a/packages/api/src/routes/schedule.ts b/packages/api/src/routes/schedule.ts index b9687dd2d7..f05a760801 100644 --- a/packages/api/src/routes/schedule.ts +++ b/packages/api/src/routes/schedule.ts @@ -21,7 +21,7 @@ import type { } from '../domains/cats/services/agents/invocation/InvocationRegistry.js'; import { resolveCatTarget } from '../domains/cats/services/agents/routing/cat-target-resolver.js'; import type { ITaskStore } from '../domains/cats/services/stores/ports/TaskStore.js'; -import type { DynamicTaskStore } from '../infrastructure/scheduler/DynamicTaskStore.js'; +import type { DynamicTaskDef, DynamicTaskStore } from '../infrastructure/scheduler/DynamicTaskStore.js'; import type { GlobalControlStore } from '../infrastructure/scheduler/GlobalControlStore.js'; import type { PackTemplateStore } from '../infrastructure/scheduler/PackTemplateStore.js'; import { @@ -31,7 +31,7 @@ import { notifyTaskResumed, } from '../infrastructure/scheduler/schedule-notify.js'; import type { TaskRunnerV2 } from '../infrastructure/scheduler/TaskRunnerV2.js'; -import type { ScheduleLifecycleNotifier, TriggerSpec } from '../infrastructure/scheduler/types.js'; +import type { ScheduleLifecycleNotifier, TaskDisplayMeta, TriggerSpec } from '../infrastructure/scheduler/types.js'; import { resolveHeaderUserId } from '../utils/request-identity.js'; import { registerCallbackAuthHook } from './callback-auth-prehandler.js'; import { deriveCallbackActor } from './callback-scope-helpers.js'; @@ -141,6 +141,48 @@ function deriveScheduleActor(request: FastifyRequest, _body: { createdBy?: strin }; } +function normalizeIdempotencyKey(value: unknown): string | null | { error: string } { + if (value == null) return null; + if (typeof value !== 'string') return { error: 'idempotencyKey must be a string' }; + const trimmed = value.trim(); + if (!trimmed) return { error: 'idempotencyKey must not be empty' }; + if (trimmed.length > 200) return { error: 'idempotencyKey must be at most 200 characters' }; + return trimmed; +} + +function dynamicTaskResponse(def: Pick) { + return { id: def.id, ...def.display, trigger: def.trigger }; +} + +function compareUtf16(left: string, right: string): number { + if (left === right) return 0; + return left < right ? -1 : 1; +} + +function canonicalizeJson(value: unknown): unknown { + if (Array.isArray(value)) return value.map(canonicalizeJson); + if (value && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value as Record) + // Idempotency fingerprints must be byte-stable across host locales. + .sort(([left], [right]) => compareUtf16(left, right)) + .map(([key, entry]) => [key, canonicalizeJson(entry)]), + ); + } + return value; +} + +function buildScheduleIdempotencyFingerprint(input: { + templateId: string; + trigger: unknown; + params: Record; + display: TaskDisplayMeta; + deliveryThreadId: string | null; + actor: ScheduleActor; +}): string { + return JSON.stringify(canonicalizeJson(input)); +} + /** Test-only export — exposes deriveScheduleActor without spinning up Fastify. */ export function deriveScheduleActorForTest(request: FastifyRequest, body: { createdBy?: string }): ScheduleActor { return deriveScheduleActor(request, body); @@ -298,6 +340,8 @@ export const scheduleRoutes: FastifyPluginAsync = async ( params?: Record; display?: { label: string; category: string; description?: string }; deliveryThreadId?: string; + createdBy?: string; + idempotencyKey?: unknown; }; if (!body.templateId) { @@ -323,7 +367,30 @@ export const scheduleRoutes: FastifyPluginAsync = async ( } else { trigger = body.trigger ?? template.defaultTrigger; } - const params = body.params ?? {}; + const rawParams = body.params ?? {}; + if (typeof rawParams !== 'object' || rawParams === null || Array.isArray(rawParams)) { + reply.status(400); + return { error: 'params must be a plain object' }; + } + const params = { ...rawParams }; + + const normalizedIdempotencyKey = normalizeIdempotencyKey(body.idempotencyKey); + if (normalizedIdempotencyKey && typeof normalizedIdempotencyKey !== 'string') { + reply.status(400); + return { error: normalizedIdempotencyKey.error }; + } + + if (params.targetCatId && typeof params.targetCatId === 'string') { + const resolved = resolveCatTarget(params.targetCatId); + if ('error' in resolved) { + reply.status(400); + return resolved.error; + } + params.targetCatId = resolved.ok; + } + + const actor = deriveScheduleActor(request, body); + params.triggerUserId = actor.triggerUserId; const display = body.display ? { label: body.display.label, @@ -349,6 +416,9 @@ export const scheduleRoutes: FastifyPluginAsync = async ( params, display, deliveryThreadId: resolution.deliveryThreadId, + targetCatId: typeof params.targetCatId === 'string' ? params.targetCatId : null, + actor, + idempotencyKey: normalizedIdempotencyKey, paramSchema: template.paramSchema, }, }; @@ -370,6 +440,7 @@ export const scheduleRoutes: FastifyPluginAsync = async ( createdBy?: string; invocationId?: string; callbackToken?: string; + idempotencyKey?: unknown; }; if (!body.templateId) { @@ -383,24 +454,32 @@ export const scheduleRoutes: FastifyPluginAsync = async ( return { error: `Unknown template: ${body.templateId}` }; } + const requestTrigger = body.trigger ?? template.defaultTrigger; // #415: normalize once trigger (delayMs → fireAt) let trigger: TriggerSpec; - if (body.trigger && (body.trigger as Record).type === 'once') { - const result = normalizeOnceTrigger(body.trigger as Record); + if ((requestTrigger as Record).type === 'once') { + const result = normalizeOnceTrigger(requestTrigger as Record); if ('error' in result) { reply.status(400); return { error: result.error }; } trigger = result; } else { - trigger = body.trigger ?? template.defaultTrigger; + trigger = requestTrigger; } - const params = body.params ?? {}; - if (typeof params !== 'object' || params === null || Array.isArray(params)) { + const rawParams = body.params ?? {}; + if (typeof rawParams !== 'object' || rawParams === null || Array.isArray(rawParams)) { reply.status(400); return { error: 'params must be a plain object' }; } + const params = { ...rawParams }; + + const normalizedIdempotencyKey = normalizeIdempotencyKey(body.idempotencyKey); + if (normalizedIdempotencyKey && typeof normalizedIdempotencyKey !== 'string') { + reply.status(400); + return { error: normalizedIdempotencyKey.error }; + } // F182 AC-C2: B class — validate params.targetCatId is available (contract 400 on disabled) if (params.targetCatId && typeof params.targetCatId === 'string') { @@ -435,6 +514,51 @@ export const scheduleRoutes: FastifyPluginAsync = async ( }; } + const idempotencyFingerprint = + normalizedIdempotencyKey == null + ? null + : buildScheduleIdempotencyFingerprint({ + templateId: body.templateId, + trigger: requestTrigger, + params, + display, + deliveryThreadId: resolution.deliveryThreadId, + actor, + }); + + const ensureRuntimeTaskRegistered = (existing: DynamicTaskDef): void => { + if (!existing.enabled || taskRunner.getRegisteredTasks().includes(existing.id)) return; + const existingTemplate = templateRegistry.get(existing.templateId); + if (!existingTemplate) return; + const spec = existingTemplate.createSpec(existing.id, { + trigger: existing.trigger, + params: existing.params, + deliveryThreadId: existing.deliveryThreadId, + }); + spec.display = existing.display; + taskRunner.registerDynamic(spec, existing.id); + }; + + const replayExistingTask = (existing: DynamicTaskDef, fingerprint: string) => { + if (existing.idempotencyFingerprint !== fingerprint) { + reply.status(409); + return { + error: 'Idempotency key already belongs to a different schedule registration request', + code: 'IDEMPOTENCY_CONFLICT', + task: dynamicTaskResponse(existing), + }; + } + ensureRuntimeTaskRegistered(existing); + return { success: true, idempotent: true, task: dynamicTaskResponse(existing) }; + }; + + if (normalizedIdempotencyKey && idempotencyFingerprint) { + const existing = dynamicTaskStore.getByIdempotencyKey(normalizedIdempotencyKey); + if (existing) { + return replayExistingTask(existing, idempotencyFingerprint); + } + } + const def = { id, templateId: body.templateId, @@ -445,9 +569,21 @@ export const scheduleRoutes: FastifyPluginAsync = async ( enabled: true, createdBy: actor.createdBy, createdAt: new Date().toISOString(), + idempotencyKey: normalizedIdempotencyKey, + idempotencyFingerprint, }; - dynamicTaskStore.insert(def); + try { + dynamicTaskStore.insert(def); + } catch (err) { + if (normalizedIdempotencyKey && idempotencyFingerprint) { + const existing = dynamicTaskStore.getByIdempotencyKey(normalizedIdempotencyKey); + if (existing) { + return replayExistingTask(existing, idempotencyFingerprint); + } + } + throw err; + } // Register in runtime const spec = template.createSpec(id, { trigger, params, deliveryThreadId: def.deliveryThreadId }); @@ -457,7 +593,7 @@ export const scheduleRoutes: FastifyPluginAsync = async ( // #415: lifecycle notification — task registered notifyTaskRegistered(notifyLifecycle, def); - return { success: true, task: { id, ...display, trigger } }; + return { success: true, task: dynamicTaskResponse(def) }; }); // DELETE /api/schedule/tasks/:id (AC-G4: remove dynamic task) diff --git a/packages/api/test/memory/schema-v17.test.js b/packages/api/test/memory/schema-v17.test.js index 0e341fc8ac..db0fa7afb6 100644 --- a/packages/api/test/memory/schema-v17.test.js +++ b/packages/api/test/memory/schema-v17.test.js @@ -4,9 +4,9 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; describe('Schema V17 migration', () => { - it('CURRENT_SCHEMA_VERSION is 26', async () => { + it('CURRENT_SCHEMA_VERSION is 28', async () => { const { CURRENT_SCHEMA_VERSION } = await import('../../dist/domains/memory/schema.js'); - assert.equal(CURRENT_SCHEMA_VERSION, 26); + assert.equal(CURRENT_SCHEMA_VERSION, 28); }); it('V17 adds collection_id and review_status to evidence_docs', async () => { diff --git a/packages/api/test/memory/schema-v19-f200.test.js b/packages/api/test/memory/schema-v19-f200.test.js index 7e3a2abddb..e6f085b1d3 100644 --- a/packages/api/test/memory/schema-v19-f200.test.js +++ b/packages/api/test/memory/schema-v19-f200.test.js @@ -89,9 +89,9 @@ describe('V19 migration — F200 recall_events + edge traversal columns', () => db.close(); }); - it('CURRENT_SCHEMA_VERSION is 26', async () => { + it('CURRENT_SCHEMA_VERSION is 28', async () => { const { CURRENT_SCHEMA_VERSION } = await import('../../dist/domains/memory/schema.js'); - assert.equal(CURRENT_SCHEMA_VERSION, 26); + assert.equal(CURRENT_SCHEMA_VERSION, 28); }); it('can insert and read recall_events', async () => { diff --git a/packages/api/test/memory/schema-v2.test.js b/packages/api/test/memory/schema-v2.test.js index 4defc7cff5..bf0ad671d2 100644 --- a/packages/api/test/memory/schema-v2.test.js +++ b/packages/api/test/memory/schema-v2.test.js @@ -80,6 +80,6 @@ describe('Schema V2 migration', () => { it('CURRENT_SCHEMA_VERSION matches expected value', async () => { const { CURRENT_SCHEMA_VERSION } = await import('../../dist/domains/memory/schema.js'); - assert.equal(CURRENT_SCHEMA_VERSION, 26, `expected 26, got ${CURRENT_SCHEMA_VERSION}`); + assert.equal(CURRENT_SCHEMA_VERSION, 28, `expected 28, got ${CURRENT_SCHEMA_VERSION}`); }); }); diff --git a/packages/api/test/memory/schema-v26-recall-result-count.test.js b/packages/api/test/memory/schema-v26-recall-result-count.test.js index 9afdacf59e..6b8931de5a 100644 --- a/packages/api/test/memory/schema-v26-recall-result-count.test.js +++ b/packages/api/test/memory/schema-v26-recall-result-count.test.js @@ -17,7 +17,7 @@ describe('V26 migration — recall_events result_count', () => { const resultCount = cols.find((col) => col.name === 'result_count'); assert.ok(resultCount, 'result_count column exists'); assert.equal(resultCount.notnull, 0, 'result_count is nullable so old rows can stay unknown'); - assert.equal(schema.CURRENT_SCHEMA_VERSION, 26); + assert.equal(schema.CURRENT_SCHEMA_VERSION, 28); } finally { db.close(); } diff --git a/packages/api/test/memory/world-scope-filter.test.js b/packages/api/test/memory/world-scope-filter.test.js index fcc6f6fb40..0a314c6810 100644 --- a/packages/api/test/memory/world-scope-filter.test.js +++ b/packages/api/test/memory/world-scope-filter.test.js @@ -4,8 +4,8 @@ import Database from 'better-sqlite3'; import { applyMigrations, CURRENT_SCHEMA_VERSION } from '../../dist/domains/memory/schema.js'; describe('Schema V16 (F093 world scope)', () => { - it('CURRENT_SCHEMA_VERSION is 26', () => { - assert.equal(CURRENT_SCHEMA_VERSION, 26); + it('CURRENT_SCHEMA_VERSION is 28', () => { + assert.equal(CURRENT_SCHEMA_VERSION, 28); }); it('migration adds world_id and scene_id columns', () => { diff --git a/packages/api/test/pack-knowledge-scope.test.js b/packages/api/test/pack-knowledge-scope.test.js index 740305e005..f68def4b67 100644 --- a/packages/api/test/pack-knowledge-scope.test.js +++ b/packages/api/test/pack-knowledge-scope.test.js @@ -186,7 +186,7 @@ describe('PackKnowledgeScope', () => { test('schema V6 migration adds pack_id column', async () => { const { CURRENT_SCHEMA_VERSION } = await import('../dist/domains/memory/schema.js'); - assert.equal(CURRENT_SCHEMA_VERSION, 26, 'Current schema version should be 26'); + assert.equal(CURRENT_SCHEMA_VERSION, 28, 'Current schema version should be 28'); // Create a store and check schema via its exposed db const { store } = await createTestStore(); @@ -199,6 +199,6 @@ describe('PackKnowledgeScope', () => { // Verify migration version const version = db.prepare('SELECT MAX(version) as v FROM schema_version').get(); - assert.equal(version.v, 26, 'Schema version should be 26'); + assert.equal(version.v, 28, 'Schema version should be 28'); }); }); diff --git a/packages/api/test/schedule-route.test.js b/packages/api/test/schedule-route.test.js index 8981b271ab..cff5d6b4d4 100644 --- a/packages/api/test/schedule-route.test.js +++ b/packages/api/test/schedule-route.test.js @@ -449,6 +449,36 @@ describe('Schedule Routes', () => { assert.equal(body.draft.deliveryThreadId, 'thread-from-callback'); }); + it('returns final registration semantics in the draft for workflow audit', async () => { + const { invocationId, callbackToken } = await registry.create('user-1', 'opus', 'thread-from-callback'); + const res = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks/preview', + headers: { 'x-invocation-id': invocationId, 'x-callback-token': callbackToken }, + payload: { + templateId: 'reminder', + trigger: { type: 'once', delayMs: 1209600000 }, + params: { message: 'hotfix upgrade review', targetCatId: '@codex', triggerUserId: 'evil-body-user' }, + display: { + label: 'Hotfix upgrade review', + category: 'pr', + description: 'review hotfix after 14 days', + }, + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92', + }, + }); + + assert.equal(res.statusCode, 200); + const body = res.json(); + assert.equal(body.draft.idempotencyKey, 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92'); + assert.deepEqual(body.draft.actor, { createdBy: 'opus', triggerUserId: 'user-1' }); + assert.equal(body.draft.deliveryThreadId, 'thread-from-callback'); + assert.equal(body.draft.targetCatId, 'codex'); + assert.equal(body.draft.params.targetCatId, 'codex'); + assert.equal(body.draft.params.triggerUserId, 'user-1'); + assert.equal(body.draft.display.label, 'Hotfix upgrade review'); + }); + it('returns 409 stale invocation error for stale callback auth invocation', async () => { const stale = await registry.create('user-1', 'opus', 'thread-from-callback'); await registry.create('user-1', 'opus', 'thread-from-callback'); @@ -659,6 +689,186 @@ describe('Schedule Routes', () => { }); }); + describe('POST /api/schedule/tasks — idempotent workflow registration', () => { + let appDyn, store, registry; + + beforeEach(async () => { + const { DynamicTaskStore } = await import('../dist/infrastructure/scheduler/DynamicTaskStore.js'); + const { templateRegistry } = await import('../dist/infrastructure/scheduler/templates/registry.js'); + const { scheduleRoutes: sr } = await import('../dist/routes/schedule.js'); + const { InvocationRegistry } = await import( + '../dist/domains/cats/services/agents/invocation/InvocationRegistry.js' + ); + store = new DynamicTaskStore(db); + registry = new InvocationRegistry(); + appDyn = Fastify({ logger: false }); + await appDyn.register(sr, { taskRunner: runner, dynamicTaskStore: store, templateRegistry, registry }); + await appDyn.ready(); + }); + + afterEach(async () => { + runner.stop(); + await appDyn.close(); + }); + + it('returns the existing task when a workflow registration is replayed with the same idempotencyKey', async () => { + const payload = { + templateId: 'reminder', + trigger: { type: 'once', delayMs: 1209600000 }, + params: { message: 'hotfix upgrade review replay' }, + display: { + label: 'Hotfix 升级 review — PR #92', + category: 'pr', + description: '2 周升级 review：PR #92 是 hotfix，需要三选一处置', + }, + deliveryThreadId: 'thread-review', + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92', + }; + + const first = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload, + }); + assert.equal(first.statusCode, 200, first.body); + const firstBody = first.json(); + assert.equal(firstBody.success, true); + + const replay = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload, + }); + assert.equal(replay.statusCode, 200, replay.body); + const replayBody = replay.json(); + + assert.equal(replayBody.success, true); + assert.equal(replayBody.idempotent, true); + assert.equal(replayBody.task.id, firstBody.task.id, 'replay should return the original task id'); + assert.equal(store.getAll().length, 1, 'replay must not persist a duplicate task'); + assert.equal( + runner.getTaskSummaries().filter((task) => task.source === 'dynamic').length, + 1, + 'replay must not register a duplicate runtime task', + ); + }); + + it('rejects the same idempotencyKey when the requested task semantics differ', async () => { + const payload = { + templateId: 'reminder', + trigger: { type: 'once', delayMs: 1209600000 }, + params: { message: 'first hotfix upgrade review', targetCatId: 'codex' }, + display: { + label: 'Hotfix 升级 review — PR #92', + category: 'pr', + description: '2 周升级 review：PR #92 是 hotfix，需要三选一处置', + }, + deliveryThreadId: 'thread-A', + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92-conflict', + }; + + const first = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload, + }); + assert.equal(first.statusCode, 200, first.body); + + const conflict = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload: { + ...payload, + params: { message: 'changed hotfix upgrade review', targetCatId: 'gemini' }, + deliveryThreadId: 'thread-B', + }, + }); + + assert.equal(conflict.statusCode, 409, conflict.body); + assert.equal(conflict.json().code, 'IDEMPOTENCY_CONFLICT'); + assert.equal(store.getAll().length, 1, 'conflict must not persist a replacement task'); + const stored = store.getAll()[0]; + assert.equal(stored.deliveryThreadId, 'thread-A'); + assert.equal(stored.params.message, 'first hotfix upgrade review'); + assert.equal(stored.params.targetCatId, 'codex'); + }); + + it('rejects the same idempotencyKey when the verified actor differs', async () => { + const firstAuth = await registry.create('user-1', 'opus', 'thread-actor-replay'); + const payload = { + templateId: 'reminder', + trigger: { type: 'once', delayMs: 1209600000 }, + params: { message: 'hotfix upgrade review actor replay' }, + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92-actor', + }; + + const first = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + headers: { 'x-invocation-id': firstAuth.invocationId, 'x-callback-token': firstAuth.callbackToken }, + payload, + }); + assert.equal(first.statusCode, 200, first.body); + + const secondAuth = await registry.create('user-2', 'opus', 'thread-actor-replay'); + const conflict = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + headers: { 'x-invocation-id': secondAuth.invocationId, 'x-callback-token': secondAuth.callbackToken }, + payload, + }); + + assert.equal(conflict.statusCode, 409, conflict.body); + assert.equal(conflict.json().code, 'IDEMPOTENCY_CONFLICT'); + assert.equal(store.getAll().length, 1, 'conflict must not persist a second task'); + assert.equal(store.getAll()[0].params.triggerUserId, 'user-1'); + }); + + it('replays the same non-ASCII request when host locale ordering changes', async () => { + const originalLocaleCompare = String.prototype.localeCompare; + const payload = { + templateId: 'reminder', + trigger: { type: 'interval', ms: 60000 }, + params: { z: 'ascii', ä: 'non-ascii', message: 'locale-stable replay' }, + display: { + label: 'Locale stable replay', + category: 'system', + description: 'same request must not depend on runtime locale collation', + }, + deliveryThreadId: 'thread-locale', + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92-locale', + }; + + const first = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload, + }); + assert.equal(first.statusCode, 200, first.body); + const firstBody = first.json(); + + String.prototype.localeCompare = function reversedLocaleCompare(other) { + if (String(this) === String(other)) return 0; + return originalLocaleCompare.call(this, other) < 0 ? 1 : -1; + }; + + try { + const replay = await appDyn.inject({ + method: 'POST', + url: '/api/schedule/tasks', + payload, + }); + + assert.equal(replay.statusCode, 200, replay.body); + assert.equal(replay.json().idempotent, true); + assert.equal(replay.json().task.id, firstBody.task.id); + assert.equal(store.getAll().length, 1, 'locale collation drift must not persist or conflict'); + } finally { + String.prototype.localeCompare = originalLocaleCompare; + } + }); + }); + describe('POST /api/schedule/tasks — callback auth infers deliveryThreadId', () => { let appDyn, store, registry; diff --git a/packages/api/test/scheduler/dynamic-task-store.test.js b/packages/api/test/scheduler/dynamic-task-store.test.js index 2230bbc7fe..727f7cd49d 100644 --- a/packages/api/test/scheduler/dynamic-task-store.test.js +++ b/packages/api/test/scheduler/dynamic-task-store.test.js @@ -37,6 +37,8 @@ test('dynamic_task_defs has correct columns', () => { assert.ok(names.includes('enabled')); assert.ok(names.includes('created_by')); assert.ok(names.includes('created_at')); + assert.ok(names.includes('idempotency_key')); + assert.ok(names.includes('idempotency_fingerprint')); db.close(); }); @@ -111,6 +113,32 @@ describe('DynamicTaskStore', () => { assert.throws(() => store.insert(SAMPLE_DEF), /UNIQUE|constraint/i); }); + test('idempotency key round-trips and can be looked up', () => { + store.insert({ + ...SAMPLE_DEF, + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:repo#92', + idempotencyFingerprint: '{"same":"request"}', + }); + const def = store.getByIdempotencyKey('workflow:merge-gate:hotfix-upgrade-review:repo#92'); + assert.ok(def, 'def should be loaded by idempotency key'); + assert.equal(def.id, 'dyn-001'); + assert.equal(def.idempotencyKey, 'workflow:merge-gate:hotfix-upgrade-review:repo#92'); + assert.equal(def.idempotencyFingerprint, '{"same":"request"}'); + }); + + test('insert rejects duplicate idempotency key', () => { + store.insert({ ...SAMPLE_DEF, idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:repo#92' }); + assert.throws( + () => + store.insert({ + ...SAMPLE_DEF, + id: 'dyn-002', + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:repo#92', + }), + /UNIQUE|constraint/i, + ); + }); + test('#415: once trigger round-trips correctly', () => { const fireAt = Date.now() + 120_000; const onceDef = { diff --git a/packages/mcp-server/src/tools/schedule-tools.ts b/packages/mcp-server/src/tools/schedule-tools.ts index 663e7816e5..feb52b8184 100644 --- a/packages/mcp-server/src/tools/schedule-tools.ts +++ b/packages/mcp-server/src/tools/schedule-tools.ts @@ -68,8 +68,30 @@ export const registerScheduledTaskInputSchema = { label: z.string().optional().describe('Human-readable task label (defaults to template label)'), category: z.string().optional().describe('Display category: pr | repo | thread | system | external'), description: z.string().optional().describe('Short description of this task instance'), + idempotencyKey: z + .string() + .min(1) + .max(200) + .optional() + .describe('Stable replay key for trusted workflow automation; retries with the same key return the existing task'), }; +function parseScheduleParams( + raw: string | undefined, +): { ok: true; params: Record } | { ok: false; error: string } { + if (!raw) return { ok: true, params: {} }; + + try { + const parsed: unknown = JSON.parse(raw); + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + return { ok: false, error: 'Invalid params JSON — must be a JSON object (not null, array, or primitive)' }; + } + return { ok: true, params: parsed as Record }; + } catch { + return { ok: false, error: 'Invalid params JSON — must be a valid JSON object' }; + } +} + export async function handleRegisterScheduledTask(input: { templateId: string; trigger: string; @@ -78,6 +100,7 @@ export async function handleRegisterScheduledTask(input: { label?: string; category?: string; description?: string; + idempotencyKey?: string; }): Promise { let trigger: unknown; try { @@ -86,18 +109,9 @@ export async function handleRegisterScheduledTask(input: { return errorResult('Invalid trigger JSON — must be a valid JSON object'); } - let params: Record = {}; - if (input.params) { - try { - const parsed: unknown = JSON.parse(input.params); - if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { - return errorResult('Invalid params JSON — must be a JSON object (not null, array, or primitive)'); - } - params = parsed as Record; - } catch { - return errorResult('Invalid params JSON — must be a valid JSON object'); - } - } + const parsedParams = parseScheduleParams(input.params); + if (!parsedParams.ok) return errorResult(parsedParams.error); + const params = parsedParams.params; // Auto-inject current cat's ID so reminder tasks wake the registering cat, not default opus const currentCatId = process.env['CAT_CAFE_CAT_ID']; @@ -112,6 +126,7 @@ export async function handleRegisterScheduledTask(input: { }; if (input.deliveryThreadId) body.deliveryThreadId = input.deliveryThreadId; + if (input.idempotencyKey) body.idempotencyKey = input.idempotencyKey; if (currentCatId) body.createdBy = currentCatId; if (input.label || input.category || input.description) { @@ -137,6 +152,15 @@ export const previewScheduledTaskInputSchema = { .describe( 'Thread ID to deliver results to. If omitted on callback-origin requests, the current invocation thread is used', ), + label: z.string().optional().describe('Human-readable task label (defaults to template label)'), + category: z.string().optional().describe('Display category: pr | repo | thread | system | external'), + description: z.string().optional().describe('Short description of this task instance'), + idempotencyKey: z + .string() + .min(1) + .max(200) + .optional() + .describe('Stable replay key to include in the draft for trusted workflow audit parity'), }; export async function handlePreviewScheduledTask(input: { @@ -144,6 +168,10 @@ export async function handlePreviewScheduledTask(input: { trigger: string; params?: string; deliveryThreadId?: string; + label?: string; + category?: string; + description?: string; + idempotencyKey?: string; }): Promise { let trigger: unknown; try { @@ -152,13 +180,15 @@ export async function handlePreviewScheduledTask(input: { return errorResult('Invalid trigger JSON'); } - let params: Record = {}; - if (input.params) { - try { - params = JSON.parse(input.params); - } catch { - return errorResult('Invalid params JSON'); - } + const parsedParams = parseScheduleParams(input.params); + if (!parsedParams.ok) return errorResult(parsedParams.error); + const params = parsedParams.params; + + // Preview must match registration semantics so the draft audits the same + // target that will be persisted by register_scheduled_task. + const currentCatId = process.env['CAT_CAFE_CAT_ID']; + if (!params.targetCatId && currentCatId) { + params.targetCatId = currentCatId; } const body: Record = { @@ -167,6 +197,15 @@ export async function handlePreviewScheduledTask(input: { params, }; if (input.deliveryThreadId) body.deliveryThreadId = input.deliveryThreadId; + if (input.idempotencyKey) body.idempotencyKey = input.idempotencyKey; + + if (input.label || input.category || input.description) { + body.display = { + label: input.label ?? input.templateId, + category: input.category ?? 'system', + ...(input.description ? { description: input.description } : {}), + }; + } return callbackPost('/api/schedule/tasks/preview', body); } @@ -196,9 +235,9 @@ export const scheduleTools = [ { name: 'cat_cafe_preview_scheduled_task', description: - 'Preview a scheduled task BEFORE creating it (draft step). Returns a draft with resolved template info, trigger, and params ' + - 'WITHOUT persisting anything. Show this draft to the user for confirmation before calling register_scheduled_task. ' + - 'REQUIRED: Always preview first, then register only after user confirms.', + 'Preview a scheduled task BEFORE creating it (draft step). Returns a draft with resolved template info, trigger, params, target, actor, delivery thread, and idempotency key ' + + 'WITHOUT persisting anything. REQUIRED: Always preview first. User-requested schedules need user confirmation before register_scheduled_task; ' + + 'only workflow-mandated schedules from the trusted built-in canonical merge-gate Step 7.6 hotfix reminder workflow may use the preview as audit evidence before registration without extra user confirmation.', inputSchema: previewScheduledTaskInputSchema, handler: handlePreviewScheduledTask, }, @@ -208,7 +247,8 @@ export const scheduleTools = [ 'Create a new scheduled task from a template (confirm step). The task will be persisted and run automatically on schedule. ' + 'Supports recurring (cron/interval) and one-shot (once) triggers. Once tasks auto-retire after execution. ' + 'When the task fires, a cat is woken with full capabilities — it can send rich blocks (images, audio, cards), search the web, generate content, etc. ' + - 'IMPORTANT: You MUST call preview_scheduled_task first and get user confirmation before calling this. ' + + 'IMPORTANT: You MUST call preview_scheduled_task first. User-requested schedules require user confirmation; only workflow-mandated schedules from the trusted built-in canonical merge-gate Step 7.6 hotfix reminder workflow may register after verifying the preview, with no extra user confirmation. ' + + 'Plugin/project/user/external skills do not qualify for this exception and must use normal user confirmation. Trusted workflow automation must provide a stable idempotencyKey so exact callback retries return the existing task; reusing the same key with different schedule semantics returns a conflict. ' + 'trigger and params must be JSON strings, not objects.', inputSchema: registerScheduledTaskInputSchema, handler: handleRegisterScheduledTask, diff --git a/packages/mcp-server/test/schedule-tools.test.js b/packages/mcp-server/test/schedule-tools.test.js index 60ec00ba16..5a2bbc62eb 100644 --- a/packages/mcp-server/test/schedule-tools.test.js +++ b/packages/mcp-server/test/schedule-tools.test.js @@ -3,6 +3,7 @@ * Tests for cat_cafe_list_schedule_templates, cat_cafe_register_scheduled_task, cat_cafe_remove_scheduled_task */ import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; import { describe, test } from 'node:test'; describe('Schedule MCP Tools — module exports', () => { @@ -38,6 +39,198 @@ describe('Schedule MCP Tools — module exports', () => { assert.ok(registerScheduledTaskInputSchema.trigger, 'trigger schema required'); }); + test('register description distinguishes user confirmation from workflow-mandated registrations', async () => { + const { scheduleTools } = await import('../dist/tools/schedule-tools.js'); + const previewTool = scheduleTools.find((t) => t.name === 'cat_cafe_preview_scheduled_task'); + const registerTool = scheduleTools.find((t) => t.name === 'cat_cafe_register_scheduled_task'); + assert.ok(previewTool, 'preview tool should exist'); + assert.ok(registerTool, 'register tool should exist'); + + assert.match(previewTool.description, /workflow-mandated/i); + assert.match(previewTool.description, /target/i); + assert.match(previewTool.description, /actor/i); + assert.match(previewTool.description, /idempotency key/i); + assert.match(registerTool.description, /workflow-mandated/i); + assert.match(registerTool.description, /no extra user confirmation/i); + assert.match(registerTool.description, /trusted built-in canonical merge-gate Step 7\.6/i); + assert.match(registerTool.description, /plugin\/project\/user\/external skills do not qualify/i); + assert.match(registerTool.description, /different schedule semantics returns a conflict/i); + assert.doesNotMatch(registerTool.description, /explicit SOP\/skill step/i); + }); + + test('schedule skills document the workflow-mandated registration exception', () => { + const scheduleSkill = readFileSync( + new URL('../../../cat-cafe-skills/schedule-tasks/SKILL.md', import.meta.url), + 'utf8', + ); + const mergeGateSkill = readFileSync( + new URL('../../../cat-cafe-skills/merge-gate/SKILL.md', import.meta.url), + 'utf8', + ); + + assert.match(scheduleSkill, /workflow-mandated/i); + assert.match(scheduleSkill, /no extra user confirmation/i); + assert.match(scheduleSkill, /trusted built-in canonical `merge-gate` Step 7\.6/i); + assert.match(scheduleSkill, /plugin\/project\/user\/external skills do not qualify/i); + assert.doesNotMatch(scheduleSkill, /明确 SOP\/skill 步骤要求注册定时任务/); + assert.match(mergeGateSkill, /workflow-mandated/i); + assert.match(mergeGateSkill, /cat_cafe_preview_scheduled_task/); + assert.match(mergeGateSkill, /idempotencyKey/); + assert.match(mergeGateSkill, /targetCatId/); + assert.match(mergeGateSkill, /actor\.createdBy/); + assert.match(mergeGateSkill, /IDEMPOTENCY_CONFLICT/); + assert.match(mergeGateSkill, /no extra user confirmation/i); + }); + + test('cat_cafe_register_scheduled_task accepts an idempotency key for workflow replay safety', async () => { + const { registerScheduledTaskInputSchema } = await import('../dist/tools/schedule-tools.js'); + assert.ok(registerScheduledTaskInputSchema.idempotencyKey, 'idempotencyKey schema required'); + }); + + test('cat_cafe_preview_scheduled_task accepts an idempotency key for workflow audit parity', async () => { + const { previewScheduledTaskInputSchema } = await import('../dist/tools/schedule-tools.js'); + assert.ok(previewScheduledTaskInputSchema.idempotencyKey, 'idempotencyKey schema required'); + }); + + test('cat_cafe_register_scheduled_task forwards idempotencyKey to the API', async () => { + const originalFetch = globalThis.fetch; + const originalApiUrl = process.env.CAT_CAFE_API_URL; + const originalInvocationId = process.env.CAT_CAFE_INVOCATION_ID; + const originalCallbackToken = process.env.CAT_CAFE_CALLBACK_TOKEN; + let postedBody; + + process.env.CAT_CAFE_API_URL = 'http://127.0.0.1:3004'; + process.env.CAT_CAFE_INVOCATION_ID = 'inv-schedule-test'; + process.env.CAT_CAFE_CALLBACK_TOKEN = 'tok-schedule-test'; + + globalThis.fetch = async (_url, options) => { + postedBody = JSON.parse(options.body); + return new Response(JSON.stringify({ success: true, task: { id: 'dyn-1' } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + try { + const { handleRegisterScheduledTask } = await import('../dist/tools/schedule-tools.js'); + const result = await handleRegisterScheduledTask({ + templateId: 'reminder', + trigger: JSON.stringify({ type: 'once', delayMs: 1209600000 }), + params: JSON.stringify({ message: 'hotfix upgrade review' }), + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92', + }); + + assert.equal(result.isError, undefined); + assert.equal(postedBody.idempotencyKey, 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92'); + } finally { + globalThis.fetch = originalFetch; + if (originalApiUrl === undefined) delete process.env.CAT_CAFE_API_URL; + else process.env.CAT_CAFE_API_URL = originalApiUrl; + if (originalInvocationId === undefined) delete process.env.CAT_CAFE_INVOCATION_ID; + else process.env.CAT_CAFE_INVOCATION_ID = originalInvocationId; + if (originalCallbackToken === undefined) delete process.env.CAT_CAFE_CALLBACK_TOKEN; + else process.env.CAT_CAFE_CALLBACK_TOKEN = originalCallbackToken; + } + }); + + test('cat_cafe_preview_scheduled_task forwards the same workflow fields as registration', async () => { + const originalFetch = globalThis.fetch; + const originalApiUrl = process.env.CAT_CAFE_API_URL; + const originalInvocationId = process.env.CAT_CAFE_INVOCATION_ID; + const originalCallbackToken = process.env.CAT_CAFE_CALLBACK_TOKEN; + const originalCatId = process.env.CAT_CAFE_CAT_ID; + let postedBody; + + process.env.CAT_CAFE_API_URL = 'http://127.0.0.1:3004'; + process.env.CAT_CAFE_INVOCATION_ID = 'inv-schedule-preview-test'; + process.env.CAT_CAFE_CALLBACK_TOKEN = 'tok-schedule-preview-test'; + process.env.CAT_CAFE_CAT_ID = 'codex'; + + globalThis.fetch = async (_url, options) => { + postedBody = JSON.parse(options.body); + return new Response(JSON.stringify({ draft: postedBody }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + try { + const { handlePreviewScheduledTask } = await import('../dist/tools/schedule-tools.js'); + const result = await handlePreviewScheduledTask({ + templateId: 'reminder', + trigger: JSON.stringify({ type: 'once', delayMs: 1209600000 }), + params: JSON.stringify({ message: 'hotfix upgrade review' }), + label: 'Hotfix upgrade review', + category: 'pr', + description: 'Review hotfix after 14 days', + idempotencyKey: 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92', + }); + + assert.equal(result.isError, undefined); + assert.equal(postedBody.params.targetCatId, 'codex'); + assert.equal(postedBody.idempotencyKey, 'workflow:merge-gate:hotfix-upgrade-review:clowder-labs/clowder-ai#92'); + assert.deepEqual(postedBody.display, { + label: 'Hotfix upgrade review', + category: 'pr', + description: 'Review hotfix after 14 days', + }); + } finally { + globalThis.fetch = originalFetch; + if (originalApiUrl === undefined) delete process.env.CAT_CAFE_API_URL; + else process.env.CAT_CAFE_API_URL = originalApiUrl; + if (originalInvocationId === undefined) delete process.env.CAT_CAFE_INVOCATION_ID; + else process.env.CAT_CAFE_INVOCATION_ID = originalInvocationId; + if (originalCallbackToken === undefined) delete process.env.CAT_CAFE_CALLBACK_TOKEN; + else process.env.CAT_CAFE_CALLBACK_TOKEN = originalCallbackToken; + if (originalCatId === undefined) delete process.env.CAT_CAFE_CAT_ID; + else process.env.CAT_CAFE_CAT_ID = originalCatId; + } + }); + + test('cat_cafe_preview_scheduled_task rejects non-object params without posting', async () => { + const originalFetch = globalThis.fetch; + const originalApiUrl = process.env.CAT_CAFE_API_URL; + const originalInvocationId = process.env.CAT_CAFE_INVOCATION_ID; + const originalCallbackToken = process.env.CAT_CAFE_CALLBACK_TOKEN; + const originalRetryDelays = process.env.CAT_CAFE_CALLBACK_RETRY_DELAYS_MS; + let fetchCalls = 0; + + process.env.CAT_CAFE_API_URL = 'http://127.0.0.1:3004'; + process.env.CAT_CAFE_INVOCATION_ID = 'inv-schedule-preview-invalid-params-test'; + process.env.CAT_CAFE_CALLBACK_TOKEN = 'tok-schedule-preview-invalid-params-test'; + process.env.CAT_CAFE_CALLBACK_RETRY_DELAYS_MS = '0'; + + globalThis.fetch = async () => { + fetchCalls += 1; + throw new Error('preview should reject invalid params before callbackPost'); + }; + + try { + const { handlePreviewScheduledTask } = await import('../dist/tools/schedule-tools.js'); + for (const params of ['null', '[]', '42']) { + const result = await handlePreviewScheduledTask({ + templateId: 'reminder', + trigger: JSON.stringify({ type: 'once', delayMs: 1209600000 }), + params, + }); + + assert.equal(result.isError, true); + assert.match(result.content[0].text, /Invalid params JSON.*JSON object/); + } + assert.equal(fetchCalls, 0, 'invalid params must fail locally without posting to the API'); + } finally { + globalThis.fetch = originalFetch; + if (originalApiUrl === undefined) delete process.env.CAT_CAFE_API_URL; + else process.env.CAT_CAFE_API_URL = originalApiUrl; + if (originalInvocationId === undefined) delete process.env.CAT_CAFE_INVOCATION_ID; + else process.env.CAT_CAFE_INVOCATION_ID = originalInvocationId; + if (originalCallbackToken === undefined) delete process.env.CAT_CAFE_CALLBACK_TOKEN; + else process.env.CAT_CAFE_CALLBACK_TOKEN = originalCallbackToken; + if (originalRetryDelays === undefined) delete process.env.CAT_CAFE_CALLBACK_RETRY_DELAYS_MS; + else process.env.CAT_CAFE_CALLBACK_RETRY_DELAYS_MS = originalRetryDelays; + } + }); + test('cat_cafe_remove_scheduled_task has taskId required', async () => { const { scheduleTools, removeScheduledTaskInputSchema } = await import('../dist/tools/schedule-tools.js'); const tool = scheduleTools.find((t) => t.name === 'cat_cafe_remove_scheduled_task'); diff --git a/review-notes/2026-07-23-hotfix-reminder-sop-review-request.md b/review-notes/2026-07-23-hotfix-reminder-sop-review-request.md new file mode 100644 index 0000000000..937bc8fb22 --- /dev/null +++ b/review-notes/2026-07-23-hotfix-reminder-sop-review-request.md @@ -0,0 +1,125 @@ +# Review Request: hotfix reminder SOP registration + +Review-Target-ID: fix-hotfix-reminder-sop +Branch: fix/hotfix-reminder-sop +PR: https://github.com/clowder-labs/clowder-ai/pull/92 +Head: use PR #92 current head; the routing message supplies the exact SHA. + +## What + +- Updated schedule MCP descriptions to distinguish user-requested schedules from the one trusted workflow-mandated exception. +- Updated `schedule-tasks` so ordinary user schedules still require preview and confirmation, while only trusted built-in canonical `merge-gate` Step 7.6 may register after preview verification. +- Added schedule registration idempotency for workflow replay safety: stable `idempotencyKey` values dedupe exact callback retries to the existing dynamic task, while same-key semantic mismatches return `409 IDEMPOTENCY_CONFLICT`. +- Bound each idempotency key to a canonical request fingerprint covering template, trigger input, params/target, display, delivery thread, and actor. +- Made preview drafts match final registration semantics: target canonicalization, actor, delivery thread, display, and idempotency key are visible in the draft before persistence. +- Added a `tips_exempt` frontmatter note because this is an internal SOP/MCP workflow clarification, not a new end-user capability. +- Updated `merge-gate` Step 7.6 so hotfix 14-day upgrade reminders must preview, verify draft fields, then register with no extra user confirmation and a repo+PR-scoped stable `idempotencyKey`; conflicts now fail closed. +- Added MCP/API regression tests for tool descriptions, skill docs, preview parity, idempotency-key passthrough, persistent dedupe, same-key conflicts, and replay behavior. + +## Why + +Daily patrol found a process contradiction: hotfix merge-gate requires registering a 14-day upgrade review reminder, but schedule guidance required user confirmation for every registration. That left several hotfix tails as previews instead of persisted reminders. + +## Original Requirements + +> 每日北京时间 00:00 系统先锋巡检。 +> 巡检 clowder-code / Cat Cafe 系统中不完善、待改进、可提升竞争力的地方。 +> 每轮必须先查真相源和证据，再给风险/价值判断与下一步动作。 +> 发现可执行事项后主导闭环。 + +- 来源：Cat Cafe patrol thread `thread_mqcj45byxoka2z7u`, task `0001784736110466-000578-8fa70fcb` +- 请 reviewer 对照上面的摘录判断这次修复是否闭合了 hotfix reminder 漏注册的流程缺口。 + +## Tradeoff + +The exception is intentionally narrow. I did not remove confirmation from normal schedule registration; user-requested reminders still require preview and explicit confirmation. Plugin/project/user/external skills do not qualify; only the built-in canonical `merge-gate` Step 7.6 hotfix reminder workflow can register after preview verification. + +The API change is additive: callers without `idempotencyKey` keep the existing create-new-task behavior. Callers with a stable key get replay-safe dedupe only for the same canonical request; conflicting reuse returns 409 and does not mutate the existing task. + +## Architecture Ownership + +Architecture cell: workflow / MCP collab schedule surface + scheduler persistence +Map delta: none +Why: This extends the existing schedule route and `DynamicTaskStore` with idempotency key/fingerprint columns and an index; it does not add a new Store, Queue, Router, Adapter, Dispatcher, Binding, or runtime ownership boundary. + +Reviewer checks: + +- Verify `Map delta: none` matches the diff. +- Verify the exception cannot be read as bypassing user confirmation for ordinary user-requested schedules. +- Verify plugin/project/user/external skills are excluded from the no-extra-confirmation exception. +- Verify schedule registration retry with the same `idempotencyKey` cannot create duplicate reminders. +- Verify same-key semantic mismatch returns `409 IDEMPOTENCY_CONFLICT` without modifying the existing task. +- Verify preview draft fields match final registration semantics, including target, actor, delivery thread, display, and idempotency key. +- Verify `merge-gate` Step 7.6 is actionable and fail-closed. + +## Open Questions + +### 技术 OQ + +- Is the trusted built-in canonical `merge-gate` Step 7.6 boundary tight enough to prevent workflow-mandated from becoming a blanket bypass? +- For cross-cat takeover of the same merge-gate reminder, this implementation chooses fail-closed `409` when actor/provenance differs. Please verify that is the right contract. + +### 价值 OQ + +无。 + +## Next Action + +Please review PR #92 against its current head. Focus on the trusted workflow boundary, merge-gate hotfix reminder flow, idempotent registration replay/conflict behavior, preview/register parity, and whether the regression tests protect the right surfaces. + +## Review Sandbox + +- Path: `/tmp/cat-cafe-review/fix-hotfix-reminder-sop/{reviewer-handle}` +- Start Command: `pnpm review:start` is not required for this PR; no runtime/frontend path is involved. +- Ports: none. + +### Sandbox Bootstrap + +```bash +unset NODE_ENV +PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm install --frozen-lockfile +``` + +## 自检证据 + +### Spec 合规 + +- 巡检范围：hotfix merge-gate reminder tail, schedule MCP descriptions, `schedule-tasks`, and `merge-gate` Step 7.6. +- 根因：schedule confirmation wording and merge-gate mandated registration were inconsistent. +- 修复：preview remains mandatory; confirmation remains mandatory for user-requested schedules; only built-in canonical `merge-gate` Step 7.6 hotfix reminder can register after verified preview with no extra confirmation; exact register retries are deduped by stable `idempotencyKey`, while same-key semantic mismatches return `409`. +- Dogfood scope: exempt. This is an internal SOP/tool-description fix with no user-facing runtime path. +- Artifact hygiene: root media/design artifact checks returned no matches. + +### 测试结果 + +```bash +git diff --check +# passed + +PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm check +# passed; existing advisory warnings only + +PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm --filter @cat-cafe/api build +# passed + +PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm --filter @cat-cafe/mcp-server build +# passed + +PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm --filter @cat-cafe/mcp-server test -- --test-name-pattern "workflow-mandated|idempotency key|workflow fields" +# passed: 387 tests, 0 failed + +node --test packages/api/test/scheduler/dynamic-task-store.test.js --test-name-pattern "idempotency|columns" +# passed: 13 tests, 0 failed + +node --test packages/api/test/schedule-route.test.js --test-name-pattern "idempotencyKey|workflow audit" +# passed: 40 tests, 0 failed + +node --test packages/api/test/memory/schema-v17.test.js packages/api/test/memory/schema-v19-f200.test.js packages/api/test/memory/schema-v2.test.js packages/api/test/memory/schema-v26-recall-result-count.test.js packages/api/test/memory/world-scope-filter.test.js packages/api/test/pack-knowledge-scope.test.js --test-name-pattern "CURRENT_SCHEMA_VERSION|schema V6 migration|V26 migration" +# passed: 30 tests, 0 failed +``` + +### Local Caveat + +Quality-gate helper caveats in this local checkout: `node scripts/check-fallback-layers.mjs` is unavailable because the script is not present, and `pnpm check:architecture-ownership` is not registered. I ran the equivalent manual diff scan for architecture-surface changes and the root artifact hygiene checks; no blocker found. + +Node 24 local API `node --test` is blocked by the local `better-sqlite3` native module being compiled for Node 22 (`NODE_MODULE_VERSION 127` vs Node 24 `137`). Focused API tests are green under default Node 22; TypeScript builds and repo `pnpm check` are green under Node 24. GitHub CI should be treated as the full-suite truth source for this PR.