Fix/create testcase case type

[SavioBS629]

No description provided.

[SavioBS629]

Claude Code Review (automated) — 1 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[SavioBS629]

[Low] .describe() exceeds the 120-char ceiling

This description is ~205 chars; tool-design.md sets a 120-char hard ceiling (target 60). It also encodes a defaulting rule in prose and lists internal names the handler's normalizeDefaultFieldValue already maps. It loads into every client's context on connect. Consistent with the already-over-budget priority field above, but that isn't a reason to add another.

Suggestion: Trim to ~60 chars, e.g. "Test case type display or internal name (per-project). Omit for project default." — and ideally trim priority in the same pass.

Reviewer: stack:code-reviewer

[SavioBS629]

Claude Code PR Review

PR: #346 • Head: ad9d1a1 • Reviewers: stack:code-reviewer

Summary

Fixes a customer bug where createTestCase silently dropped case_type (the parameter was never in the schema). Adds case_type to CreateTestCaseSchema, generalizes normalizePriority → normalizeDefaultFields to normalize both priority and case_type in a single form-fields lookup, and adds four tests.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No secrets; auth unchanged.
High	Security	Authentication/authorization checks present	Pass	Uses getBrowserStackAuth(config); untouched.
High	Security	Input validation and sanitization	Pass	New field validated by Zod z.string().optional().
High	Security	No IDOR — resource ownership validated	N/A	No ownership surface changed.
High	Security	No SQL injection (parameterized queries)	N/A	No SQL; REST via apiClient.
High	Correctness	Logic is correct, handles edge cases	Pass	Normalizes on display+internal name; graceful fallback on lookup failure.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	catch logs + passes raw value through, matching prior priority behavior.
High	Correctness	No race conditions or concurrency issues	Pass	All state function-scoped; no shared/global state.
Medium	Testing	New code has corresponding tests	Pass	4 new tests: normalize, omit, fallback, combined single-lookup.
Medium	Testing	Error paths and edge cases tested	Pass	Lookup-failure fallback covered.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Build gate green locally (224 tests).
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Single form-fields lookup for both fields (was already 1 for priority).
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	Mirrors the established priority path and updateTestCase.
Medium	Quality	Changes are focused (single concern)	Pass	Single concern: case_type parity on create.
Low	Quality	Meaningful names, no dead code	Pass	Helper renamed to reflect widened responsibility.
Low	Quality	Comments explain why, not what	Pass	Doc comment explains the "reject lowercase" rationale.
Low	Quality	No unnecessary dependencies added	Pass	No new deps.

Findings

• File: src/tools/testmanagement-utils/create-testcase.ts:165

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The case_type .describe() is ~205 chars, over the tool-design.md hard ceiling of 120 (target 60). It also encodes a defaulting rule in prose ("If omitted, the project default…") and teaches internal names the handler's normalizeDefaultFieldValue already maps. This string loads into every client's context on every connect. Consistent with the already-over-budget priority field, but that isn't a reason to add another.

• Suggestion: Trim to ~60 chars, e.g. "Test case type display or internal name (per-project). Omit for project default." — and ideally trim priority in the same pass.

• File: src/tools/testmanagement-utils/create-testcase.ts:211

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The new normalizeDefaultFields duplicates a same-named helper in update-testcase.ts:128. They now differ (update also handles automation_status and takes the full update request), so they can drift.

• Suggestion: Follow-up (non-blocking): extract a shared helper in TCG-utils/api.ts taking default_fields + a field spec. They already share the normalizeDefaultFieldValue primitive, so this is low-risk cleanup, not a merge blocker.

• File: tests/tools/testmanagement.test.ts:1167

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: New tests exercise only the v2 create path. When template_id is set, the v1 path spreads testCaseParams, so normalized case_type flows into the v1 body too — untested. If v1 keys case_type differently, that path could silently misbehave.

• Suggestion: Add a one-line test asserting case_type survives into the v1 body when template_id is set.

Raised by other reviewers (not independently confirmed)

• PR-description hygiene (non-code): the reviewer recommends the PR body capture the "customer bug + parity with updateTestCase" justification for the new optional field, and link the relevant BrowserStack docs page for the v2/v1 create contract so the case_type field and default_fields.case_type.values shape can be confirmed. Non-gating.

---

Verdict: PASS — small, well-tested, pattern-consistent fix; no blocking issues. Findings are Low-severity cleanups (trim descriptions, dedupe helper, add a v1-path test) suitable as follow-ups.