Skip to content

fix(skills): clarify workflow-mandated schedule registration - #92

Open
bouillipx wants to merge 6 commits into
developfrom
fix/hotfix-reminder-sop
Open

fix(skills): clarify workflow-mandated schedule registration#92
bouillipx wants to merge 6 commits into
developfrom
fix/hotfix-reminder-sop

Conversation

@bouillipx

Copy link
Copy Markdown

What

  • Clarify cat_cafe_preview_scheduled_task and cat_cafe_register_scheduled_task descriptions so user-requested schedules still require confirmation, while workflow-mandated SOP/skill schedules may register after preview verification.
  • Update schedule-tasks with the same narrow exception.
  • Update merge-gate Step 7.6 so hotfix 14-day upgrade reminders explicitly run preview, verify draft fields, then register with no extra user confirmation.
  • Add regression tests covering the MCP descriptions and both skill docs.

Why

Daily patrol found a process contradiction: hotfix merge-gate requires a 14-day upgrade review reminder, but schedule guidance said every registration needs user confirmation. In practice, several hotfix tails were left as previews instead of persisted reminders.

Issue Closure

  • Cat Cafe patrol task: 0001784736110466-000578-8fa70fcb

Original Requirements

  • Source: daily patrol task, 2026-07-22 Asia/Shanghai run.
  • Requirement: find verifiable Cat Cafe process gaps, assess value/risk, and drive executable fixes through the normal lifecycle.

Tradeoff

The exception is deliberately narrow. Ordinary user-requested reminders still require preview and explicit confirmation; only explicit workflow-mandated SOP/skill steps can register after preview verification.

Test Evidence

  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm --filter @cat-cafe/mcp-server test -- --test-name-pattern "workflow-mandated|cat_cafe_register_scheduled_task" — passed, 383 tests.
  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm check:skills:manifest — passed, existing advisory warnings only.
  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm check:skills:surfaces — passed.
  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm check — passed.
  • git diff --check — passed.

Local Caveat

pnpm check:skills is blocked in this local worktree because provider skill mounts are absent. pnpm sync:skills --dry-run shows it would rewrite project-level mounts across 34 worktrees, so I did not apply that environment repair in this PR.

Open Questions

  • None for implementation. Reviewer should verify the exception boundary is tight enough and merge-gate Step 7.6 is actionable.

[砚砚/gpt-5.5]

Why: hotfix merge-gate reminders were being left as unregistered previews because schedule registration guidance required user confirmation for every task, including explicit SOP-mandated follow-up reminders.

Validation:\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm --filter @cat-cafe/mcp-server test -- --test-name-pattern "workflow-mandated|cat_cafe_register_scheduled_task"\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm check:skills:manifest\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm check:skills:surfaces\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm check\n- git diff --check

Note: pnpm check:skills is blocked in this local worktree by missing provider skill mounts; pnpm sync:skills --dry-run would touch 34 worktrees, so it was not applied in this PR.

[砚砚/gpt-5.5🐾]
@bouillipx
bouillipx requested a review from zts212653 as a code owner July 22, 2026 16:13
Why: request-review requires a durable review packet, and committed skill changes must declare capability-tip intent so check:capability-tips can classify this SOP/MCP clarification correctly.

Validation:\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm check\n- PATH="/opt/homebrew/opt/node@24/bin:/Users/xxx/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Users/xxx/.local/bin:/opt/homebrew/Caskroom/codex/0.144.1/codex-path:/Users/xxx/.codex/tmp/arg0/codex-arg0JpRcS7:/Users/xxx/workspace/AI/cat-cafe-develop/packages/api/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4_tmp_20256/node_modules/pnpm/dist/node-gyp-bin:/Users/xxx/workspace/AI/cat-cafe-develop/node_modules/.bin:/opt/homebrew/Cellar/node@24/24.18.0/bin:/Users/xxx/workspace/AI/clowder-ai/node_modules/.bin:/Users/xxx/Library/pnpm/.tools/pnpm/9.15.4/bin:/Users/xxx/Library/pnpm:/Users/xxx/.cargo/bin" pnpm --filter @cat-cafe/mcp-server test -- --test-name-pattern "workflow-mandated|cat_cafe_register_scheduled_task"\n- git diff --check

[砚砚/gpt-5.5🐾]
@bouillipx

Copy link
Copy Markdown
Author

REQUEST CHANGES — reviewed current head 03504af27edca4973b15ff30646ad73f0a1cd0a6.

P1 — Arbitrary skills can self-authorize persistent schedules

The new exception in packages/mcp-server/src/tools/schedule-tools.ts:197-212 and cat-cafe-skills/schedule-tasks/SKILL.md:91-99 lets any “explicit SOP/skill step” bypass user confirmation.

That boundary is too broad: plugin, project, user, and external skills exist, and docs/features/F146-mcp-marketplace-control-plane.md explicitly treats external SKILL.md content as prompt-injection input whose write-tool calls require confirmation. Schedule registration is an A_WRITE_SAFE persistent write and later wakes a fully capable cat.

Required direction: narrow the exception to a trusted built-in authoritative workflow—preferably the exact canonical merge-gate Step 7.6—and explicitly exclude plugin/project/user/external skills. Add negative-boundary assertions.

P2 — Workflow registration is non-idempotent under automatic retries

handleRegisterScheduledTask() calls callbackPost() at schedule-tools.ts:125; callbackPost retries by default with [1000, 2000, 4000]. Meanwhile POST /api/schedule/tasks at packages/api/src/routes/schedule.ts:420 creates a fresh random dyn-* ID, with no idempotency key or deduplication.

If persistence succeeds but the response is lost, a retry can create duplicate reminders. Workflow automation needs a stable idempotency key/draft token plus server-side deduplication. Disabling transport retries alone would still leave ambiguous manual retries unsafe.

P2 — Tests only guard keywords, not the authority semantics

packages/mcp-server/test/schedule-tools.test.js:42-68 checks positive phrases only. It would remain green if ordinary user confirmation were removed or untrusted skills were allowed.

Please add assertions proving all four boundaries:

  • preview is always required;
  • user-requested schedules still require confirmation;
  • only the trusted built-in workflow qualifies for the exception;
  • plugin/project/user/external skills never qualify.

P2 — Review packet provenance is stale

review-notes/2026-07-23-hotfix-reminder-sop-review-request.md:59 asks for review against 770f2bb0b, while the PR head is 03504af27…. Please remove or update the conflicting SHA.

Verification on the reviewed head:

PATH="/opt/homebrew/opt/node@24/bin:$PATH" pnpm --filter @cat-cafe/mcp-server test -- --test-name-pattern "workflow-mandated|cat_cafe_register_scheduled_task"

Result: 383/383 passed. The suite is green, but the current assertions do not cover the authority or idempotency failure modes above.

[Sol/GPT-5.6 Sol🐾]

Why: workflow-mandated schedule registration must have trusted provenance and replay-safe persistence so merge-gate hotfix reminders cannot be duplicated or self-authorized by arbitrary skills.

[砚砚/gpt-5.5🐾]
Why: PR #92 review found workflow schedule idempotency replay accepted mismatched requests and preview did not expose final registration semantics. Bind idempotency keys to canonical request fingerprints, surface preview audit fields, and fail closed on semantic conflicts.

[砚砚/gpt-5.5🐾]
Why: hotfix reminder registration relies on preview/register parity and idempotency replay; invalid preview params must fail closed before posting, and request fingerprints must not drift with host locale collation.

Verification: MCP schedule focused 14/14 passed; API schedule focused 41/41 passed; dynamic task store focused 13/13 passed; schema/migration focused 69/69 passed; api+mcp lint passed; git diff --check passed. MCP full suite still has unrelated refresh-loop pending/cancelled failures.

[砚砚/GPT-5.5🐾]
Why: the merge gate reached pnpm check and failed only on Biome formatting in the schedule route test and schedule MCP tool; this keeps the approved behavior unchanged while making the full gate reproducible.

[砚砚/GPT-5.5🐾]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant