Skip to content

fix(tasks): don't share one retention allowance across anonymous callers (#320) - #326

Open
mcollina wants to merge 1 commit into
mainfrom
fix/320-anonymous-task-retention
Open

mcollina wants to merge 1 commit into
mainfrom
fix/320-anonymous-task-retention

Conversation

@mcollina

Copy link
Copy Markdown
Member

Problem

taskMaxRetainedPerPrincipal (default 100) counted every caller without an identity as a single owner: MemoryTaskStore matched undefined === undefined, and RedisTaskStore put every anonymous task in the fleet into one index, mcp:tasks:anonymous. On a deployment without authorization, any client could therefore evict another client's finished task before that client collected it, or block all new anonymous tasks by keeping 100 running.

Fix

  • Tasks with no subject and no issuer (new isAnonymousOwner() helper) are no longer held to the per-principal cap. A per-principal quota for callers who can't be told apart is one allowance that any of them can use up for everyone else.
  • All anonymous tasks together are capped by a new option, taskMaxRetainedAnonymous (default 1000, the same as the memory store's total capacity). Eviction works as before: at the cap, the oldest finished anonymous task is evicted (orphaned leases are failed first). If every task is still running, the call fails with -32029 "Task limit reached".
  • Memory: the existing per-owner check now uses the anonymous cap for anonymous owners. The store-wide maxTasks (1000) still applies on top.
  • Redis: the existing atomic CREATE_SCRIPT runs against mcp:tasks:anonymous with the anonymous cap as its limit. The cap therefore holds across instances with no new keys and no extra round trips.
  • Identified principals (issuer + subject, see fix(tasks): bind tasks to token issuer as well as subject #296) work exactly as before, each with its own quota.
  • README and the JSDoc for taskMaxRetainedPerPrincipal / taskMaxRetainedAnonymous are updated. The old advice to raise the per-principal quota for anonymous workloads now points to taskMaxRetainedAnonymous.

Tests

  • Memory store: anonymous tasks above the per-principal cap are kept, and a completed task is not evicted (the repro from the issue). The anonymous cap refuses or evicts the oldest finished task. The store-wide cap still applies. Identified quota is unchanged. Bad option values are rejected.
  • Redis store: the same scenarios, including the anonymous cap being enforced across two store instances.
  • Plugin level: the repro from the issue over tools/call / tasks/get, plus validation of taskMaxRetainedAnonymous. The two existing anonymous over-the-wire quota tests now use taskMaxRetainedAnonymous.
  • npm run lint, npm run typecheck and npm run test pass (775/775).

Fixes #320

🤖 Generated with Claude Code

https://claude.ai/code/session_01LmhsFjk8W96X2PvmShmDm9

…ers (#320)

taskMaxRetainedPerPrincipal treated every caller without an identity as one
owner (undefined === undefined in MemoryTaskStore, the single
mcp:tasks:anonymous index in RedisTaskStore), so on a deployment without
authorization any client could evict or lock out everyone else's tasks.

Anonymous tasks (no subject and no issuer) are no longer subject to the
per-principal cap. They are bounded together by a new store-wide anonymous
cap, taskMaxRetainedAnonymous (default 1000, matching the memory store's
total capacity), with the same eviction rules. RedisTaskStore enforces it
atomically in the existing create script against the anonymous index, so
it holds across instances. Identified principals keep their own quota.

Fixes #320

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LmhsFjk8W96X2PvmShmDm9

This branch has not been deployed

No deployments
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.

All unauthenticated callers share one retained-task allowance

1 participant