Repository navigation
Preserve trigger maxConcurrency in persona-kit - #230
Conversation
📝 WalkthroughWalkthroughAdds an optional integer ChangesTrigger maxConcurrency Support
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
pr-reviewer could not complete review for #230 in AgentWorkforce/workforce. |
![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 an optional maxConcurrency property to the agent trigger specification, which serves as a delivery backpressure hint. The parser has been updated to validate and preserve only positive integers, treating invalid values as unset. The changes include updates to TypeScript interfaces, JSON schemas, and unit tests to support and verify this new field. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
pr-reviewer could not complete review for #230 in AgentWorkforce/workforce. |
|
Bot/CI audit at HEAD a7d3e6d:\n\n- GitHub CI |
|
Current-head audit after reviewer-requested follow-up at HEAD 866e65f:\n\n- Added the intentional-leniency parser comment for .\n- Added absent-value coverage and explicit assertions that / remain strict and throw when present-but-invalid.\n- Local focused gates rerun:
✔ definePersona returns authored specs that parse successfully (60.201459ms)
|
|
Current-head audit after reviewer-requested follow-up at HEAD 866e65f:
Ready for clr-bp re-verification at current head. |
|
Reviewed PR #230 against the current checkout and current GitHub PR state. I found no reproducible breakage and made no code changes. Validation
Addressed comments
Advisory Notes |
|
Implemented one scoped fix for the PR. The parser only preserves positive integer Changed: Addressed comments
Advisory Notes
ValidationPassed locally:
I did not print |
|
Implementation-complete handoff for clr-bp review at HEAD 866e65f:
Relay CLI DM/channel posting is currently blocked for cdx-bp with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/persona-kit/scripts/emit-schema.mjs (1)
137-147: ⚡ Quick winAdd explanatory comment for consistency.
The two preceding post-processing blocks (workspace_service_account.name at lines 50-54, tags at lines 84-89) include comments explaining why the post-processing is necessary. Adding a similar comment here would maintain consistency and clarify for future maintainers why maxConcurrency requires integer+minimum constraints.
📝 Suggested comment structure
+// Post-process: tighten the PersonaIntegrationTrigger.maxConcurrency constraint +// to match parseIntegrationTrigger (integer >= 1). The generator emits a bare +// `{ "type": "number" }` because the constraint lives in the parser. Without this, +// the schema accepts 0 or 1.5 which the parser then omits as invalid. const agentTrigger = agentSchema.definitions?.PersonaIntegrationTrigger;🤖 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/persona-kit/scripts/emit-schema.mjs` around lines 137 - 147, Add a short explanatory comment above the post-processing block that modifies agentSchema.definitions?.PersonaIntegrationTrigger.properties.maxConcurrency explaining why we coerce type to 'integer' and add minimum: 1 (e.g., to enforce whole-number concurrency and disallow non-positive values for runtime/validation compatibility), matching the style of the earlier comments for workspace_service_account.name and tags; place the comment immediately above the agentTrigger modification so future maintainers see the intent for the maxConcurrency integer+minimum constraint.
🤖 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/persona-kit/scripts/emit-schema.mjs`:
- Around line 137-147: Add a short explanatory comment above the post-processing
block that modifies
agentSchema.definitions?.PersonaIntegrationTrigger.properties.maxConcurrency
explaining why we coerce type to 'integer' and add minimum: 1 (e.g., to enforce
whole-number concurrency and disallow non-positive values for runtime/validation
compatibility), matching the style of the earlier comments for
workspace_service_account.name and tags; place the comment immediately above the
agentTrigger modification so future maintainers see the intent for the
maxConcurrency integer+minimum constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f01c0ec9-b1a3-4868-af0d-4c8ad1ebcc99
📒 Files selected for processing (5)
packages/persona-kit/schemas/agent.schema.jsonpackages/persona-kit/scripts/emit-schema.mjspackages/persona-kit/src/emit-schema.test.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/persona-kit/schemas/agent.schema.json
- packages/persona-kit/src/parse.ts
|
Merge triage: clr-bp impl signoff @ e2685af (independent clean-worktree verify, 277 persona-kit tests pass, RED-without-fix proof — reverting parse.ts maxConcurrency handling fails 2 tests). CI green, no actionable inline bot findings (gemini summary-only; cubic NEUTRAL). This is the durable declaration contract for cloud#2092's stopgap (cloud reads raw input.agent today). Merging. |
|
Reviewed and fixed the PR-scoped issue that appeared after the branch advanced: the agent schema now emits Local validation passed:
Remote status observed at final check: PR head Addressed comments
Advisory NotesNone. |
Summary
Fixes #229.
Persona-kit now preserves trigger-level
maxConcurrencyfromdefineAgentspecs instead of stripping it duringparseAgentSpec().Changes:
maxConcurrency?: numbertoPersonaIntegrationTrigger.maxConcurrency?: numberto the typed authoringTypedTrigger<P>surface used by runtimedefineAgenttrigger maps.parseIntegrationTrigger(); invalid values are treated as unset to match Cloud #2092 semantics.match/whereremain strict and throw when present-but-invalid, whilemaxConcurrencyis lenient because it is an optional backpressure hint and Cloud treats invalid values as unset.packages/persona-kit/schemas/agent.schema.jsonsomaxConcurrencyis schema-visible.match/wherestrictness, and typed authoring in tests.Out of scope: Cloud still keeps its #2092 raw-read stopgap until the later Cloud follow-up consumes parsed trigger fields.
Worktree provenance: implemented in
/tmp/wt-workforce-bp-pr1, branched from workforceorigin/mainat96273fd, not fromfix/normalize-trigger-aliases-rebased.Validation
Passed:
pnpm install --frozen-lockfilepnpm --filter @agentworkforce/persona-kit test(277 tests)pnpm --filter @agentworkforce/persona-kit buildpnpm run buildgit diff --checkCI/bot status at the first PR head:
check: SUCCESSagent-relay-codepr-reviewer failed to run with harness exit code 1 and posted no review; no code finding was produced to address. Bot audit comment: Preserve trigger maxConcurrency in persona-kit #230 (comment)Caveat:
pnpm run checkpassed lint and typecheck afterpnpm run build, then failed inpackages/cli/dist/local-personas.test.jswith 34 local persona cascade assertions about missing lower-layer personas/sidecars. Those failures are outside the touched package and unrelated tomaxConcurrency; details are recorded in/tmp/subs-fix/REPORT-followup-bp.md.