fix(manifest): bridge database_url credential to DATABASE_URL env; require it#7
Open
mgoldsborough wants to merge 1 commit into
Open
fix(manifest): bridge database_url credential to DATABASE_URL env; require it#7mgoldsborough wants to merge 1 commit into
mgoldsborough wants to merge 1 commit into
Conversation
…quire it
The manifest declared `user_config.database_url` but did not declare
`mcp_config.env: { DATABASE_URL: "${user_config.database_url}" }`, so
setting the URL via the Configure UI wrote to the workspace credential
store but never reached the bundle subprocess. The Python code only
reads `DATABASE_URL` from `os.environ`; without the env-substitution
mapping there was no path from credential file to env var.
In practice the bundle only worked when operators bypassed the
Configure flow entirely and set `DATABASE_URL` on the platform process
env (via K8s Secret + `allowedEnv: ["DATABASE_URL"]` passthrough),
which is the wrong primary path — it's per-tenant ops config instead
of per-workspace user config.
This commit:
- Adds the missing `mcp_config.env` mapping so the Configure UI value
flows through `prepareServer` env substitution at spawn time.
- Marks `database_url` as `required: true` in `user_config`. Existing
installs with the field unset will fail `prepareServer` at next
spawn with a clear "missing required configuration" error and the
`nb config set` hint — much better than the silent "DATABASE_URL is
not set" stack trace at first tool call.
- Drops the manifest description's reference to the
host-env-passthrough workaround. Configure UI is now the canonical
path.
Operators who currently rely solely on host-env passthrough on
production tenants need to set the credential via Configure UI (or
`nb config set ... database_url=...`) for affected workspaces. The
platform's set_user_config handler already auto-respawns the bundle
after a credential write — surfaced in v0.4.3 because the credential
now actually does something.
Bump 0.4.2 → 0.4.3.
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
The manifest declared
user_config.database_urlbut didn't declare amcp_config.envmapping to bridge it to theDATABASE_URLenv var the Python code reads. Setting the URL via Configure UI wrote to the workspace credential store but never reached the bundle subprocess.In production this manifested in
tenant-ipsdi— the bundle only worked because the operator had also setDATABASE_URLon the platform process env via K8s Secret +allowedEnv: ["DATABASE_URL"]passthrough. That's the wrong primary path: per-tenant ops config instead of per-workspace user config.Changes
mcp_config.env: { "DATABASE_URL": "${user_config.database_url}" }so the Configure-UI value flows through mpak'sprepareServerenv substitution at spawn time.database_urlasrequired: true. Pre-fix the bundle would silently start and fail at first tool call with a stack trace; post-fix it fails atprepareServerwith the canonical "missing required configuration" message and thenb config sethint.Bump 0.4.2 → 0.4.3.
Operator action required (breaking for misconfigured installs)
Existing workspaces with the field unset will fail at next bundle spawn after upgrading. Affected operators set the credential via Configure UI or
nb config set @nimblebraininc/synapse-db-query database_url=... -w <wsId>.Workspaces that already have the credential saved continue to work —
set_user_configalready auto-respawns the bundle, and v0.4.3 makes that respawn actually meaningful.Test plan
make check— 12 passingdatabase_urlvia Configure UI, run a query end-to-enddatabase_urland includes thenb config sethint