Skip to content

feat(deploy): support optional integrations#252

Merged
khaliqgant merged 2 commits into
mainfrom
feat/optional-deploy-integrations
Jun 24, 2026
Merged

feat(deploy): support optional integrations#252
khaliqgant merged 2 commits into
mainfrom
feat/optional-deploy-integrations

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

  • add persona-kit metadata for optional integrations enabled by declared persona inputs
  • filter inactive optional integrations and their provider triggers before connect, bundle staging, runtime credentials, and launch
  • validate optional integration input wiring and document the deploy-time behavior

Testing

  • pnpm --filter @agentworkforce/persona-kit test
  • pnpm --filter @agentworkforce/deploy test

Review in cubic

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 20 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 51d35567-c5bc-47dd-991c-9a955725813e

📥 Commits

Reviewing files that changed from the base of the PR and between 5de4da2 and 2833c1a.

📒 Files selected for processing (10)
  • docs/plans/deploy-v1.md
  • packages/deploy/src/deploy.test.ts
  • packages/deploy/src/deploy.ts
  • packages/deploy/src/preflight.ts
  • packages/persona-kit/schemas/persona.schema.json
  • packages/persona-kit/src/define.ts
  • packages/persona-kit/src/emit-schema.test.ts
  • packages/persona-kit/src/parse.test.ts
  • packages/persona-kit/src/parse.ts
  • packages/persona-kit/src/types.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/optional-deploy-integrations

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d1d7a6655

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/deploy/src/deploy.ts Outdated
const mode: DeployMode = opts.mode ?? pickMode(opts);
warnings.push(...preflight.warnings);
for (const w of preflight.warnings) io.warn(w);
const activePreflight = selectActiveOptionalIntegrations(preflight, opts.inputs ?? {});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resolve picker-backed inputs before pruning integrations

When enabledByInput is itself a picker backed by the optional provider, such as a Slack channel input with picker.provider: "slack", this filters Slack out before connectAndCollectIntegrations and before the later collectPickerInputs phase. Since collectPickerInputs only prompts for providers present in connectedProviders, the provider is never connected and the picker is skipped, so interactive deploy cannot enable this optional integration unless the user already knows and passes --input manually.

Useful? React with 👍 / 👎.

Comment thread packages/deploy/src/deploy.ts Outdated
Comment on lines +429 to +431
const envName = spec?.env ?? inputName;
const envValue = env[envName];
if (typeof envValue === 'string') return envValue.trim().length > 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Forward env-resolved activation values to remote modes

When an optional integration is enabled only by a local environment variable, including a custom spec.env, this check activates the provider but the resolved launch inputs are still initialized from opts.inputs only, and sandbox/cloud deployments do not inherit the deployer's process.env as persona input values. That leaves remote deployments with provider triggers registered but without the input value that caused activation, so required input resolution can fail or handlers can run without the expected configuration.

Useful? React with 👍 / 👎.

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Review: PR #252 — feat(deploy): support optional integrations

Summary

This PR adds opt-in integrations to the deploy flow. An integration can declare optional: true + enabledByInput: "<INPUT>"; the provider's connection and trigger registration are skipped unless that persona input resolves to a non-empty value (explicit input > env > input default). Changes span persona-kit (parse/types/schema/define) and deploy (preflight validation + selectActiveOptionalIntegrations filtering). I traced callers, exports, and types; the new exports (PersonaInputSpec, KNOWN_TRIGGER_PROVIDER_ALIASES) are all present and used.

I made no file edits — there is no mechanical fix to apply, and the one logic/test concern is not safe for me to auto-edit.

Verification (ran the way CI does)

Reproduced the canonical CI pipeline (.github/workflows/ci.yml) end-to-end with pnpm 10.17.1 in a clean environment:

  • pnpm -r run build — all 16 packages built; committed persona.schema.json matches emit:schema output (tree stayed clean).
  • pnpm -r typecheck — all packages pass.
  • pnpm -r lint — all packages pass.
  • pnpm run test — all packages report fail 0 (persona-kit 286, deploy 210, others green).

Important note on a sandbox-only false failure

When I first ran the deploy tests, deploy refuses an agent whose optional integration inputs leave no active listeners failed with deploy aborted: 1 integration(s) failed to connect: slack. Root cause: this sandbox injects SLACK_CHANNEL="C0ALQ06AAUT" into process.env, and personaInputIsSet (deploy.ts:431) intentionally falls back to process.env, so the "slack" optional integration activated instead of being skipped. GitHub Actions CI (ci.yml) sets no such variable, and with SLACK_CHANNEL unset the test passes. So this is not a CI failure — it's an artifact of the harness environment. I did not touch the test (test changes are a human decision, and weakening a passing-on-CI test would be wrong).

Addressed comments

  • No bot or human review comments were available in this run (gh CLI is not installed in the sandbox and .workforce/context.json carries no review threads). Nothing to reconcile.

Findings

No blocking issues. The new logic is fail-closed where it matters:

  • validateActiveAgent (deploy.ts:470) throws when optional filtering removes the last listener, rather than silently launching a no-op agent — correct and safe.
  • preflightPersona (preflight.ts:107) rejects an enabledByInput that isn't declared in persona.inputs — good guard, exercised by tests.
  • Alias handling in filterInactiveTriggers/selectActiveOptionalIntegrations correctly accounts for preflight.agent already being alias-normalized (it adds both the raw provider slug and its canonical alias to the inactive set).

Advisory Notes

  • Test is coupled to ambient process.env (hardening suggestion, not in scope for me to change). deploy refuses an agent whose optional integration inputs leave no active listeners (deploy.test.ts:~669) and deploy activates optional integrations from supplied persona inputs rely on SLACK_CHANNEL/TELEGRAM_CHAT being unset in the environment. Because production personaInputIsSet consults process.env, these tests are non-hermetic and will flake on any machine/CI step that exports those names. A human may want to isolate the env in these tests (e.g., delete/override process.env for the duration, or inject a fixed env map). I deliberately did not modify the tests or the env-fallback behavior, since the fallback is an intentional resolution rule and altering the test to mask environmental coupling is a human call.

Conclusion

On the canonical CI environment this PR builds, typechecks, lints, and tests green; the working tree is clean. The single observed failure is a sandbox-environment artifact, not a CI failure. The remaining advisory (test env hermeticity) needs human judgment. I cannot confirm GitHub's mergeable/checks state from here (no gh access), so I am not declaring the PR ready for merge from this run.

@khaliqgant khaliqgant merged commit 12357f4 into main Jun 24, 2026
3 checks passed
@khaliqgant khaliqgant deleted the feat/optional-deploy-integrations branch June 24, 2026 13:29
@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

No files were edited — the PR is mechanically and semantically clean, and the full CI pipeline passes locally. Here is my review.


Review: PR #252feat(deploy): support optional integrations

Summary

Adds opt-in "optional integrations" to the deploy flow. A persona integration can declare optional: true + enabledByInput: "SOME_INPUT"; deploy includes that provider (its connection + agent triggers) only when the named input resolves to a non-empty value (explicit --input, env, or default). Inactive optional providers are pruned before connect/trigger registration, and resolved input values are forwarded to launchers as WORKFORCE_INPUT_* env. Schema, types, parser, and preflight validation are extended in lockstep.

Verification (ran the full CI pipeline locally — build → lint → typecheck → test)

  • Build: passes. Notably persona-kit's emit:schema generator and workload-router's generate:personas both ran; git status is clean afterward, so the hand-edited persona.schema.json matches generator output (no committed-artifact drift).
  • Lint: passes (exit 0).
  • Typecheck: passes (exit 0).
  • Test: passes — deploy 212/212, persona-kit 286/286, repo-wide 0 failures. All 7 new tests pass, and the existing …without provider-token leakage guard still passes.

Correctness traces I validated against the current checkout

  • Alias handling (filterInactiveTriggers + triggerProviderAlias): preflight normalizes agent trigger keys to canonical names (google-mailgmail) while persona integrations stay aliased. The code adds both the raw provider key and its canonical alias to the inactive set, so a pruned google-mail integration correctly removes the gmail-keyed agent trigger. Correct.
  • Two-pass selection: the second selectActiveOptionalIntegrations(preflight, resolvedInputs) recomputes from the original preflight (not the already-filtered activePreflight), so picker-deferred providers are correctly pruned/kept after picker collection. Correct.
  • WORKFORCE_INPUT_* env: composes with the existing wrapper in index.ts:145 (wrapLauncher); on overlap the values are identical, and the new path additionally forwards env/default-resolved activation inputs when opts.inputs is empty (covered by the cloud-mode test). No conflict, no token leak.
  • Fail-closed preserved: validateActiveAgent throws when pruning leaves no listeners (fail-closed), and preflightPersona rejects an enabledByInput that isn't declared in persona.inputs. No fail-open conversions introduced.

Addressed comments

  • No bot or human review comments were present in .workforce/context.json or the provided context files, so there were none to reconcile. (PR has no prior human approval recorded in context; review treated as first pass.)

Files auto-edited

  • None. The change is internally consistent and passes the full pipeline; nothing mechanical (lint/format/typo/import-order) needed fixing.

Advisory Notes

These are out of scope for a code change in this PR (or require human judgment) and are left as notes only:

  • Connect-then-prune for picker-deferred optionals (deploy.ts:249 connect vs. deploy.ts:305 re-select): when an optional integration is picker-backed and deferred, connectAndCollectIntegrations runs against the first-pass activePreflight.persona before the picker prompt resolves. If the operator then declines the picker, that provider is pruned from the result via activeConnectedIntegrations (deploy.ts:316) but the connection side-effect already occurred. Functionally harmless (the result list is filtered correctly), but it can connect a provider that ends up inactive. Worth a human decision on whether to defer the connect for picker-deferred optionals; I did not change it because reordering the connect/picker/prune sequence is a behavior change beyond a mechanical fix.
  • resolvedInputValues only set when input not already in inputs (deploy.ts:421): intentional (avoids clobbering caller-supplied values), and consistent across both passes — flagging only so a reviewer confirms the intent matches the spec.

The PR is feature-complete, internally consistent, and green across the canonical build/lint/typecheck/test pipeline. The remaining items above are advisory only and need human judgment rather than a mechanical fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant