One-click deploy: persona as single source of truth (persona-kit + CLI)#146
Open
khaliqgant wants to merge 3 commits into
Open
One-click deploy: persona as single source of truth (persona-kit + CLI)#146khaliqgant wants to merge 3 commits into
khaliqgant wants to merge 3 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an ChangesDeploy Requirements Feature
CLI One‑Click Deploy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 a mechanism to derive deployment requirements directly from a persona definition, eliminating the need for a separate manifest. It adds support for marking integrations as optional, updates the schema and parser accordingly, and includes comprehensive unit tests. The reviewer suggested improving the robustness of the platform secret derivation by handling unknown backends defensively and aggregating provider names in the reason field for shared secrets.
Comment on lines
+184
to
+211
| function derivePlatformSecrets( | ||
| integrations: RequiredIntegration[], | ||
| opts: DeriveDeployRequirementsOptions, | ||
| ): RequiredPlatformSecret[] { | ||
| const resolveBackend = opts.resolveBackend ?? (() => 'nango' as const); | ||
| const isWebhookGap = opts.isWebhookGapProvider ?? (() => false); | ||
| const seen = new Map<string, RequiredPlatformSecret>(); | ||
|
|
||
| for (const { provider } of integrations) { | ||
| const backend = resolveBackend(provider); | ||
| const names = [...PLATFORM_SECRETS_BY_BACKEND[backend]]; | ||
| if (isWebhookGap(provider)) { | ||
| names.push(WEBHOOK_GAP_PLATFORM_SECRET); | ||
| } | ||
| for (const name of names) { | ||
| if (!seen.has(name)) { | ||
| seen.set(name, { | ||
| name, | ||
| required: true, | ||
| reason: `required for the ${provider} integration (${backend}) to function`, | ||
| provider, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return [...seen.values()].sort((a, b) => a.name.localeCompare(b.name)); | ||
| } |
There was a problem hiding this comment.
Robustness & Clarity Improvement
- Defensive Programming: If
resolveBackendreturns a backend that is not registered inPLATFORM_SECRETS_BY_BACKEND(e.g., due to a new backend added in the cloud app beforepersona-kitis updated),PLATFORM_SECRETS_BY_BACKEND[backend]will beundefined. Attempting to spread it ([...undefined]) will throw a runtimeTypeError. We should check for this and throw a descriptive error instead. - Aggregated Reasons: When multiple integrations share the same platform secret (e.g.,
NangoSecretKeyorWebRelayauthApiKey), the current implementation associates the secret with only the first provider processed. If that provider is optional and not connected, the deployer might see a confusing reason (e.g., "required for the github integration" when they only connected slack). Aggregating the providers in thereasonfield makes the requirements much clearer and more accurate.
function derivePlatformSecrets(
integrations: RequiredIntegration[],
opts: DeriveDeployRequirementsOptions,
): RequiredPlatformSecret[] {
const resolveBackend = opts.resolveBackend ?? (() => 'nango' as const);
const isWebhookGap = opts.isWebhookGapProvider ?? (() => false);
const secretProviders = new Map<string, { providers: string[]; backend: string }>();
for (const { provider } of integrations) {
const backend = resolveBackend(provider);
const backendSecrets = PLATFORM_SECRETS_BY_BACKEND[backend];
if (!backendSecrets) {
throw new Error("Unsupported integration backend \"" + backend + "\" for provider \"" + provider + "\"");
}
const names = [...backendSecrets];
if (isWebhookGap(provider)) {
names.push(WEBHOOK_GAP_PLATFORM_SECRET);
}
for (const name of names) {
let entry = secretProviders.get(name);
if (!entry) {
entry = { providers: [], backend };
secretProviders.set(name, entry);
}
entry.providers.push(provider);
}
}
return [...secretProviders.entries()]
.map(([name, { providers, backend }]) => ({
name,
required: true,
reason: "required for the following integration(s) to function: " + providers.join(', ') + " (" + backend + ")",
provider: providers[0],
}))
.sort((a, b) => a.name.localeCompare(b.name));
}![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
khaliqgant
changed the title
This was referenced May 27, 2026
…integration optional?
One-click deploy reads the persona definition as the single source of truth (no
separate manifest — that was a drift-prone 2nd source).
- PersonaIntegrationConfig.optional?: boolean (additive, default required) +
parse; lets persona authors mark non-essential integrations optional.
- deploy-requirements.ts: deriveDeployRequirements(persona: PersonaSpec, opts?)
→ { integrations (required = !optional), inputs (no default & not optional),
firesOn, platformSecrets:[] for Layer A }. Layer-B (--isolated) backend-keyed
platform-secret derivation is a documented fast-follow (needs the
provider→backend registry; no hand-list).
- Regenerated persona.schema.json for the new field.
Salvages wf#142's secret-derivation into the read-from-persona path; AgentManifest
dropped. persona-kit typecheck + full suite green (230/230).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ation Salvages the dropped AgentManifest's secret-derivation into the read-from-persona path. platformSecrets is now derived for Layer-B (--isolated) from each declared provider's credential BACKEND (PLATFORM_SECRETS_BY_BACKEND: nango→Nango+WebRelayauth, composio→Composio+WebRelayauth; +HookdeckSigningSecret for Nango-webhook-gap providers like gitlab) — keyed by BACKEND, not a drift-prone per-provider hand-list. The provider→backend registry lives in the cloud app, so resolveBackend/isWebhookGapProvider are INJECTED by the cloud caller (default backend = nango). Layer A stays platformSecrets: []. persona-kit typecheck + full suite green (231/231). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
khaliqgant
force-pushed
the
feat/one-click-read-persona
branch
from
19c9cbc to
785aa81
Compare
![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: 1
🤖 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 `@packages/cli/src/deploy-command.ts`:
- Around line 86-89: The guard that routes to the one-click flow only checks for
the standalone token '--one-click' so inline flags like
'--one-click=persona.json' fall through; update the condition that calls
runOneClickDeploy(...) to detect both the exact token and the inline form (e.g.,
check args.some(a => a === '--one-click' || a.startsWith('--one-click='))). This
will ensure parseOneClickDeployArgs(...) and runOneClickDeploy(...) receive
inline args correctly and prevent the flag from being treated as unknown by
parseDeployArgs(...).
🪄 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: 0601fa81-c655-4e45-b429-b15ecd2d0294
📒 Files selected for processing (8)
packages/cli/src/deploy-command.test.tspackages/cli/src/deploy-command.tspackages/persona-kit/schemas/persona.schema.jsonpackages/persona-kit/src/deploy-requirements.test.tspackages/persona-kit/src/deploy-requirements.tspackages/persona-kit/src/index.tspackages/persona-kit/src/parse.tspackages/persona-kit/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/persona-kit/schemas/persona.schema.json
- packages/persona-kit/src/index.ts
- packages/persona-kit/src/parse.ts
- packages/persona-kit/src/deploy-requirements.test.ts
- packages/persona-kit/src/types.ts
- packages/persona-kit/src/deploy-requirements.ts
Comment on lines
+86
to
+89
| if (args.includes('--one-click')) { | ||
| await runOneClickDeploy(args); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Dispatch --one-click=<path> through the one-click code path.
parseOneClickDeployArgs(...) supports the inline form, but this guard only recognizes a standalone --one-click token. agentworkforce deploy --one-click=persona.json currently falls through to parseDeployArgs(...) and fails as an unknown flag.
Suggested fix
- if (args.includes('--one-click')) {
+ if (args.some((arg) => arg === '--one-click' || arg.startsWith('--one-click='))) {
await runOneClickDeploy(args);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (args.includes('--one-click')) { | |
| await runOneClickDeploy(args); | |
| return; | |
| } | |
| if (args.some((arg) => arg === '--one-click' || arg.startsWith('--one-click='))) { | |
| await runOneClickDeploy(args); | |
| return; | |
| } |
🤖 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/cli/src/deploy-command.ts` around lines 86 - 89, The guard that
routes to the one-click flow only checks for the standalone token '--one-click'
so inline flags like '--one-click=persona.json' fall through; update the
condition that calls runOneClickDeploy(...) to detect both the exact token and
the inline form (e.g., check args.some(a => a === '--one-click' ||
a.startsWith('--one-click='))). This will ensure parseOneClickDeployArgs(...)
and runOneClickDeploy(...) receive inline args correctly and prevent the flag
from being treated as unknown by parseDeployArgs(...).
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.
Consolidates the one-click deploy work into a single PR. Previously split as #146 (persona-kit) + #145 (CLI); the CLI commit has been folded in here so the library and its only consumer land together.
What
One-click agent deploy reads the persona definition as the single source of truth — no separate manifest (the AgentManifest approach in #142 was closed/superseded as drift-prone).
persona-kit
PersonaIntegrationConfig.optional?: boolean(additive, default required) + parse + regeneratedpersona.schema.json.deriveDeployRequirements(persona, opts?)→{ integrations, inputs, firesOn, platformSecrets }.platformSecrets: []— only prompt is "connect these integrations".--isolated): backend-keyed platform-secret derivation salvaged from feat(persona-kit): agent manifest schema v1 for one-click deploy #142 (PLATFORM_SECRETS_BY_BACKEND), provider→backend registry injected by the cloud caller (defaultnango).CLI
deploy --one-click <persona.json|persona.ts>: load/compile the persona, derive requirements from the PersonaSpec, render the Layer A plan, and delegate to the existing cloud deploy (mode=cloud,onExists=update).Diff
8 files — 6 persona-kit +
cli/src/deploy-command.ts(+test). +782/-2.Supersedes
Tests
persona-kit suite green; CLI deploy-command tests included. (Branch is ~2 commits behind main — will rebase before merge.)