Repository navigation
fix: numeric skill names, ZWSP agent lookups, ultrawork run_in_background - #3436
Merged
Merged
Conversation
…gent lookups - #3354: Coerce data.name to String in loadSkillFromPath/loadSkillFromPathAsync to prevent crash when YAML parses numeric skill names (e.g., name: 12306) - #3416: Add required run_in_background parameter to all task() examples in ultrawork prompts (default, gpt, gemini, planner) to match tool schema - #3379/#3417/#3418/#3337/#3335: Strip ZWSP (U+200B) before agent name comparisons in agent-tool-restrictions, sync-prompt-sender, tool-execute-after, tool-execute-before, oracle-verification-detector, call-omo-agent, recovery-prompt-config, and agent-variant to prevent ZWSP-prefixed display names from breaking exact-match lookups
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 14 files
Confidence score: 4/5
- This PR looks generally safe to merge, with moderate confidence: both findings are medium severity (5/10) and limited to falsy numeric name handling rather than broad runtime stability issues.
- The most impactful issue is in
src/features/opencode-skill-loader/async-loader.ts, where||before coercion causes a valid numeric skill name like0to be replaced bydefaultName, which can mislabel user-defined skills. - A similar edge-case regression exists in
src/features/opencode-skill-loader/loaded-skill-from-path.ts, where frontmattername: 0is dropped fordefaultNamedue to the same||-before-String()pattern. - Pay close attention to
src/features/opencode-skill-loader/async-loader.ts,src/features/opencode-skill-loader/loaded-skill-from-path.ts- preserve falsy numeric names by coercing before fallback logic.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/opencode-skill-loader/async-loader.ts">
<violation number="1" location="src/features/opencode-skill-loader/async-loader.ts:90">
P2: Falsy numeric skill names (e.g. `0`) still fall back to `defaultName` because the `||` check happens before string coercion.</violation>
</file>
<file name="src/features/opencode-skill-loader/loaded-skill-from-path.ts">
<violation number="1" location="src/features/opencode-skill-loader/loaded-skill-from-path.ts:28">
P2: Falsy frontmatter names are still dropped because `||` runs before `String()`, so `name: 0` falls back to `defaultName` instead of being preserved as "0".</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const mcpConfig = mcpJsonMcp || frontmatterMcp | ||
|
|
||
| const baseName = data.name || defaultName | ||
| const baseName = String(data.name || defaultName) |
There was a problem hiding this comment.
P2: Falsy numeric skill names (e.g. 0) still fall back to defaultName because the || check happens before string coercion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/opencode-skill-loader/async-loader.ts, line 90:
<comment>Falsy numeric skill names (e.g. `0`) still fall back to `defaultName` because the `||` check happens before string coercion.</comment>
<file context>
@@ -87,7 +87,7 @@ export async function loadSkillFromPathAsync(
const mcpConfig = mcpJsonMcp || frontmatterMcp
- const baseName = data.name || defaultName
+ const baseName = String(data.name || defaultName)
const skillName = namePrefix ? `${namePrefix}/${baseName}` : baseName
const originalDescription = data.description || ""
</file context>
| const mcpConfig = mcpJsonMcp || frontmatterMcp | ||
|
|
||
| const baseName = data.name || options.defaultName | ||
| const baseName = String(data.name || options.defaultName) |
There was a problem hiding this comment.
P2: Falsy frontmatter names are still dropped because || runs before String(), so name: 0 falls back to defaultName instead of being preserved as "0".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/opencode-skill-loader/loaded-skill-from-path.ts, line 28:
<comment>Falsy frontmatter names are still dropped because `||` runs before `String()`, so `name: 0` falls back to `defaultName` instead of being preserved as "0".</comment>
<file context>
@@ -25,7 +25,7 @@ export async function loadSkillFromPath(options: {
const mcpConfig = mcpJsonMcp || frontmatterMcp
- const baseName = data.name || options.defaultName
+ const baseName = String(data.name || options.defaultName)
const skillName = namePrefix ? `${namePrefix}/${baseName}` : baseName
const originalDescription = data.description || ""
</file context>
Suggested change
| const baseName = String(data.name || options.defaultName) | |
| const baseName = String(data.name ?? options.defaultName) |
This was referenced Apr 15, 2026
This was referenced Apr 16, 2026
fix(agents): replace deprecated getAgentListDisplayName with getAgentDisplayName (fixes #3281)
#3407
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug Fixes
#3354: Numeric skill name crashes ALL slash commands
String()coercion inloadSkillFromPath()andloadSkillFromPathAsync()name: 12306, js-yaml parses as number, causingcmd.name.toLowerCase()crash#3379 / #3417 / #3418 / #3337 / #3335: ZWSP prefix breaks agent name lookups
stripInvisibleAgentCharacters()calls across 8 files:agent-tool-restrictions.ts- agent restriction lookupsync-prompt-sender.ts- oracle agent detectiontool-execute-after.ts- verification context checktool-execute-before.ts- subagent type normalizationoracle-verification-detector.ts- oracle agent matchingcall-omo-agent/tools.ts- agent validationrecovery-prompt-config.ts- agent comparisonagent-variant.ts- variant resolutionresolveKnownAgentConfigKey()to handle parenthetical suffixes#3416: ultrawork prompts missing
run_in_backgroundrun_in_background: true/falseto all task() examples in:ultrawork/default.tsultrawork/gemini.tsultrawork/gpt.tsultrawork/planner.tsAlready fixed in dev (verified):
isPlanFamily()already uses exact matchreview-workgetAgentConfigKeyVerification:
bun tsc --noEmitclean, 5451 tests passing (0 failures)Closes #3354, closes #3379, closes #3417, closes #3418, closes #3337, closes #3335, closes #3416
Summary by cubic
Fixes crashes from numeric skill names, makes agent name lookups robust to zero‑width spaces, and updates ultrawork
task()examples to include the requiredrun_in_backgroundflag. This prevents command failures, ensures correct agent resolution, and aligns examples with the tool schema.nametoStringinloadSkillFromPath()andloadSkillFromPathAsync()to handle YAML numeric names.call_omo_agent, recovery prompts, and variant resolution.run_in_backgroundto all ultraworktask()examples (default, gpt, gemini, planner) to match the schema and clarify foreground vs background runs.Written for commit 62c60ae. Summary will update on new commits.