fix(mcp-workforce): align memory scope enum with narrowed PersonaMemoryScope#105
fix(mcp-workforce): align memory scope enum with narrowed PersonaMemoryScope#105khaliqgant wants to merge 1 commit into
Conversation
mcp-workforce was landed in #91 against an older `PersonaMemoryScope` shape (`session | user | workspace | org | object`). #94 then tightened the type to `workspace | user | global`. Both PRs passed CI independently, but main is now broken at build time because the zod enum in `server.ts` and the runtime `VALID_SCOPES` Set in `tools/memory.ts` still reference the removed literals. Aligning both call sites to the canonical persona-kit shape: - `MEMORY_SCOPE_ENUM` → z.enum(['workspace', 'user', 'global']) - `VALID_SCOPES` → new Set(['workspace', 'user', 'global']) - memory.save tool description updated to match The default scope stays `workspace`. Callers that previously passed `'session'`/`'org'`/`'object'` will now get a validation error from the zod schema before the runtime check — preferable to silently mapping them to a different scope. Verified: `pnpm -F @agentworkforce/mcp-workforce typecheck` + `build` + `test` (23/23) all pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR narrows the ChangesMemory Scope Narrowing
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mcp-workforce/src/server.ts (1)
13-13: ⚡ Quick winSingle-source the memory scope literals to prevent future drift.
This PR fixes a drift issue, but the literals are still duplicated between
server.tsandtools/memory.ts. Please centralize them in one exportedas consttuple and derive both the zod enum and runtime validation from it.♻️ Suggested refactor
-const MEMORY_SCOPE_ENUM = z.enum(['workspace', 'user', 'global']); +import { MEMORY_SCOPES } from './tools/memory.js'; +const MEMORY_SCOPE_ENUM = z.enum(MEMORY_SCOPES);+export const MEMORY_SCOPES = ['workspace', 'user', 'global'] as const; const VALID_SCOPES: ReadonlySet<PersonaMemoryScope> = new Set([ - 'workspace', - 'user', - 'global' + ...MEMORY_SCOPES ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mcp-workforce/src/server.ts` at line 13, The MEMORY_SCOPE_ENUM literal array is duplicated; centralize the allowed scope strings by exporting a single as const tuple (e.g. MEMORY_SCOPES) from tools/memory.ts, then replace the local z.enum([...]) in server.ts with z.enum(MEMORY_SCOPES) (or z.enum([...MEMORY_SCOPES]) if needed) and derive any runtime type/validation from that same exported tuple (update imports and usages of MEMORY_SCOPE_ENUM to use MEMORY_SCOPES and its derived types), ensuring all places that previously referenced MEMORY_SCOPE_ENUM now import the single source of truth from tools/memory.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/mcp-workforce/src/server.ts`:
- Line 13: The MEMORY_SCOPE_ENUM literal array is duplicated; centralize the
allowed scope strings by exporting a single as const tuple (e.g. MEMORY_SCOPES)
from tools/memory.ts, then replace the local z.enum([...]) in server.ts with
z.enum(MEMORY_SCOPES) (or z.enum([...MEMORY_SCOPES]) if needed) and derive any
runtime type/validation from that same exported tuple (update imports and usages
of MEMORY_SCOPE_ENUM to use MEMORY_SCOPES and its derived types), ensuring all
places that previously referenced MEMORY_SCOPE_ENUM now import the single source
of truth from tools/memory.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 922fa5f2-6768-43e7-a49e-c74000d37e08
📒 Files selected for processing (2)
packages/mcp-workforce/src/server.tspackages/mcp-workforce/src/tools/memory.ts
Why CI is red on main
mcp-workforce(added in #91) was authored against an olderPersonaMemoryScopeshape —'session' | 'user' | 'workspace' | 'org' | 'object'.#94then narrowed the type to'workspace' | 'user' | 'global'. Each PR passed CI independently. After both merged, main fails to build:This is the exact case CodeRabbit flagged on #94 (
packages/persona-kit/src/types.ts:186— "remaining uses of removed literals"). At the time of the #94 sweep,mcp-workforcewasn't yet on main, so the audit returned clean. The cross-PR collision only surfaces post-merge.What this changes
Two call sites in
mcp-workforce, both pointing at the canonical persona-kit shape:src/server.ts:13z.enum(['session', 'user', 'workspace', 'org', 'object'])z.enum(['workspace', 'user', 'global'])src/server.ts:65src/tools/memory.ts:38-44Setof 5 old literalsSetof 3 canonical literalsmemory.save's default scope staysworkspace.Behavior changes for consumers
Callers that previously passed
'session','org', or'object'will now get azodvalidation error before the runtime check — preferable to silently mapping to a different scope. No mapping layer is added because none of these old values were in production (mcp-workforce is0.0.0on npm — a placeholder).Verified locally
pnpm -F @agentworkforce/mcp-workforce typecheck— cleanpnpm -F @agentworkforce/mcp-workforce build— cleanpnpm -F @agentworkforce/mcp-workforce test— 23/23 pass🤖 Generated with Claude Code