feat(persona-kit): typed ProactiveCapabilities on PersonaSpec (#1555 PR-B)#169
Conversation
…555 PR-B source-of-truth)
|
Warning Review limit reached
More reviews will be available in 14 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces typed proactive capabilities (ProactiveCapabilities) to the persona-kit PersonaSpec, updating the JSON schema, types, parser, tests, and fixtures. The review feedback correctly identifies a potential validation gap where arrays could bypass the isObject check and be treated as objects, and provides a code suggestion to explicitly reject arrays using Array.isArray.
| function parseCapabilityValue(value: unknown, context: string): CapabilityValue { | ||
| if (typeof value === 'boolean') return value; | ||
| if (!isObject(value)) { | ||
| throw new Error(`${context} must be a boolean or object if provided`); | ||
| } | ||
| if (value.enabled !== undefined && typeof value.enabled !== 'boolean') { | ||
| throw new Error(`${context}.enabled must be a boolean if provided`); | ||
| } | ||
| return { ...value } as CapabilityValue; | ||
| } | ||
|
|
||
| export function parseCapabilities( | ||
| value: unknown, | ||
| context: string | ||
| ): ProactiveCapabilities | undefined { | ||
| if (value === undefined) return undefined; | ||
| if (!isObject(value)) { | ||
| throw new Error(`${context} must be an object if provided`); | ||
| } |
There was a problem hiding this comment.
In JavaScript/TypeScript, typeof [] is 'object' and null is already checked by isObject, but arrays will still pass the isObject check. This means that if an array is passed as a capability value (e.g., review: [true]) or for the capabilities field itself (e.g., capabilities: []), it will not throw a validation error. Instead, parseCapabilityValue will return an object representation of the array (e.g., { '0': true }), and parseCapabilities will silently ignore the array and return undefined.
To prevent this, we should explicitly check and reject arrays using Array.isArray(value).
| function parseCapabilityValue(value: unknown, context: string): CapabilityValue { | |
| if (typeof value === 'boolean') return value; | |
| if (!isObject(value)) { | |
| throw new Error(`${context} must be a boolean or object if provided`); | |
| } | |
| if (value.enabled !== undefined && typeof value.enabled !== 'boolean') { | |
| throw new Error(`${context}.enabled must be a boolean if provided`); | |
| } | |
| return { ...value } as CapabilityValue; | |
| } | |
| export function parseCapabilities( | |
| value: unknown, | |
| context: string | |
| ): ProactiveCapabilities | undefined { | |
| if (value === undefined) return undefined; | |
| if (!isObject(value)) { | |
| throw new Error(`${context} must be an object if provided`); | |
| } | |
| function parseCapabilityValue(value: unknown, context: string): CapabilityValue { | |
| if (typeof value === 'boolean') return value; | |
| if (!isObject(value) || Array.isArray(value)) { | |
| throw new Error(`${context} must be a boolean or object if provided`); | |
| } | |
| if (value.enabled !== undefined && typeof value.enabled !== 'boolean') { | |
| throw new Error(`${context}.enabled must be a boolean if provided`); | |
| } | |
| return { ...value } as CapabilityValue; | |
| } | |
| export function parseCapabilities( | |
| value: unknown, | |
| context: string | |
| ): ProactiveCapabilities | undefined { | |
| if (value === undefined) return undefined; | |
| if (!isObject(value) || Array.isArray(value)) { | |
| throw new Error(`${context} must be an object if provided`); | |
| } |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/persona-kit/src/parse.ts">
<violation number="1" location="packages/persona-kit/src/parse.ts:858">
P2: Top-level capabilities accepts array. Invalid config gets silently ignored. Reject array input.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| function parseCapabilityValue(value: unknown, context: string): CapabilityValue { | ||
| if (typeof value === 'boolean') return value; | ||
| if (!isObject(value)) { |
There was a problem hiding this comment.
P2: Top-level capabilities accepts array. Invalid config gets silently ignored. Reject array input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/persona-kit/src/parse.ts, line 858:
<comment>Top-level capabilities accepts array. Invalid config gets silently ignored. Reject array input.</comment>
<file context>
@@ -851,6 +853,44 @@ export function parseMemory(value: unknown, context: string): PersonaMemory | un
+function parseCapabilityValue(value: unknown, context: string): CapabilityValue {
+ if (typeof value === 'boolean') return value;
+ if (!isObject(value)) {
+ throw new Error(`${context} must be a boolean or object if provided`);
+ }
</file context>
| if (!isObject(value)) { | |
| if (!isObject(value) || Array.isArray(value)) { |
|
Reviewed PR #169 locally and made two fixes:
Local verification completed:
|
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed PR #169 locally and made two fixes:
Local verification completed:
|
![agent-relay-bot[bot]](https://avatars.githubusercontent.com/in/3624277?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ pr-reviewer applied fixes — committed and pushed b209965 to this PR. The notes below describe what changed.
Reviewed PR #169 locally and made two fixes:
- Tightened
parseCapabilitiesso arrays are rejected for both the top-level capabilities object and individual capability values, matching the generated schema and TypeScript shape. - Added focused parser tests for valid capability declarations and malformed capability inputs.
- Fixed an unrelated runtime test isolation failure exposed by the full check by clearing all ambient relay/workforce token env vars in the no-auth memory test.
Local verification completed:
corepack pnpm --filter @agentworkforce/persona-kit testcorepack pnpm --filter @agentworkforce/runtime testcorepack pnpm run check
1 participant
Adds the canonical typed capabilities schema (review/issueClaim/conflictAutofix + legacy pullRequest alias; CapabilityValue = boolean | {enabled?}) to PersonaSpec. Source-of-truth home for cloud PR-A's locally-typed registry. Additive + optional, zero behavior change. Cloud will re-import these types after this publishes.