Skip to content

[fix][plat] Allow for npm registry env vars to be set - #364

Merged
jamie-retool merged 5 commits into
mainfrom
jd/npm-proxy-env-vars
Aug 17, 2026
Merged

[fix][plat] Allow for npm registry env vars to be set#364
jamie-retool merged 5 commits into
mainfrom
jd/npm-proxy-env-vars

Conversation

@jamie-retool

Copy link
Copy Markdown
Contributor

Adds easy setting of npm registry settings for the agent executor proxy.

@greptile-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 3/5

This PR should not merge until the mirrored values file is updated and incomplete npm token Secret references are rejected.

The change currently fails the repository's exact values-file synchronization check, and a partially configured token Secret renders an unusable proxy Deployment.

Files Needing Attention: charts/retool/values.yaml, values.yaml, and charts/retool/templates/deployment_agent_sandbox.yaml

Important Files Changed

Filename Overview
charts/retool/templates/deployment_agent_sandbox.yaml Adds proxy npm-registry environment variables, but permits a named Secret with an empty key, which prevents the proxy pod from starting.
charts/retool/values.yaml Defines the npm-registry configuration surface, but omits the required matching update to the root values.yaml and therefore fails the synchronization workflow.

Reviews (1): Last reviewed commit: "[fix][plat] Allow for npm registry env v..." | Re-trigger Greptile

Comment thread charts/retool/values.yaml
Comment thread charts/retool/templates/deployment_agent_sandbox.yaml

@ryanartecona ryanartecona left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment on lines +783 to +785
{{- if and $npm.authTokenSecret.name (not $npm.authTokenSecret.key) -}}
{{- fail "agentSandbox.proxy.npmRegistry.authTokenSecret.name is set without .key, which would render an empty secretKeyRef and leave the proxy unable to start. Set .key to the entry holding the registry token." -}}
{{- end -}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe worth another check here to fail if $npm.authToken and $npm.authTokenSecret.name are both set, since the secret won't get used at all in that case?

@jamie-retool
jamie-retool merged commit c43969c into main Aug 17, 2026
13 checks passed
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.

2 participants