-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: unify server-side selector execution #7185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
BillLeoutsakosvl346
wants to merge
23
commits into
staging
Choose a base branch
from
feat/unified-server-selector-execution
base: staging
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
b094718
feat(selectors): execute dynamic selectors server-side
a343f97
fix(selectors): expose route verb to contract audit
db0ffc0
test(selectors): align migration expectations
855ce1f
fix(selectors): address review findings
a135092
Merge origin/staging into feat/unified-server-selector-execution
8075e62
Merge origin/staging into feat/unified-server-selector-execution
d328092
fix(selectors): harden exact reference handling
7e0caab
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
96df0b7
fix(selectors): address Cubic review findings
f78fc4e
fix(selectors): address Cubic rerun findings
316c2f0
fix(selectors): close final Cubic review gaps
589a326
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
5558e19
fix(selectors): close final review follow-ups
8720e07
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
d78e567
fix(imap): restore deployment binding on rollback
46e31a3
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
d8db8ac
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
a5ddb7f
fix(imap): scope webhook updates to active deployment
677f3ea
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
c4365b4
test(selectors): consolidate migration regression coverage
7fe8c90
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
845acaa
refactor(selectors): isolate unified selector contract
8a68b31
Merge remote-tracking branch 'origin/staging' into feat/unified-serve…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| --- | ||
| name: add-selector | ||
| description: Add or update a Sim dynamic selector using the shared manifest, server attachment, and selectors.execute path. Use for provider-backed, internal, or local option lists referenced by block, trigger, or connector selectorKey fields. | ||
| argument-hint: <selector-key> | ||
| --- | ||
|
|
||
| # Add Selector | ||
|
|
||
| Dynamic selectors expose option metadata while a workflow or connector is being configured. Every | ||
| remote selector executes through the authorized `selectors.execute` application operation; the | ||
| browser never resolves credentials or calls a provider directly. | ||
|
|
||
| ## Read the shared boundary | ||
|
|
||
| Before editing, read: | ||
|
|
||
| - `apps/sim/lib/selectors/types.ts` | ||
| - `apps/sim/lib/selectors/manifest.ts` | ||
| - `apps/sim/lib/selectors/context.ts` | ||
| - `apps/sim/lib/selectors/server/types.ts` | ||
| - `apps/sim/lib/selectors/server/registry.ts` | ||
| - `apps/sim/hooks/queries/selectors.ts` | ||
|
|
||
| Then read the nearest existing selector attachment and the block, trigger, or connector declaration | ||
| that will consume the key. | ||
|
|
||
| ## Classify the selector | ||
|
|
||
| - `provider-server`: contacts an external provider or uses provider credentials. | ||
| - `internal-server`: reads protected Sim data through an existing authorized application use case. | ||
| - `local`: pure browser-safe data with no protected data, credentials, references, or network I/O. | ||
|
|
||
| Add every key to the browser-safe manifest in `lib/selectors/manifest.ts`. `SelectorKey` derives from | ||
| that manifest; do not maintain a second union. Manifest entries contain data only: allowed context, | ||
| readiness, scope kinds, list/search/detail capabilities, and stale time. Do not import provider SDKs, | ||
| credentials, server helpers, or attachment functions into the manifest. | ||
|
|
||
| ## Build context from active values | ||
|
|
||
| Declare `dependsOn` on the consuming sub-block or connector field. The shared context builder sends | ||
| only declared, active dependencies: | ||
|
|
||
| - Canonical basic/advanced pairs contribute the active value under their canonical key. | ||
| - Action and trigger modes contribute only fields active on that surface. | ||
| - Exact environment references such as `{{GMAIL_CREDENTIAL_ID}}` remain unresolved in the browser. | ||
| - Runtime block-output references are not selector context. | ||
| - Embedded environment interpolation such as `https://{{HOST}}/path` is unsupported. | ||
|
|
||
| Add a new `SelectorContextKey` only when the value is a real, reusable selector dependency. Allow it | ||
| explicitly on each relevant manifest entry. Never send a full block or connector configuration. | ||
|
|
||
| ## Add the server attachment | ||
|
|
||
| For `provider-server`, add the service's attachment map under | ||
| `apps/sim/lib/selectors/server/providers/` and include it in the exhaustive server registry. For | ||
| `internal-server`, add the attachment in `apps/sim/lib/selectors/server/internal.ts`. Local keys use | ||
| the exhaustive browser-safe registry in `apps/sim/lib/selectors/client/local.ts` and never enter the | ||
| server registry. A provider attachment declares: | ||
|
|
||
| - Credential policy, including the exact context field and trusted `serviceIds`. | ||
| - Destination policy: `fixed`, `credential-bound`, or `user-controlled`. | ||
| - A list/detail adapter that explicitly projects `id`, `label`, and allowlisted scalar `meta`. | ||
|
|
||
| Stored credentials must pass actor-use, workspace, and provider/service binding checks. Do not trust | ||
| a provider, service, operation kind, origin, or module name supplied by the browser. | ||
|
|
||
| Choose the destination policy deliberately: | ||
|
|
||
| - `fixed`: provider origin is code-defined. | ||
| - `credential-bound`: origin/account/site comes from, or is verified against, the authorized | ||
| credential. | ||
| - `user-controlled`: the user selects the destination. Hidden use-only authentication requires an | ||
| explicit security policy; do not combine it with an arbitrary destination by default. | ||
|
|
||
| Reuse or extract a server-only provider listing primitive. If an existing provider route has | ||
| non-selector callers, keep the route as a thin caller of that primitive. If it is selector-only, | ||
| move the logic and remove the obsolete route and contract. Never import a route handler or make an | ||
| internal HTTP request from an attachment. | ||
|
|
||
| The attachment must return normalized selector results only. It must not return provider payloads, | ||
| resolved context, credential IDs, tokens, or secrets. Let the shared executor own scope | ||
| authorization, exact-reference resolution, credential authorization, error projection, output | ||
| sanitization, and abort propagation. | ||
|
|
||
| ## Wire the UI declaration | ||
|
|
||
| Point the block, trigger, or connector field at `selectorKey` and declare its `dependsOn` fields. | ||
| Keep connector selector/manual canonical pairs and fork reconfiguration behavior intact. Static | ||
| `options` stay local and need no selector. | ||
|
|
||
| Do not add: | ||
|
|
||
| - A module under `hooks/selectors/providers` or any client provider fetcher. | ||
| - A provider-specific React Query key. | ||
| - A selector-specific OAuth-token request. | ||
| - A selector-only API route when the provider primitive can be called directly. | ||
|
|
||
| All server selectors use the shared POST contract and React Query facade. Query identities must stay | ||
| opaque and must not include context values, references, credential IDs, secrets, or their hashes. | ||
|
|
||
| ## Focused validation | ||
|
|
||
| Follow nearby Vitest and route-test style. Do not add an authorization matrix for every ordinary | ||
| provider attachment; the shared executor tests own shared security behavior. | ||
|
|
||
| Add a focused adapter test when behavior is special, such as pagination, nontrivial destination | ||
| binding, provider-specific projection, or a raw-connection policy. For an ordinary fixed-origin OAuth | ||
| list, manifest/registry exhaustiveness plus an existing provider primitive test is usually enough. | ||
|
|
||
| Run the smallest relevant set, then: | ||
|
|
||
| ```bash | ||
| bunx vitest run <focused selector tests> | ||
| bun run --cwd apps/sim type-check | ||
| bun run check:fork-dependent-coverage | ||
| bun run check:client-boundary | ||
| git diff --check | ||
| ``` | ||
|
|
||
| Confirm there is no browser-side provider call, every server key has one attachment, and every | ||
| returned option is explicitly projected. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| interface: | ||
| display_name: "Add Selector" | ||
| short_description: "Build a secure dynamic selector" | ||
| brand_color: "#2563EB" | ||
| default_prompt: "Use $add-selector to add or update a Sim dynamic selector through the unified server execution path." |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.