refactor(serve): simplify parsePositiveInt (drop type-blind catch + redundant trim)#80
Merged
Merged
Conversation
…redundant trim
Review findings F4/F5 (no behavior change).
parsePositiveInt routed a pre-trimmed string through parseBoundedInt inside a
try/catch, then re-framed ANY thrown error as "must be a positive integer". Two
smells:
- F4: the bare `catch {}` was type-blind — a future non-validation throw from
parseBoundedInt would be silently relabeled, hiding the real fault.
- F5: parseBoundedInt re-trimmed the already-trimmed string.
Resolve directly through the shared parseDecimalDigits primitive (trims once,
caps at MAX_SAFE_INTEGER) and range-check `>= 1` inline. No exception is used for
control flow, so there's no catch to mask anything, and the value is trimmed
without a round-trip. Error code/message and all accept/reject behavior are
identical (parseDecimalDigits already returns null past MAX_SAFE_INTEGER, so the
old upper-bound check is preserved). Suite 499 pass / 0 fail, check clean.
Claude-Session: https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad
This was referenced Jun 23, 2026
abpai
added a commit
that referenced
this pull request
Jun 24, 2026
The serve-hardening and CLI work since v0.7.0 (PRs #76, #77, #78, #79) merged without changesets, so the Release workflow had nothing to consume and the package is still published at 0.7.0. Add the missing changesets so the next release captures this work. - serve-api-webhook-hardening (patch): #76 - envelope webhook-route errors plus tunnel / Postgres-receipt / broadcast / base-path fixes (10 defects). - webhook-secret-fail-open (patch): #77 - single WEBHOOK_SECRET_ENV source of truth for the tunnel gate + stricter KANBAN_SYNC_INTERVAL_MS parsing. - honor-default-task-column (patch): #78 - SQLite createTask honors the configured default task column, with first-column fallback parity with Postgres. - strict-jira-board-id (minor): #79 - a malformed JIRA_BOARD_ID now throws INVALID_CONFIG instead of being silently misparsed (behavior change). Net bump: minor -> 0.8.0. #80 (pure internal refactor) and the README link fix are intentionally omitted as they have no user-facing change. Claude-Session: https://claude.ai/code/session_01X9j5Rs6kXK8BuHguhy3x33
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
1 participant
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.
Cleanup of review findings F4 and F5 from the #77/#79 review. No behavior change — pure simplification.
parsePositiveIntrouted a pre-trimmed string throughparseBoundedIntinside atry/catch, then re-framed any thrown error as "must be a positive integer":catch {}was type-blind: a future non-validation throw fromparseBoundedIntwould be silently relabeled, hiding the real fault (exception-as-control-flow).parseBoundedIntre-trimmed the already-trimmed string.Now resolves directly through the shared
parseDecimalDigitsprimitive (trims once, caps atMAX_SAFE_INTEGER) with an inline>= 1check. Notry/catch, so there's no catch to mask anything, and no re-trim round-trip.Behavior preserved
INVALID_ARGUMENT) and message (<field> must be a positive integer (received '<value>')) unchanged.parseDecimalDigitsalready returnsnullpastMAX_SAFE_INTEGER, so the old upper-bound check is preserved; absent/blank →undefined;<= 0, decimals, signs, hex/scientific all rejected.Verification
bun run checkclean; suite 499 pass / 27 skip / 0 fail (unchanged — pure refactor, existingparsePositiveInttests lock the behavior).https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad