mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Preserve skill descriptions outside model context (#29006)
## Why Skill descriptions are used in model-visible lists: the default available-skills catalog that supports implicit selection, and the on-demand `skills.list` tool response used to discover orchestrator skills. A single overlong description should not consume a disproportionate share of either list. Enforcing the 1024-character limit while loading or migrating skills is the wrong boundary: it rejects otherwise-valid skills and discards metadata that non-model consumers and full skill reads may need. Skill metadata and `SKILL.md` content should remain intact; the cap belongs at model-visible list rendering boundaries. ## What changed - Preserve full `description` and `metadata.short-description` values when loading skills. - Preserve full external-agent command descriptions during `source-command-*` migration instead of skipping commands solely because their descriptions exceed 1024 characters. - Preserve full normalized orchestrator descriptions in the underlying skills catalog. - Cap each description at 1024 Unicode characters when rendering the default available-skills context in `codex-core-skills` and `codex-skills-extension`. - Apply the same cap when serializing descriptions in the model-visible `skills.list` response. - Render truncated descriptions as 1021 original characters plus `...`. - Leave explicit `$skill` injection, `skills.read`, underlying metadata, and on-disk `SKILL.md` files unchanged and full-fidelity. ## Implicit skill selection Codex injects a bounded catalog containing each implicitly allowed skill's name, description, and source locator, together with instructions to use a skill when the task clearly matches its description. The model makes that semantic choice; after selecting a skill, it reads the full `SKILL.md` from its filesystem or provider resource. Explicit `$skill` mentions remain a separate path that injects the full skill instructions. For orchestrator skills, `skills.list` provides bounded discovery metadata before `skills.read` returns the full selected resource. ## Test plan - `just test -p codex-core-skills` - `just test -p codex-skills-extension` - `just test -p codex-external-agent-migration` The focused regressions verify that overlong metadata is preserved at load and migration boundaries while default available-skills rendering and `skills.list` output produce the 1021-character prefix plus `...`.
This commit is contained in:
committed by
GitHub
Unverified
parent
abd901770e
commit
64bdeed9f7
@@ -18,7 +18,6 @@ const EXTERNAL_AGENT_HOOKS_SUBDIR: &str = "hooks";
|
||||
const EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR: &str = "hooks";
|
||||
const COMMAND_SKILL_PREFIX: &str = "source-command";
|
||||
const MAX_SKILL_NAME_LEN: usize = 64;
|
||||
const MAX_SKILL_DESCRIPTION_LEN: usize = 1024;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ParsedDocument {
|
||||
@@ -1130,14 +1129,11 @@ fn command_skill_name_if_supported(
|
||||
return None;
|
||||
}
|
||||
let source_name = command_source_name(source_commands, source_file);
|
||||
let description = command_skill_description(document, &source_name)?;
|
||||
command_skill_description(document, &source_name)?;
|
||||
let name = command_skill_name(source_commands, source_file);
|
||||
if name.chars().count() > MAX_SKILL_NAME_LEN {
|
||||
return None;
|
||||
}
|
||||
if description.chars().count() > MAX_SKILL_DESCRIPTION_LEN {
|
||||
return None;
|
||||
}
|
||||
if has_unsupported_command_template_features(&document.body) {
|
||||
return None;
|
||||
}
|
||||
@@ -1388,6 +1384,8 @@ mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
const MAX_SKILL_DESCRIPTION_LEN: usize = 1024;
|
||||
|
||||
fn source_path(relative_path: &str) -> PathBuf {
|
||||
Path::new("/repo")
|
||||
.join(external_agent_config_dir())
|
||||
@@ -1722,6 +1720,35 @@ command = "enabled-server"
|
||||
assert!(command_skill_name_if_supported(&root, &file, &document).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn commands_with_overlong_descriptions_are_preserved() {
|
||||
let root = source_path("commands");
|
||||
let file = source_path("commands/review.md");
|
||||
let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1);
|
||||
let document =
|
||||
parse_document_content(&format!("---\ndescription: {description}\n---\nReview\n"));
|
||||
|
||||
assert_eq!(
|
||||
command_skill_name_if_supported(&root, &file, &document),
|
||||
Some("source-command-review".to_string())
|
||||
);
|
||||
|
||||
let rendered = render_command_skill(
|
||||
&document.body,
|
||||
"source-command-review",
|
||||
&description,
|
||||
"review",
|
||||
);
|
||||
let rendered_document = parse_document_content(&rendered);
|
||||
assert_eq!(
|
||||
rendered_document
|
||||
.frontmatter
|
||||
.get("description")
|
||||
.and_then(FrontmatterValue::as_scalar),
|
||||
Some(description.as_str())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn commands_with_provider_runtime_expansion_are_skipped() {
|
||||
let root = source_path("commands");
|
||||
|
||||
Reference in New Issue
Block a user