feat(deploy/persona-kit): cloud mode + relayfile watch personas (spec 03)#130
Merged
khaliqgant merged 3 commits intoMay 22, 2026
Merged
Conversation
Spec 03 workforce-side remaining work. Unstubs the cloud deploy mode so
proactive personas reach the cloud proactive-personas endpoint, and adds
the persona-kit surface (watch rules, mount.enabled, lint codes) that
makes those personas authorable.
- packages/deploy/src/modes/cloud/: replace the 1013-line stub
cloud.ts with a real launcher that routes proactive personas to
/api/v1/workspaces/:workspaceId/proactive-personas/:personaId.
- packages/deploy/src/{deploy,index,types,modes/input-values.test}.ts:
redirect cloud.js imports to cloud/index.js.
- packages/deploy/src/modes/cloud.test.ts: assert proactive personas
hit the proactive-personas endpoint exactly once and non-proactive
personas do not.
- packages/persona-kit/schemas/persona.schema.json: add WatchRule
definition and mount.enabled field.
- packages/persona-kit/src/{triggers,parse,plan,types,index}.ts +
matching .test.ts files: lint codes watch_path_not_absolute /
watch_empty_events, parse + plan the watch field, surface in
the public API.
- packages/persona-kit/src/__fixtures__/personas/proactive-watch-persona.json:
reference fixture for the new schema.
- examples/proactive-issue-resolver/: end-to-end example using a
watch-rule persona to react to relayfile changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds Relayfile watch rule types and parsing, mount.enabled support, cloud deploy changes to handle proactive personas and 'ready' status, and a complete proactive-issue-resolver example (agent, persona, docs, CLI) that generates specs and dispatches to Ricky with GitHub/Slack reporting. ChangesProactive Personas & Issue Resolver
Sequence DiagramsequenceDiagram
participant GitHub
participant Agent as proactive-issue-resolver.agent
participant Harness as Claude (harness)
participant Ricky
participant Slack
GitHub->>Agent: github.issues.opened payload
Agent->>GitHub: claim label / comment (start)
Agent->>Harness: run investigation prompt -> spec
Harness->>Agent: spec markdown
Agent->>Ricky: dispatch spec (local or cloud)
Ricky->>Agent: result (PR URL or failure)
Agent->>GitHub: comment (PR link or failure summary)
Agent->>Slack: notify (DM or channel)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/persona-kit/src/parse.ts (1)
266-280: ⚡ Quick winConsider simplifying the mount.enabled inference logic.
The nested ternary for inferring
enabled: truewhen patterns are present is difficult to follow. Consider refactoring for clarity:- const enabled = value.enabled; - if (enabled !== undefined && typeof enabled !== 'boolean') { - throw new Error(`${context}.enabled must be a boolean if provided`); - } - return ignoredPatterns || readonlyPatterns || enabled !== undefined - ? { - ...(typeof enabled === 'boolean' - ? { enabled } - : ignoredPatterns || readonlyPatterns - ? { enabled: true } - : {}), - ...(ignoredPatterns ? { ignoredPatterns } : {}), - ...(readonlyPatterns ? { readonlyPatterns } : {}) - } - : undefined; + const enabled = value.enabled; + if (enabled !== undefined && typeof enabled !== 'boolean') { + throw new Error(`${context}.enabled must be a boolean if provided`); + } + + // Return undefined when nothing is configured + if (!ignoredPatterns && !readonlyPatterns && enabled === undefined) { + return undefined; + } + + const result: PersonaMount = {}; + + // Set enabled: explicit value, or true if patterns present + if (enabled !== undefined) { + result.enabled = enabled; + } else if (ignoredPatterns || readonlyPatterns) { + result.enabled = true; + } + + if (ignoredPatterns) result.ignoredPatterns = ignoredPatterns; + if (readonlyPatterns) result.readonlyPatterns = readonlyPatterns; + + return result;🤖 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/src/parse.ts` around lines 266 - 280, The nested ternary that infers mount.enabled is hard to read; instead compute a simple boolean like hasPatterns = !!(ignoredPatterns || readonlyPatterns) and determine finalEnabled = typeof enabled === 'boolean' ? enabled : hasPatterns ? true : undefined, then only build and return the result object when hasPatterns || finalEnabled !== undefined, adding enabled, ignoredPatterns and readonlyPatterns conditionally; update the block that currently returns using the nested ternary (referencing enabled, ignoredPatterns, readonlyPatterns and context) to use these clearer intermediate variables and conditional spreads.examples/proactive-issue-resolver/agent.ts (2)
301-307: ⚡ Quick winSpec validity check is too narrow.
spec.startsWith('#')rejects any reasonable variant such as a leading```markdownfence, BOM, or a stray blank line that the harness can easily emit. Consider stripping a single leading code fence + whitespace before the check, or assert "contains an^# Spec:line within the first N lines" instead.♻️ Suggested loosening
- const spec = investigation.output.trim(); - if (investigation.exitCode !== 0 || !spec.startsWith('#')) { + const rawSpec = investigation.output + .replace(/^\uFEFF/, '') + .replace(/^```(?:markdown|md)?\s*\n([\s\S]*?)\n```$/m, '$1') + .trim(); + const spec = rawSpec; + if (investigation.exitCode !== 0 || !/^#\s+Spec:/m.test(spec.split('\n').slice(0, 5).join('\n'))) {🤖 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 `@examples/proactive-issue-resolver/agent.ts` around lines 301 - 307, The current spec validity check uses spec.startsWith('#') and is too strict; instead normalize investigation.output by removing a leading UTF-8 BOM, trimming leading blank lines and a single leading triple-backtick code fence (optionally with "markdown" or "md"), assign that cleaned string to spec, and then keep the existing exitCode guard but replace the startsWith check with a regex that asserts a "# Spec:" style header within the first few lines (e.g., test /^#\s+Spec:/m against spec.split('\n').slice(0,5).join('\n')); keep using investigation.exitCode, spec, ctx.github.comment, notifySlack and target.number for the existing error path.
62-78: 💤 Low valueUse exported
WorkforceCtxinstead of derivingctxfromhandler
Parameters<Parameters<typeof handler>[0]>[0]is effectively tying these helpers tohandler’s current signature, while@agentworkforce/runtimealready exportsWorkforceCtx(andWorkforceCtx’sIntegrationClientsincludesslack?: SlackClient). Importtype { WorkforceCtx }and use it directly fornotifySlack/claimViaLabel. [examples/proactive-issue-resolver/agent.ts]🤖 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 `@examples/proactive-issue-resolver/agent.ts` around lines 62 - 78, Replace the ad-hoc ctx type used in notifySlack (and similarly in claimViaLabel) with the exported WorkforceCtx type from `@agentworkforce/runtime`: import type { WorkforceCtx } and change the function signatures from using Parameters<Parameters<typeof handler>[0]>[0] to ctx: WorkforceCtx; update both notifySlack and claimViaLabel to use WorkforceCtx so the Slack integration typing (slack?: SlackClient) is correct and no longer tied to handler's signature.examples/proactive-issue-resolver/README.md (1)
49-81: 💤 Low valueReplace hardcoded personal absolute paths with repo-relative examples.
The install / run snippets bake in
/Users/khaliqgant/Projects/AgentWorkforce/workforce/.... Anyone else cloning the repo has to mentally translate these. Prefer something likecd $(git rev-parse --show-toplevel)/examples/proactive-issue-resolverso the doc is copy-pasteable.🤖 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 `@examples/proactive-issue-resolver/README.md` around lines 49 - 81, Replace the hardcoded absolute paths in the README run/install examples with repo-relative commands so others can copy-paste; update the install and run snippets that reference /Users/khaliqgant/... to compute the repository root dynamically (for example using git rev-parse --show-toplevel) or to use a relative path from the repo root, and update the example invocation of trigger-issue.sh (referenced as trigger-issue.sh) to use that repo-root-relative path; also update the cd example at the top of the file to use the same repo-relative approach so the documentation is portable.
🤖 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.
Inline comments:
In `@examples/proactive-issue-resolver/agent.ts`:
- Around line 230-236: The example call to ricky.generateLocalWorkflow sets
bestJudgement: true which conflicts with SPEC.md §4.6; either change the call to
use bestJudgement: false in the generateLocalWorkflow invocation (referencing
the response creation where spec, workflowName and run are passed) to strictly
follow the spec, or if you intend to allow improvisation update SPEC.md §4.6 to
explicitly allow bestJudgement: true and document the behavioral change; make
the decision and align either the generateLocalWorkflow call (bestJudgement
flag) or the SPEC text accordingly.
- Around line 80-111: claimViaLabel is vulnerable to race conditions because it
reads labels before writing; replace the pre-read approach with an
add-then-verify flow: call `gh issue edit --add-label ${CLAIM_LABEL}`
immediately (no prior gh issue view), then re-fetch the issue timeline/labels
and verify that the CLAIM_LABEL addition was authored by this run (e.g., inspect
timeline events or audit info to confirm this process was the first writer); if
verification shows another actor wrote the label, treat as 'already-claimed' and
abort (or, as a simpler mitigation, sleep briefly after add then re-read labels
and back out if you’re not the first). Update the misleading comment in the
handler that claims atomicity to accurately reflect the new behavior. Ensure
references to claimViaLabel and the handler comment are changed accordingly.
- Around line 1-9: The TypeScript error TS2307 occurs because imports from
'`@agentworkforce/ricky`' (seen in agent.ts via symbols handler, createRickySdk,
CloudGenerateRequest/CloudGenerateResponse/LocalResponse) are missing in CI; fix
by ensuring CI's typecheck ignores or has access to this example: either add
"examples/proactive-issue-resolver/**" to the "exclude" array in the
examples/tsconfig.json used by CI so these imports are skipped, or add the
examples/proactive-issue-resolver folder to pnpm-workspace.yaml so its
dependencies (including `@agentworkforce/ricky`) are installed during pnpm
install; pick one approach and apply it consistently so CI no longer reports
TS2307 for those imports.
In `@examples/proactive-issue-resolver/trigger-issue.sh`:
- Around line 28-47: The script currently allows an empty/"null" WORKSPACE_ID
and unvalidated OWNER/REPO/NUM which can produce invalid API calls; update the
block that sets WORKSPACE_ID (the WORKSPACE_ID=$(jq -r '.workspace' "$ACTIVE")
assignment) to detect if jq returned "null" or an empty string and exit with a
clear error if so, and validate/normalize OWNER, REPO, and NUM (the positional
vars used in the gh api "repos/$OWNER/$REPO/issues/$NUM" call) by trimming
whitespace, ensuring OWNER and REPO match expected GitHub name patterns (e.g.,
alphanumeric, hyphens, no slashes) and NUM is a positive integer; if any check
fails, print an error and exit before constructing the envelope or calling gh.
In `@packages/persona-kit/src/emit-schema.test.ts`:
- Line 87: The test currently uses assert.ok(personaSpec.properties?.watch)
which will pass if watch is the boolean true; update the assertion to ensure
watch is an object schema rather than a permissive boolean by asserting
personaSpec.properties?.watch exists and is not the boolean true (or by
asserting its typeof is 'object'), referencing the personaSpec.properties.watch
symbol so the test fails if watch becomes a boolean schema.
---
Nitpick comments:
In `@examples/proactive-issue-resolver/agent.ts`:
- Around line 301-307: The current spec validity check uses spec.startsWith('#')
and is too strict; instead normalize investigation.output by removing a leading
UTF-8 BOM, trimming leading blank lines and a single leading triple-backtick
code fence (optionally with "markdown" or "md"), assign that cleaned string to
spec, and then keep the existing exitCode guard but replace the startsWith check
with a regex that asserts a "# Spec:" style header within the first few lines
(e.g., test /^#\s+Spec:/m against spec.split('\n').slice(0,5).join('\n')); keep
using investigation.exitCode, spec, ctx.github.comment, notifySlack and
target.number for the existing error path.
- Around line 62-78: Replace the ad-hoc ctx type used in notifySlack (and
similarly in claimViaLabel) with the exported WorkforceCtx type from
`@agentworkforce/runtime`: import type { WorkforceCtx } and change the function
signatures from using Parameters<Parameters<typeof handler>[0]>[0] to ctx:
WorkforceCtx; update both notifySlack and claimViaLabel to use WorkforceCtx so
the Slack integration typing (slack?: SlackClient) is correct and no longer tied
to handler's signature.
In `@examples/proactive-issue-resolver/README.md`:
- Around line 49-81: Replace the hardcoded absolute paths in the README
run/install examples with repo-relative commands so others can copy-paste;
update the install and run snippets that reference /Users/khaliqgant/... to
compute the repository root dynamically (for example using git rev-parse
--show-toplevel) or to use a relative path from the repo root, and update the
example invocation of trigger-issue.sh (referenced as trigger-issue.sh) to use
that repo-root-relative path; also update the cd example at the top of the file
to use the same repo-relative approach so the documentation is portable.
In `@packages/persona-kit/src/parse.ts`:
- Around line 266-280: The nested ternary that infers mount.enabled is hard to
read; instead compute a simple boolean like hasPatterns = !!(ignoredPatterns ||
readonlyPatterns) and determine finalEnabled = typeof enabled === 'boolean' ?
enabled : hasPatterns ? true : undefined, then only build and return the result
object when hasPatterns || finalEnabled !== undefined, adding enabled,
ignoredPatterns and readonlyPatterns conditionally; update the block that
currently returns using the nested ternary (referencing enabled,
ignoredPatterns, readonlyPatterns and context) to use these clearer intermediate
variables and conditional spreads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 647a5932-c52a-4a72-9998-7412f7b832e8
⛔ Files ignored due to path filters (1)
examples/proactive-issue-resolver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
examples/proactive-issue-resolver/README.mdexamples/proactive-issue-resolver/SPEC.mdexamples/proactive-issue-resolver/agent.tsexamples/proactive-issue-resolver/package.jsonexamples/proactive-issue-resolver/persona.jsonexamples/proactive-issue-resolver/specs/cloud-mode-stub.mdexamples/proactive-issue-resolver/trigger-issue.shpackages/deploy/src/deploy.tspackages/deploy/src/index.tspackages/deploy/src/modes/cloud.test.tspackages/deploy/src/modes/cloud/index.tspackages/deploy/src/modes/input-values.test.tspackages/deploy/src/types.tspackages/persona-kit/schemas/persona.schema.jsonpackages/persona-kit/src/__fixtures__/personas/proactive-watch-persona.jsonpackages/persona-kit/src/emit-schema.test.tspackages/persona-kit/src/index.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.tspackages/persona-kit/src/plan.test.tspackages/persona-kit/src/plan.tspackages/persona-kit/src/triggers.test.tspackages/persona-kit/src/triggers.tspackages/persona-kit/src/types.ts
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+87
to
+89
| const view = await ctx.sandbox.exec( | ||
| `gh issue view ${t.number} --repo ${t.owner}/${t.repo} --json labels`, | ||
| ); |
Contributor
There was a problem hiding this comment.
🟡 Shell command injection via unescaped owner/repo in ctx.sandbox.exec calls
In claimViaLabel, t.owner and t.repo from the event payload are interpolated directly into shell command strings passed to ctx.sandbox.exec(). The targetFromPayload function at examples/proactive-issue-resolver/agent.ts:38-50 only validates these are non-empty strings — it does not sanitize shell metacharacters. In --mode dev, the NDJSON envelope comes from stdin, so a crafted payload with repository.name set to e.g. "; rm -rf /; echo " would result in arbitrary command execution inside the sandbox.
Affected call sites
Line 88: gh issue view ${t.number} --repo ${t.owner}/${t.repo} --json labels
Line 105: gh issue edit ${t.number} --repo ${t.owner}/${t.repo} --add-label ${CLAIM_LABEL}
Both interpolate payload-sourced strings into a shell command without quoting or escaping.
Prompt for agents
The claimViaLabel function in examples/proactive-issue-resolver/agent.ts interpolates t.owner and t.repo directly into shell command strings at lines 88 and 104-105. These values come from the event payload (via targetFromPayload) and are only checked for non-emptiness, not sanitized for shell metacharacters. Since ctx.sandbox.exec takes a string command that is likely executed via a shell, this is a command injection risk.
To fix, either:
1. Shell-escape the owner and repo values before interpolation (e.g. wrap each in single quotes with internal single quotes escaped), or
2. Validate that owner and repo match a safe pattern like /^[a-zA-Z0-9._-]+$/ in targetFromPayload before returning them, or
3. If the sandbox.exec API supports an argv-style call (array of args), use that instead of string interpolation.
The same issue applies to both sandbox.exec calls in claimViaLabel (lines 88 and 104-105).
Was this helpful? React with 👍 or 👎 to provide feedback.
Per the v1-consolidation discussion on AgentWorkforce/cloud#912 (closed)
→ AgentWorkforce/cloud#919 (draft).
Previously the cloud launcher bifurcated on `isProactivePersona(persona)`:
proactive personas (persona.watch.length > 0) hit
/api/v1/workspaces/{ws}/proactive-personas/{id} with a body that
duplicated watch[] + mount{} as sibling fields, and used
deleteProactivePersona to tear down. Regular personas hit
/api/v1/workspaces/{ws}/deployments and used deleteAgent.
The v1 surface (agents + deployments) already handles
persona.watch via cloud's preparePersonaDeploy + the new
agents.watch_rules column (cloud#919). The parallel surface was
~90% redundant per the reviewer's stabilization-trail analysis.
Now every persona — proactive or not — flows through:
- POST /api/v1/workspaces/{ws}/deployments with the same body shape
the regular path always used. The persona's top-level watch[]
travels inside the persona object; cloud extracts and persists it
as watch_rules on the agents row.
- DELETE /api/v1/workspaces/{ws}/deployments/{agentId} via deleteAgent.
- The handleExistingPersona check now applies universally (was
previously skipped for proactive personas).
Removed:
- isProactivePersona, readPersonaWatch, readPersonaMount helpers
- proactivePersonasEndpoint URL builder
- deleteProactivePersona network helper
- Sibling `watch` + `mount` fields from the POST body (read from persona)
Tests:
- cloud.test.ts: the two proactive-specific tests now assert the
unified /deployments path (and that /proactive-personas is never
called). The "keeps non-proactive on deployments" test is unchanged.
The persona-kit watch / mount.enabled schema additions remain in this PR
unchanged — those are how authors *declare* the watch DSL, independent
of how the launcher routes it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spec 03 workforce-side remaining work. Unstubs the cloud deploy mode so proactive personas reach the cloud `proactive-personas` endpoint, and adds the persona-kit surface (`watch` rules, `mount.enabled`, lint codes) that makes those personas authorable. Branched fresh from `origin/main` to keep unrelated cli/version-bump churn out of this PR.
deploy
persona-kit
example
Acceptance gate covered (from pear spec 03)
Test plan
🤖 Generated with Claude Code