Skip to content

[tiny-agents] Handle env variables in tiny-agents (JS client) #1501

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

Merged
merged 2 commits into from
Jun 3, 2025

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented May 28, 2025

This PR adds support for the inputs configuration defined by VSCode (see docs).

PR adapted from huggingface/huggingface_hub#3129 using Cursor. Made some minor adjustments manually.

How to test

pnpm run cli run wauplin/library-pr-reviewer

image

@Wauplin
Copy link
Contributor Author

Wauplin commented May 28, 2025

Cursor prompt:

both huggingface.js and huggingface_hub are defining a tiny-agents CLI. For huggingface_hub (Python) it happens in @cli.py . For huggingface.js, it happens in @cli.ts . Recently I've recently opened @huggingface/huggingface_hub#3129 to handle env variables in the Python CLI. Could you port this PR to the JS version? Do not change anything else that is not directly related to the PR. Thank you!

Copy link
Collaborator

@evalstate evalstate left a comment

Choose a reason for hiding this comment

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

Tested with this config file:

{
	"model": "Qwen/Qwen2.5-72B-Instruct",
	"provider": "nebius",
	"inputs": [
		{
			"type": "promptString",
			"id": "hftoken",
			"description": "Hugging Face Token",
			"password": true
		}
	],
	"servers": [
		{
			"type": "stdio",
			"config": {
				"command": "docker",
				"args": ["run", "-i", "--rm", "-p", "3000:3000", "-e", "TRANSPORT=stdio", "-e", "HF_TOKEN", "hf-mcp-server"],
				"env": {
					"HF_TOKEN": "${input:hftoken}"
				}
			}
		}
	]
}

During setup, I had some errors in my Docker arguments and was getting stuck with the CTRL+C handler not exiting. I haven't been able to reproduce though.

The prompt for the token was not appearing for this config (WSL2) without the extra newline so have left that as a suggestion.

stdout.write(ANSI.BLUE);
stdout.write(` • ${inputId}`);
stdout.write(ANSI.RESET);
stdout.write(`: ${description}. (default: load from ${Array.from(inputVars).join(", ")}) `);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
stdout.write(`: ${description}. (default: load from ${Array.from(inputVars).join(", ")}) `);
stdout.write("\n");

I am having rendering issues without this (it doesn't display the variable)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've reproduced the issue with this config file:

{
	"model": "Qwen/Qwen2.5-72B-Instruct",
	"provider": "nebius",
	"inputs": [
		{
			"type": "passwordInput",
			"id": "hftoken",
			"description": "Hugging Face Token",
			"password": true
		}
	],
	"servers": [
		{
			"type": "stdio",
			"config": {
				"command": "docker",
				"args": ["run", "-i", "--rm", "-e", "TRANSPORT=stdio", "-e", "HF_TOKEN=${input:hftoken}", "hf-mcp-server"],
				"env": {
					"HF_TOKEN": "${input:hftoken}"
				}
			}
		},
		{
			"type": "stdio",
			"config": {
				"command": "npx",
				"args": ["@playwright/mcp@latest"]
			}
		}
	]
}

Might be related to having two STDIO servers (so probably not related specifically to this PR).

i'm not super satisfied by the verbosity but...
Comment on lines +166 to +188
if ((server.type === "http" || server.type === "sse") && server.config.options?.requestInit?.headers) {
for (const [key, value] of Object.entries(server.config.options.requestInit.headers)) {
if (value.includes(envSpecialValue)) {
if (userInput) {
server.config.options.requestInit.headers[key] = value.replace(envSpecialValue, userInput);
} else {
const valueFromEnv = process.env[key] || "";
server.config.options.requestInit.headers[key] = value.replace(envSpecialValue, valueFromEnv);
if (valueFromEnv) {
stdout.write(ANSI.GREEN);
stdout.write(`Value successfully loaded from '${key}'`);
stdout.write(ANSI.RESET);
stdout.write("\n");
} else {
stdout.write(ANSI.YELLOW);
stdout.write(`No value found for '${key}' in environment variables. Continuing.`);
stdout.write(ANSI.RESET);
stdout.write("\n");
}
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

note: Claude 4 generated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👀 well Claude could have done some refactoring efforts instead of duplicating the entire "load from env" logic ^^

Comment on lines +24 to +31
/**
* Customizes HTTP requests to the server.
*/
requestInit: z
.object({
headers: z.record(z.string()).optional(),
})
.optional(),
Copy link
Member

Choose a reason for hiding this comment

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

semi-related comment for @Wauplin, @hanouticelina and @evalstate:

i am starting to think we should have gone with a simplified configuration schema (the one from VS Code, probably), rather than be more explicit and use the types from the MCP SDK, given here for instance, we need to nest the headers quite a bit inside of the config.

Maybe for a v2 we'll change this! no rush, though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well making the switch to something more standard right now is probably better (not in this PR but as follow-up). Agree the config nesting is not really needed

Copy link
Contributor Author

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Cannot approve myself but still ✔️ @julien-c 's changes. Let's merge :)

@Wauplin Wauplin merged commit 0416cec into main Jun 3, 2025
5 checks passed
@Wauplin Wauplin deleted the handle-env-variables-in-tiny-agents branch June 3, 2025 15:55
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.

4 participants