From a652dbec4c26d1bc551ce721ac759424b1c28fd7 Mon Sep 17 00:00:00 2001 From: bradAGI <46579244+bradAGI@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:09:46 -0400 Subject: [PATCH] test(rules): mirror and cover OAI-027, OAI-028 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Engine half of a coordinated pair with trustabl/trustabl-rules#92, on a branch of the same name so the rules-sync job resolves the matching pack rather than main. Neither half should merge alone — check-rules-sync.sh fails if they do. Mirrors openai_sdk/tool_definition.yaml into testdata/rules-fixture and adds cases to policyRuleCases, as TestPolicyRules_AllRulesCovered requires. Five cases rather than four. OAI-028 pairs description_length_lt with has_docstring so an absent description stays OAI-022's finding instead of double-reporting, and the fifth case pins that guard. Note for sequencing: rules#87 (OAI-025/026, the Python pair) appends to the same file, so whichever of the two pairs lands second needs a trivial rebase in both repos — the rule blocks and the test cases are independent and the resolution is "keep both". --- internal/rules/policies_test.go | 33 +++++++++- .../openai_sdk/tool_definition.yaml | 61 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/internal/rules/policies_test.go b/internal/rules/policies_test.go index 65af6c0a..6ff5b898 100644 --- a/internal/rules/policies_test.go +++ b/internal/rules/policies_test.go @@ -1986,6 +1986,38 @@ def fetch_data(x: str) -> dict: return {} `, toolConfig: nil, wantFires: false}, + + // ─── OAI-027 / OAI-028: TS OpenAI Agents description quality ──────────── + { + name: "OAI-027 fires on placeholder description", ruleID: "OAI-027", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: true, + src: "import { tool } from \"@openai/agents\";\n" + + "export const t = tool({ name: \"lookup_order\", description: \"TODO: describe this tool.\", parameters: {}, execute: async () => \"x\" });\n", + }, + { + name: "OAI-027 silent on a real description", ruleID: "OAI-027", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\n" + + "export const t = tool({ name: \"lookup_order\", description: \"Look up one order by its number and return its fulfillment status.\", parameters: {}, execute: async () => \"x\" });\n", + }, + { + name: "OAI-028 fires on a too-short description", ruleID: "OAI-028", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: true, + src: "import { tool } from \"@openai/agents\";\n" + + "export const t = tool({ name: \"list_orders\", description: \"Gets data.\", parameters: {}, execute: async () => \"x\" });\n", + }, + { + name: "OAI-028 silent on a full description", ruleID: "OAI-028", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\n" + + "export const t = tool({ name: \"list_orders\", description: \"List every order belonging to one customer, most recent first.\", parameters: {}, execute: async () => \"x\" });\n", + }, + { + name: "OAI-028 silent when the description is absent (OAI-022's case)", ruleID: "OAI-028", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\n" + + "export const t = tool({ name: \"list_orders\", parameters: {}, execute: async () => \"x\" });\n", + }, } // policyRepoRuleCases covers repo-scoped rules. @@ -2246,7 +2278,6 @@ var policyRepoRuleCases = []policyRepoCase{ }, models.RepoInventory{SDKsDetected: []models.SDK{models.SDKOpenAIAgents}}, false}, - } // optionsWithPermissionMode builds a ClaudeAgentOptionsDef whose captured diff --git a/testdata/rules-fixture/openai_sdk/tool_definition.yaml b/testdata/rules-fixture/openai_sdk/tool_definition.yaml index 7ef835b6..5471c5d4 100644 --- a/testdata/rules-fixture/openai_sdk/tool_definition.yaml +++ b/testdata/rules-fixture/openai_sdk/tool_definition.yaml @@ -95,3 +95,64 @@ rules: Provide a concise `description` string in the `tool({...})` options stating what the tool does and when the model should call it. The description is the model's primary routing signal alongside the tool name. + + - id: OAI-027 + title: TypeScript tool description is a placeholder + severity: low + confidence: 0.85 + language: typescript + applies_to: + - openai_tool + scope: tool + match: + has_description_text: + - todo + - tbd + - fixme + - placeholder + - no description + - does stuff + explanation: > + The tool sets a description, so it passes OAI-022, but the string is a + placeholder rather than real content. That leaves the tool in exactly the + state OAI-022 exists to prevent: the description is the model's primary + routing signal alongside the name, and "TODO: describe this tool" carries + no signal the name did not already. The Zod parameters schema does not + compensate — it constrains the shape of the arguments once the model has + decided to call this tool, never whether that decision was right, so a + placeholder description yields a well-formed call to the wrong tool. With + handoffs the reach is wider still: an agent picks between its own tools + and its peers' from these strings, so the stub can route the conversation + to the wrong agent rather than merely the wrong function. + fix: > + Replace the placeholder with a real description covering what the tool + does, what it returns, and when the model should call it rather than a + neighboring tool. The SDK passes the string to the model verbatim, so + write it for the model rather than a human maintainer. + + - id: OAI-028 + title: TypeScript tool description is too short to guide model selection + severity: low + confidence: 0.8 + language: typescript + applies_to: + - openai_tool + scope: tool + match: + all: + - has_docstring: true + - description_length_lt: 40 + explanation: > + A description under 40 characters is rarely enough to convey what a tool + does, what it returns, and when to call it rather than a similarly named + neighbor. It is the model's primary routing signal alongside the tool + name, so a stub like "Gets data." leaves scope and preconditions to + guesswork. Each resulting mis-selection also spends a turn against the + agent's max turns budget, and nothing in a trace attributes the wasted + turn to the description that caused it — the run just looks slow or + confused. + fix: > + Expand the description to at least a full sentence covering inputs, + outputs, and the situation in which this tool should be used over the + alternatives. Where two tools are easy to confuse, say in each which one + the other case belongs to.