From d2f8f64574654f173b0d6b26936104cfff2a8c48 Mon Sep 17 00:00:00 2001 From: Letiancheng Lee <130898955+PeterTianbuhan@users.noreply.github.com> Date: Sat, 30 May 2026 20:49:49 -0700 Subject: [PATCH 1/2] fix codex agent extension validation --- extensions/README.md | 2 + extensions/codex-agents/convert.js | 20 +++++-- extensions/codex-agents/md-toml.js | 3 + internal/sync/extension_test.go | 72 +++++++++++++++++++++++ website/docs/reference/commands/extras.md | 2 +- 5 files changed, 92 insertions(+), 7 deletions(-) diff --git a/extensions/README.md b/extensions/README.md index 42b08d72..1313c614 100644 --- a/extensions/README.md +++ b/extensions/README.md @@ -89,6 +89,8 @@ convert(({ body, frontmatter, stem }) => ({ })); ``` +For targets with required fields, validate them in `convert.js` and exit non-zero with a clear message. For example, Codex TOML agents require `description`, so `extensions/codex-agents` rejects inputs without `description` frontmatter instead of generating a TOML file that Codex cannot load. + Plain values become TOML strings: ```toml diff --git a/extensions/codex-agents/convert.js b/extensions/codex-agents/convert.js index 4253dfe0..a609856b 100644 --- a/extensions/codex-agents/convert.js +++ b/extensions/codex-agents/convert.js @@ -1,9 +1,17 @@ #!/usr/bin/env node const { block, convert } = require("./md-toml"); -convert(({ body, frontmatter, stem }) => ({ - name: frontmatter.name || stem, - description: frontmatter.description, - model: frontmatter.model, - developer_instructions: block(body), -})); +convert(({ body, frontmatter, stem }) => { + if (!frontmatter.description) { + throw new Error( + "codex-agents: missing required frontmatter 'description' (Codex custom agents require description)" + ); + } + + return { + name: frontmatter.name || stem, + description: frontmatter.description, + model: frontmatter.model, + developer_instructions: block(body), + }; +}); diff --git a/extensions/codex-agents/md-toml.js b/extensions/codex-agents/md-toml.js index d3e77b2b..5b27b143 100644 --- a/extensions/codex-agents/md-toml.js +++ b/extensions/codex-agents/md-toml.js @@ -20,6 +20,9 @@ function convert(mapFields) { } process.stdout.write(lines.join("\n") + (lines.length ? "\n" : "")); + }).catch((err) => { + process.stderr.write((err && err.message ? err.message : String(err)) + "\n"); + process.exit(1); }); } diff --git a/internal/sync/extension_test.go b/internal/sync/extension_test.go index 9802c4c9..6b87b1bd 100644 --- a/internal/sync/extension_test.go +++ b/internal/sync/extension_test.go @@ -2,6 +2,7 @@ package sync import ( "os" + "os/exec" "path/filepath" "strings" "testing" @@ -439,3 +440,74 @@ func TestSyncExtra_TransformRejectsNonCopyMode(t *testing.T) { } } } + +func TestBundledCodexAgentExtension_TransformsRequiredFields(t *testing.T) { + node := requireNode(t) + root := testRepoRoot(t) + + cmd := exec.Command(node, filepath.Join(root, "extensions", "codex-agents", "convert.js")) + cmd.Env = append(os.Environ(), "SS_REL_PATH=reviewer.md") + cmd.Stdin = strings.NewReader("---\nname: reviewer\ndescription: Review PRs for correctness\nmodel: gpt-5.4\n---\nReview code like an owner.\n") + + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("codex-agents convert failed: %v\n%s", err, out) + } + + got := string(out) + for _, want := range []string{ + `name = "reviewer"`, + `description = "Review PRs for correctness"`, + `model = "gpt-5.4"`, + `developer_instructions = """`, + `Review code like an owner.`, + } { + if !strings.Contains(got, want) { + t.Errorf("converted output missing %q:\n%s", want, got) + } + } +} + +func TestBundledCodexAgentExtension_RequiresDescription(t *testing.T) { + node := requireNode(t) + root := testRepoRoot(t) + + cmd := exec.Command(node, filepath.Join(root, "extensions", "codex-agents", "convert.js")) + cmd.Env = append(os.Environ(), "SS_REL_PATH=reviewer.md") + cmd.Stdin = strings.NewReader("# Reviewer\nReview code like an owner.\n") + + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("expected missing description to fail, got success:\n%s", out) + } + if !strings.Contains(string(out), "missing required frontmatter 'description'") { + t.Fatalf("expected missing description error, got:\n%s", out) + } +} + +func requireNode(t *testing.T) string { + t.Helper() + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node not available; skipping bundled JavaScript extension check") + } + return node +} + +func testRepoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("could not find repo root") + } + dir = parent + } +} diff --git a/website/docs/reference/commands/extras.md b/website/docs/reference/commands/extras.md index 70b368a3..5320ef59 100644 --- a/website/docs/reference/commands/extras.md +++ b/website/docs/reference/commands/extras.md @@ -338,7 +338,7 @@ extras: extension: codex-agents ``` -`skillshare sync extras` converts each `.md` into `~/.codex/agents/.toml`, mapping frontmatter `name`, `description`, and `model` and folding the markdown body into `developer_instructions` (other frontmatter keys are dropped). No separate copy of the agents is needed. +`skillshare sync extras` converts each `.md` into `~/.codex/agents/.toml`, mapping frontmatter `name`, `description`, and `model` and folding the markdown body into `developer_instructions` (other frontmatter keys are dropped). Codex requires `description`, so the reference transform reports a clear error when that frontmatter field is missing. No separate copy of the agents is needed. --- From 56f97f9a3d38c2303f70abfb80bfa63ca1605f7e Mon Sep 17 00:00:00 2001 From: Letiancheng Lee <130898955+PeterTianbuhan@users.noreply.github.com> Date: Sat, 30 May 2026 20:53:28 -0700 Subject: [PATCH 2/2] reject blank codex agent descriptions --- extensions/codex-agents/convert.js | 2 +- internal/sync/extension_test.go | 34 ++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/extensions/codex-agents/convert.js b/extensions/codex-agents/convert.js index a609856b..5ee114c4 100644 --- a/extensions/codex-agents/convert.js +++ b/extensions/codex-agents/convert.js @@ -2,7 +2,7 @@ const { block, convert } = require("./md-toml"); convert(({ body, frontmatter, stem }) => { - if (!frontmatter.description) { + if (!frontmatter.description || !frontmatter.description.trim()) { throw new Error( "codex-agents: missing required frontmatter 'description' (Codex custom agents require description)" ); diff --git a/internal/sync/extension_test.go b/internal/sync/extension_test.go index 6b87b1bd..d6fd5bc2 100644 --- a/internal/sync/extension_test.go +++ b/internal/sync/extension_test.go @@ -472,16 +472,32 @@ func TestBundledCodexAgentExtension_RequiresDescription(t *testing.T) { node := requireNode(t) root := testRepoRoot(t) - cmd := exec.Command(node, filepath.Join(root, "extensions", "codex-agents", "convert.js")) - cmd.Env = append(os.Environ(), "SS_REL_PATH=reviewer.md") - cmd.Stdin = strings.NewReader("# Reviewer\nReview code like an owner.\n") + for _, tc := range []struct { + name string + input string + }{ + { + name: "missing", + input: "# Reviewer\nReview code like an owner.\n", + }, + { + name: "whitespace-only", + input: "---\ndescription: \" \"\n---\nReview code like an owner.\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + cmd := exec.Command(node, filepath.Join(root, "extensions", "codex-agents", "convert.js")) + cmd.Env = append(os.Environ(), "SS_REL_PATH=reviewer.md") + cmd.Stdin = strings.NewReader(tc.input) - out, err := cmd.CombinedOutput() - if err == nil { - t.Fatalf("expected missing description to fail, got success:\n%s", out) - } - if !strings.Contains(string(out), "missing required frontmatter 'description'") { - t.Fatalf("expected missing description error, got:\n%s", out) + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("expected missing description to fail, got success:\n%s", out) + } + if !strings.Contains(string(out), "missing required frontmatter 'description'") { + t.Fatalf("expected missing description error, got:\n%s", out) + } + }) } }