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.

[SavioBS629]

Claude Code PR Review

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

Continues the previous review — changes since ad9d1a1 (FULL re-review).

Summary

Follow-up commit f084848 addresses all three Low findings from the prior review: extracts the default-field normalization into a shared normalizeDefaultFields(defaultFields, inputs) in TCG-utils/api.ts (used by both createTestCase and updateTestCase), trims the case_type .describe() under the char budget, and adds a v1-path test asserting normalized case_type reaches the /api/v1 create body when template_id is set.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None.
High	Security	Authentication/authorization checks present	Pass	getBrowserStackAuth(config) throughout; no process.env reads.
High	Security	Input validation and sanitization	Pass	Zod-validated inputs; normalization is pure.
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	Shared helper preserves automation_status quirk byte-for-byte; graceful fallback intact.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Each caller keeps its own fetch + catch/fallback.
High	Correctness	No race conditions or concurrency issues	Pass	No new shared mutable state; helper is pure.
Medium	Testing	New code has corresponding tests	Pass	5 new create tests incl. v1-path; existing update tests still green.
Medium	Testing	Error paths and edge cases tested	Pass	Lookup-failure fallback covered for both fields.
Medium	Testing	Existing tests still pass (no regressions)	Pass	225 tests pass; tsc clean.
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Single form-fields lookup for both fields.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	Centralizes duplicated logic; callers keep endpoint-specific wrappers.
Medium	Quality	Changes are focused (single concern)	Pass	Refactor + prompt trim + test, all scoped to the case_type fix.
Low	Quality	Meaningful names, no dead code	Pass	Unused normalizeMock removed; helper named to reflect scope.
Low	Quality	Comments explain why, not what	Pass	Shared helper documents the automation_status quirk.
Low	Quality	No unnecessary dependencies added	Pass	No new deps.

Findings

All three findings from the prior review are resolved by f084848:

• src/tools/testmanagement-utils/create-testcase.ts:165 Low — .describe() exceeds 120-char ceiling Resolved — trimmed to 80 chars.
• src/tools/testmanagement-utils/create-testcase.ts:211 Low — normalizeDefaultFields duplicated across create/update Resolved — extracted to shared TCG-utils/api.ts helper; automation_status behavior preserved exactly.
• tests/tools/testmanagement.test.ts:1167 Low — v1 create path case_type untested Resolved — new test asserts normalized case_type reaches the /api/v1 body when template_id is set.

No new issues introduced. The reviewer verified: automation_status parity in the shared helper, no drift from the importActual-spread test mock (all tests pass), the trimmed prompt is under budget, and create's priority normalization is equivalent through the shared path.

---

Verdict: PASS — all prior findings resolved, no new issues; instrumentation, error format, auth, and multi-tenant safety clean.