Better handling skill depdenencies on ENV VAR. #9017

xl-openai · 2026-01-10T03:07:49Z

An experimental flow for env var skill dependencies. Skills can now declare required env vars in SKILL.md; if missing, the CLI prompts the user to get the value, and Core will store it in memory (eventually to a local persistent store)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08c5dfd013

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/app-server/src/bespoke_event_handling.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b9694f7dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

codex-rs/core/src/skills/dependency_flow.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77ba509ded

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-01-28T07:53:55Z

codex-rs/core/src/skills/env_var_dependencies.rs

+        let mut user_note = None;
+        for entry in &answer.answers {
+            if let Some(note) = entry.strip_prefix("user_note: ") {
+                if !note.trim().is_empty() {
+                    user_note = Some(note.trim().to_string());


Accept non-"user_note" request_user_input answers

This extraction only looks for entries prefixed with "user_note: ". The RequestUserInputAnswer protocol doesn’t require that prefix, and other clients (including core tests that submit answers: ["yes"]) send plain answer strings. In those cases, values stays empty and the env var is never persisted or injected, so skills will still be missing their required env vars after the prompt. Consider also accepting the first non-empty answer (or any non-user_note entry) when parsing responses.

Useful? React with 👍 / 👎.

Seems like a legit concern.

I think all clients should follow the same behavior, there is no reason to only set "user_note:" for TUI.

mzeng-openai · 2026-01-28T21:51:38Z

codex-rs/core/src/skills/env_store.rs

+/// Env var persistence uses keyring first, then falls back to a local file.
+/// Keys are the raw env var names, without skill-level namespacing.
+/// The file lives at `codex_home/env_vars.json` with 0600 permissions on Unix.
+pub(crate) fn load_env_var(codex_home: &Path, name: &str) -> io::Result<Option<String>> {


Not a blocker but we should consider consolidating this with the existing logic in codex-rs/rmcp-client/src/oauth.rs. Feels like we should have a secret manager for handling these things.

this is coming soon!

mzeng-openai · 2026-01-28T21:55:12Z

codex-rs/core/src/codex.rs

    )
    .await;

+    let config = turn_context.client.config();


Should this happen before we spin up missing mcp servers?

mzeng-openai

Stamping to unblock! Please take a look at the issue that Codex spotted if possible.

viyatb-oai · 2026-01-29T03:04:34Z

codex-rs/core/src/skills/env_var_dependencies.rs

+    pub(crate) description: Option<String>,
+}
+use crate::skills::SkillMetadata;
+/// Resolve required dependency values (session cache, then env vars),


nit:new line between imports and function signature?

viyatb-oai · 2026-01-29T03:05:17Z

codex-rs/core/src/skills/env_var_dependencies.rs

+        let name = dependency.name.clone();
+        if !seen_names.insert(name.clone()) {
+            continue;
+        }


can you use Hashmap.difference method?

difference only gives you set membership and drops per‑item context; here we must keep each dependency’s metadata (skill name/description)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fba945ca6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-01-29T03:06:25Z

codex-rs/core/src/skills/env_var_dependencies.rs

+        match env::var(&name) {
+            Ok(value) => {
+                loaded_values.insert(name.clone(), value);
+                continue;


Respect shell env policy when auto-loading deps

When the feature is enabled, this reads env::var directly and caches the value even if it would be excluded by ShellEnvironmentPolicy (e.g., the default *KEY*/*TOKEN* filters). Because ShellHandler later merges dependency_env into exec_params.env, those secrets get injected into every shell run without prompting, bypassing the policy and potentially leaking credentials. Consider only auto-loading vars that would pass create_env (or prompt if excluded) so the policy continues to gate secret propagation.

Useful? React with 👍 / 👎.

It's OK for now.

chatgpt-codex-connector · 2026-01-29T03:06:25Z

codex-rs/core/src/skills/env_var_dependencies.rs

+                is_other: false,
+                is_secret: true,


Preserve isSecret in app-server request_user_input

is_secret: true is now set for env-var dependency prompts, but the app-server v2 protocol (ToolRequestUserInputQuestion) doesn’t carry isSecret, and bespoke_event_handling drops any extra fields. In app-server clients the flag will be lost, so secret prompts will render as plain text even though core intends masking. This should be forwarded through the app-server protocol/mapping so non-TUI clients can respect secret inputs.

Useful? React with 👍 / 👎.

viyatb-oai

minor nits but lgtm

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e3d4dfd49

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-01-29T18:42:49Z

codex-rs/core/src/skills/env_var_dependencies.rs

+        match env::var(&name) {
+            Ok(value) => {
+                loaded_values.insert(name.clone(), value);
+                continue;


Treat empty env vars as missing dependencies

resolve_skill_dependencies_for_turn treats any Ok(value) from env::var as satisfied, so a dependency like API_KEY set to the empty string will be cached and skip the prompt. In shells/CI it’s common to export empty placeholders, and downstream tools typically treat empty values as “missing,” so this will silently proceed with an unusable credential. Consider treating empty/whitespace-only values as missing and prompting instead.

Useful? React with 👍 / 👎.

chatgpt-codex-connector bot reviewed Jan 10, 2026

View reviewed changes

codex-rs/app-server/src/bespoke_event_handling.rs Outdated Show resolved Hide resolved

xl-openai force-pushed the xl/skill branch from 08c5dfd to 8b9694f Compare January 22, 2026 01:37

chatgpt-codex-connector bot reviewed Jan 22, 2026

View reviewed changes

codex-rs/core/src/skills/dependency_flow.rs Outdated Show resolved Hide resolved

xl-openai force-pushed the xl/skill branch from 8b9694f to 77ba509 Compare January 28, 2026 07:50

chatgpt-codex-connector bot reviewed Jan 28, 2026

View reviewed changes

xl-openai requested a review from mzeng-openai January 28, 2026 07:54

mzeng-openai reviewed Jan 28, 2026

View reviewed changes

mzeng-openai approved these changes Jan 28, 2026

View reviewed changes

xl-openai requested a review from viyatb-oai January 29, 2026 02:58

xl-openai force-pushed the xl/skill branch from 77ba509 to 4fba945 Compare January 29, 2026 02:59

viyatb-oai reviewed Jan 29, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Jan 29, 2026

View reviewed changes

viyatb-oai approved these changes Jan 29, 2026

View reviewed changes

xl-openai added 3 commits January 29, 2026 13:12

Better handling skill dependencies on ENV VAR.

bbbd5e0

Remove env_store, add mask behavior

c19fb85

Address comments.

2e3d4df

xl-openai force-pushed the xl/skill branch from 1dd4258 to 2e3d4df Compare January 29, 2026 18:35

chatgpt-codex-connector bot reviewed Jan 29, 2026

View reviewed changes

xl-openai merged commit bdd8a7d into main Jan 29, 2026
56 of 60 checks passed

xl-openai deleted the xl/skill branch January 29, 2026 19:13

github-actions bot locked and limited conversation to collaborators Jan 29, 2026

Better handling skill depdenencies on ENV VAR. #9017

Better handling skill depdenencies on ENV VAR. #9017

Uh oh!

Conversation

xl-openai commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzeng-openai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

viyatb-oai left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xl-openai commented Jan 10, 2026 •

edited

Loading