Skip to content

cleanup: Remove skill env var dependency prompting#22721

Merged
xl-openai merged 1 commit into
mainfrom
xl/plugins2
May 19, 2026
Merged

cleanup: Remove skill env var dependency prompting#22721
xl-openai merged 1 commit into
mainfrom
xl/plugins2

Conversation

@xl-openai
Copy link
Copy Markdown
Collaborator

Deletes the skill env var dependency prompt feature and its runtime path. env_var entries in skill dependency metadata are now silently ignored during skill loading.

@xl-openai xl-openai requested a review from a team as a code owner May 14, 2026 23:27
Comment on lines 1034 to -1039
},
FeatureSpec {
id: Feature::SkillEnvVarDependencyPrompt,
key: "skill_env_var_dependency_prompt",
stage: Stage::UnderDevelopment,
default_enabled: false,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rm'ing outright would break for anyone who has it set. maybe make it Stage::Removed instead

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we are fail open to unrecognized feature, no?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yea with warning. with --strict-config it breaks though. not a huge deal but we can play it safe

Comment on lines 392 to 396
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

codex:

This makes the test stop asserting that env_var dependencies are loaded, but the loader still accepts them in resolve_dependency_tool and keeps them in SkillMetadata.dependencies.tools. That means they are still surfaced through downstream skill metadata APIs; only the runtime prompt path was removed.

If the intended behavior is that env_var entries are "silently ignored during skill loading", should we filter them out in the loader and replace this deletion with a test that asserts they are dropped?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, you can actually put whatever you want into dependencies — today it’s just treated as a string. It’s basically a “ignore what we don’t understand” model.

@xl-openai xl-openai enabled auto-merge (squash) May 19, 2026 01:23
@xl-openai xl-openai merged commit 6b54ced into main May 19, 2026
31 checks passed
@xl-openai xl-openai deleted the xl/plugins2 branch May 19, 2026 01:24
@github-actions github-actions Bot locked and limited conversation to collaborators May 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants