Skip to content

fix: align description fallback pattern across commands, subagents and rules - #1205

Closed
saitota wants to merge 1 commit into
dyoshikawa:mainfrom
saitota:ii1078
Closed

fix: align description fallback pattern across commands, subagents and rules#1205
saitota wants to merge 1 commit into
dyoshikawa:mainfrom
saitota:ii1078

Conversation

@saitota

@saitota saitota commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove description ?? "" and description || "" fallback patterns in commands, subagents and rules to propagate undefined instead of empty string
  • Make description field optional (z.optional(z.string())) across all command/subagent schemas for consistency with RulesyncRuleFrontmatterSchema
  • Improve cursor-rule.ts truthiness check to use explicit !== undefined && !== null && !== "" guard instead of falsy check
  • Add deepRemoveNullishObject edge case test for nested objects with all-null values

Closes #1078

Note

This PR addresses all occurrences of the description ?? "" / description || "" pattern found across the codebase, not just the two files mentioned in the issue #1078 .

References

Test plan

  • pnpm check
  • pnpm test (4098 passed)

@dyoshikawa-claw

Copy link
Copy Markdown
Collaborator

/opencode review

1 similar comment
@dyoshikawa-claw

Copy link
Copy Markdown
Collaborator

/opencode review

@github-actions

Copy link
Copy Markdown
Contributor

PR Review Summary (PR addressing issue #1078)

Code Review Findings

  1. Overall Assessment: ✅ Good Quality PR - The PR successfully aligns the description field handling across commands, subagents, and rules. All CI checks pass (4098 tests).

  2. Inconsistency in kiro-subagent.ts ⚠️ Minor Issue - description: description ?? undefined is unnecessarily verbose. Should be just description: description for consistency.

  3. Deletion Placeholder Values - Acceptable ✅ - Empty strings in simulated-command.ts and simulated-subagent.ts are acceptable since validate: false bypasses schema validation for deletion scenarios.

  4. cursor-rule.ts Truthiness Check - Good Improvement ✅ - The explicit !== undefined && !== null && !== "" guard is clearer than the previous falsy check.

  5. Test Coverage - Comprehensive ✅ - All affected tests updated, new edge case test added for deepRemoveNullishObject.

  6. Schema Consistency - Complete ✅ - All command/subagent schemas now use z.optional(z.string()) matching RulesyncRuleFrontmatterSchema.

  7. No Missed Files ✅ - No remaining description ?? "" or description || "" patterns exist.

Security Review Findings

  1. No New Security Vulnerabilities ✅ - Zod schema validation ensures description must be a string when present, maintaining type safety.

  2. Safe Fallback Pattern Removal ✅ - undefined values properly handled by deepRemoveNullishObject() before YAML serialization.

  3. Pre-existing YAML Concern in cursor-rule.ts ⚠️ (Not new to this PR) - Manual YAML building in stringifyCursorFrontmatter() lacks proper escaping. Low severity since it's a local CLI tool.

  4. No Command Injection Risk ✅ - Description values only used for file output and internal data structures.

  5. Input Validation Adequate ✅ - z.optional(z.string()) properly validates input.

Recommendation

Ready to merge with one minor optional fix: remove redundant ?? undefined in kiro-subagent.ts for consistency.

github run

@saitota saitota closed this by deleting the head repository Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: align description fallback pattern across commands and subagents

2 participants