feat: add ability to attach source to an environment #809
Conversation
🦋 Changeset detectedLatest commit: c341462 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
qstearns
force-pushed
the
quinn/source-environments
branch
from
e1c6218 to
2f9e885
Compare
qstearns
force-pushed
the
quinn/source-environments
branch
from
2f9e885 to
6173181
Compare
qstearns
changed the base branch from
main
to
quinn/add-source-environments-table
qstearns
force-pushed
the
quinn/source-environments
branch
from
6173181 to
7d03814
Compare
qstearns
force-pushed
the
quinn/add-source-environments-table
branch
from
303753a to
c665eb6
Compare
qstearns
force-pushed
the
quinn/source-environments
branch
from
7d03814 to
5cc8a5f
Compare
qstearns
force-pushed
the
quinn/source-environments
branch
from
5cc8a5f to
cbc03e8
Compare
…e such that all tool calls originating from that source will have those environment variables apply
| ciEnv.Set(k, v) | ||
| } | ||
| } | ||
|
|
||
| // IMPORTANT: MCP servers accessed in a public manner or not gram authenticated, there is no concept of using stored environments for them | ||
| if envSlug != "" && payload.authenticated { | ||
| storedEnvVars, err := env.Load(ctx, payload.projectID, gateway.Slug(envSlug)) |
There was a problem hiding this comment.
It feels like this is where your code should hook in. Like we should only ever be loading one environment (I think?) so we either need to load the attached one or the one requested by the envSlug
There was a problem hiding this comment.
This is more or less how the delineation between user and system configuration works in this system. The idea is that if environment is specified by slug that's a means of using gram environments to specify user configuration, which might be entirely disjoint from system configuration
| @@ -111,11 +138,6 @@ func handleToolsCall( | |||
| ciEnv.Set(k, v) | |||
There was a problem hiding this comment.
What is supposed to happen when a user provides an env var that overrides one from an attached env?
There was a problem hiding this comment.
I wouldn't worry about introducing that. It's not an increase in scope. The user won't be able to set anything they already aren't able to today.
There was a problem hiding this comment.
Just to address both comments, we definitely want to load both environments, but environments loaded by slug and user supplied variables should be filtered by variables that are available in the tool call plan.
We do want to prefer client variables over server variables but only those explicitly provided in the spec
qstearns
force-pushed
the
quinn/source-environments
branch
from
5917b28 to
1f89c4c
Compare
move oauth token into environement resolution exclude server url when system environments are used pass through all environment variables not specified by plan
qstearns
force-pushed
the
quinn/source-environments
branch
from
1f89c4c to
c341462
Compare
Adds a dialog to attach environments to sources
UX
Affordance to access functionality is currently buried behind:
The interface for attaching an environment is implemented in this modal:
Things to pay Attention To
zustandfor state management)Dialogs and Dropdown Menu's to the moonshine versions of those components. I couldn't visually tell the difference and there were some really nasty interplay issues. Hoping this is a nice stability moveConsiderations