Skip to content

Conversation

@27Bslash6
Copy link

Problem

Issue #3 identified critical validation bugs in src/sequentialthinking/lib.ts:

  1. Falsy checks reject valid values: Lines 30-35 used !data.thoughtNumber which incorrectly rejects 0
  2. No bounds checking: Negative numbers, NaN, and Infinity passed validation
  3. Unsafe type assertions: Lines 45-49 used as boolean | undefined without runtime validation
  4. Empty strings allowed: Falsy check on thought allowed empty strings to pass

Solution

Replaced unsafe falsy checks with explicit validation:

Required Fields

  • thought: Check typeof === 'string' AND length > 0
  • thoughtNumber & totalThoughts:
    • Type check: typeof === 'number'
    • Finite check: Number.isFinite() (rejects NaN/Infinity)
    • Bounds check: >= 1 (rejects negatives and zero)
  • nextThoughtNeeded: Type check only (boolean)

Optional Fields

  • Only validate when !== undefined
  • isRevision, needsMoreThoughts: Boolean type check
  • revisesThought, branchFromThought: Number type + finite + bounds
  • branchId: String type + non-empty check

Test Coverage

Added 12 new edge case tests:

  • ✅ Negative numbers (thoughtNumber, totalThoughts, revisesThought)
  • ✅ Zero values (thoughtNumber, totalThoughts)
  • ✅ NaN and Infinity
  • ✅ Wrong types for optional fields (string instead of number, etc.)
  • ✅ Empty strings for branchId

All 37 tests pass
Coverage: 94.59% statements, 91.8% branches

Why This First?

This PR establishes the foundation. Validation must be bulletproof before tackling:

Garbage in, garbage out. Fix validation first.

Files Changed

  • src/sequentialthinking/lib.ts (validateThoughtData method)
  • src/sequentialthinking/__tests__/lib.test.ts (12 new tests)

Testing

cd src/sequentialthinking
npm ci
npm test    # All 37 tests pass
npm run build  # TypeScript compiles clean

Closes #3

Fixes #3

Problem:
- Falsy checks (!data.thoughtNumber) rejected valid 0 values
- No bounds checking for negative numbers, NaN, Infinity
- Unsafe type assertions on optional fields without validation
- Empty strings passed validation

Solution:
- Replace falsy checks with explicit typeof and bounds validation
- Add Number.isFinite() checks to reject NaN and Infinity
- Add explicit >= 1 checks for numeric fields
- Validate optional fields only when present (undefined check)
- Add length check for string fields

Tests:
- Added 12 new tests covering edge cases
- All 37 tests pass
- Coverage: 94.59% statements, 91.8% branches

This is the foundation for subsequent fixes - validation must be
rock-solid before addressing concurrency and state management.
@27Bslash6 27Bslash6 closed this Nov 15, 2025
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.

1 participant