Apply parseMaxMessages pattern to limit and other numeric option args

[kiki830621]

Problem

From verification of #8/#11/#12 by devils-advocate (MEDIUM):
「一致性問題 — Server.swift 其他 numeric option args 仍有同款 silent fallback：

374: let limit = args["limit"]?.intValue ?? 50
387: let limit = args["limit"]?.intValue ?? 20
446: let limit = args["limit"]?.intValue ?? 50
454: let limit = args["limit"]?.intValue ?? 200

這些 limit 欄位都有同款 silent-fallback 缺陷：

• .string("0") → silently fall back to default
• .double(20.0) → silently fall back to default

#8 A1 的 logic「靜默 fallback 是 bug」對所有 numeric option args 都成立。Batch 3 只 fix max_messages，limit 沒 fix。」
— Source: team:devils-advocate

Type

bug (consistency)

Expected

把 parseMaxMessages 風格擴展到所有 Server.swift 的 numeric option args：

internal func parseLimit(_ args: [String: Value], default defaultValue: Int) throws -> Int {
    guard let raw = args["limit"] else { return defaultValue }
    guard let value = Int(raw, strict: false) else {
        throw HandlerArgError(message: "limit must be an integer")
    }
    if value <= 0 {
        throw HandlerArgError(message: "limit must be positive; got \(value)")
    }
    // 視 handler 而定加上 upper bound
    return value
}

或：抽更通用的 parsePositiveInt(_ args:_ key:default:) throws -> Int。

Acceptance

• Server.swift:374, 387, 446, 454 的 4 處 limit ?? N 改用新 helper
• .string("invalid") for limit throws (not silently default)
• .double(0.5) for limit throws
• Tests for each handler's limit validation
• CHANGELOG entry

Code Reference

• Sources/CheTelegramAllMCPCore/Server.swift:374, 387, 446, 454
• Sources/CheTelegramAllMCPCore/HandlerArgs.swift:115 (parseMaxMessages — pattern source)

Related: #8 (extends pattern from max_messages to limit)

---

Current Status

Phase: implemented
Last updated: 2026-05-25 by /idd-implement (PR #32)

Key Decisions

• Cluster: Apply parseMaxMessages pattern to limit and other numeric option args #25 (leader) + chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22 + parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23 — same parser-consistency theme (silent-fallback / type-strictness)
• Complexity: Simple (Layer V V1=2/V4=2; no Layer 2 / Layer P trigger)
• Strategy: 3-layer fix (helper extraction int64ArgValueStrict + parseLimit; caller swap × 4; belt-and-suspenders cap) in 1 file pair (HandlerArgs.swift + Server.swift) + 6 new tests
• Diagnosis comment: Apply parseMaxMessages pattern to limit and other numeric option args #25 (comment)

Open Questions

• (none — DA-derived spec is crisp)

[kiki830621]

Diagnosis

Type

bug (consistency / latent invariant)

Cluster Scope

This diagnosis covers a 3-issue consistency cluster — all three share the same root cause shape (silent-fallback / type-strictness gap in argument parsing), all touch HandlerArgs.swift, and the fixes compose into one helper extraction + caller swap PR:

Issue	Surface	Layer
#25 (leader)	args["limit"]?.intValue ?? N × 4 in Server.swift	Caller layer
#22	int64ArgValue returns nil on .string("not-numeric") → caller throws "X is required" (misleading)	Helper layer
#23	parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap	Caller layer

All three are residue from the #8/#11/#12 → parseMaxMessages consistency wave (commit a918b11). When parseMaxMessages was extracted as the strict-typed helper for max_messages, the same shape was not propagated to: (a) sibling numeric args (limit), (b) sibling type-strict cases (chat_id), (c) sibling cap-protected defaults (parseDumpChatToMarkdownArgs's 5000 path).

Root Cause / Analysis

Layer 1 — Helper int64ArgValue (#22)

Sources/CheTelegramAllMCPCore/HandlerArgs.swift:220-227:

internal func int64ArgValue(_ args: [String: Value], _ key: String) -> Int64? {
    guard let value = args[key] else { return nil }
    if let n = value.intValue { return Int64(n) }
    if let s = value.stringValue { return Int64(s) }   // ← silent nil on parse fail
    return nil
}

When caller passes .string("not-a-number"):

1. value.intValue returns nil (not an int variant)
2. value.stringValue returns "not-a-number"
3. Int64("not-a-number") returns nil → function returns nil
4. Caller at L78-79 sees nil → throws "chat_id is required"

User-facing error implies "key missing" when the key was actually present with wrong type. The asymmetry vs parseMaxMessages (L175, which throws "max_messages must be an integer") was caught at #8 verify by devils-advocate but never propagated.

Layer 2 — parseDumpChatToMarkdownArgs default path (#23)

HandlerArgs.swift:136:

let maxMessages = try parseMaxMessages(args) ?? 5000  // ← default 5000 bypasses validateMaxMessagesCap

When args["max_messages"] is absent, parseMaxMessages returns nil → fallback to literal 5000. The cap check inside parseMaxMessages (L180 try validateMaxMessagesCap(value)) only runs when args carries the key. The literal 5000 default never sees the cap.

Currently harmless because 5000 < 10_000 cap, but the cap's own docstring (L196-199) explicitly markets itself as single source of truth: "propagates atomically". The default-5000 path is silently outside that scope. Future cap tightening (e.g. to 1_000 for paid-tier) would silently produce 5000-message dumps for callers omitting max_messages.

Layer 3 — Other numeric args (#25)

Server.swift:433/446/503/511:

let limit = args["limit"]?.intValue ?? 50    // L433
let limit = args["limit"]?.intValue ?? 20    // L446
let limit = args["limit"]?.intValue ?? 50    // L503
let limit = args["limit"]?.intValue ?? 200   // L511

Each line has the same shape parseMaxMessages was extracted to fix:

• .string("0") → intValue returns nil → silent fallback to default
• .double(20.0) → intValue returns nil → silent fallback (whole-number doubles via MCP JSON encoder)
• Negative / zero values not rejected

The parseMaxMessages shape was specifically built to handle all three categories (see Int(value, strict: false) accept of .double from commit f0203ac). It just was never reused.

Impact

Affected files:

• Sources/CheTelegramAllMCPCore/HandlerArgs.swift (helpers — int64ArgValue, add parseLimit, fix parseDumpChatToMarkdownArgs default)
• Sources/CheTelegramAllMCPCore/Server.swift (4 callsites — L433/446/503/511)
• Tests/CheTelegramAllMCPTests/ServerHandlerLogicTests.swift (add tests for each new error path)

User-facing:

• chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22: confusing error message wastes debug time — user sees "chat_id is required" and checks args structure instead of value type
• parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23: latent — no visible impact today (5000 < 10_000); becomes silently wrong if cap tightens
• Apply parseMaxMessages pattern to limit and other numeric option args #25: same UX gap as the pre-MCP handler 型別強制 + CLI parity: max_messages string bypass + CLI date behavior 不一致 #8 max_messages — JS / Python callers whose JSON encoders emit 5000.0 get silently capped at default instead of seeing a clear "limit must be an integer" error

Workflow:

• After cluster ship, limit accepts same shape as max_messages → consistent parsing across all numeric option args is a documented invariant going forward (no more case-by-case)

Strategy

Helper layer (HandlerArgs.swift):

• Upgrade int64ArgValue to throwing variant int64ArgValueStrict(_ args, _ key) throws -> Int64?:
  • .int(n) → Int64(n)
  • .string(s) → Int64(s) or throw HandlerArgError(message: "\(key) must be an integer; got \"\(s)\"")
  • .double(d) where d.rounded() == d → Int64(d) (parity with parseMaxMessages)
  • Otherwise nil (genuinely absent)
• Update parseGetChatHistoryArgs (L78) + parseDumpChatToMarkdownArgs (L130) to call int64ArgValueStrict; "X is required" only thrown when result is nil from absence (not parse fail)
• Fix parseDumpChatToMarkdownArgs default path: change L136 to belt-and-suspenders pattern:

  let maxMessages = try parseMaxMessages(args) ?? 5000
  try validateMaxMessagesCap(maxMessages)   // ← explicit cap on default path

• Add parseLimit(_ args:, default defaultValue: Int) throws -> Int modeled on parseMaxMessages

Caller layer (Server.swift):

• L433/446/503/511 → let limit = try parseLimit(args, default: 50) (defaults vary 20/50/200 per tool, pass through)

Test layer (ServerHandlerLogicTests.swift):

• testChatIdAsStringInvalidThrowsTypeError — .string("not-a-number") now throws "chat_id must be an integer; got \"not-a-number\"" (not "is required")
• testFromMessageIdAsStringInvalidThrowsTypeError — same shape for from_message_id
• testParseDumpDefaultMaxMessagesRespectsCap — assert default 5000 path still runs validateMaxMessagesCap (regression guard for future cap policy change)
• testLimitRejectsString × 4 callsites — .string("not-a-number") → throws "limit must be an integer"
• testLimitAcceptsWholeNumberDouble — .double(20.0) accepted (parity with parseMaxMessages)
• testLimitRejectsZeroOrNegative — if parseLimit enforces positive

Complexity

Simple

Layer-by-layer assessment:

• Layer 1 disqualifiers: not applicable (not narrative, not ad-hoc analysis, not doc-only). Pass through.
• Layer V Vagueness (Step 3.4 score below): V1=2, V4=2 — both axes well-specified. Acceptance criteria are explicit in each issue body (error message text, file paths, line numbers, expected behavior tables). No trigger.
• Layer 2 (Spectra necessary condition): helpers stay internal — no public API change, no MCP tool surface change, no plugin skill change. Fail.
• Layer P signals:
  • 2+ files with order dependency? Yes — HandlerArgs.swift helper must change before Server.swift callsites. But ordering is mechanical (compile-driven), not decision-heavy.
  • 5+ ordered steps? Strategy has 8 checklist items, but they're parallel-decomposable into 2 layers (helper / caller / test). Not sequential complexity that needs checkpoints.
  • 2+ valid approaches with shape-impact? Slight — int64ArgValueStrict as new function vs upgrading int64ArgValue in-place. Both shapes equivalent at call sites; prefer new-function to keep blast radius narrow.
  • Risk-sensitive boundary? No — these are argument-parser strictness changes. Error messages are part of contract (asserted at Test: assert MCP handler error message text (contract test) #11 / Test refactor: contract assertions use prefix-match instead of exact-equality #24), but the contract change is strictly broader (accepting all previous cases + new error path for previously-silent fail).

Verdict: Simple. Strategy is decomposed into 3 mechanical layers, all in 1 file pair, with clear test coverage strategy. parseMaxMessages already established the shape — this PR is "apply that shape 3 more times".

Risks

Risk	Mitigation
int64ArgValueStrict upgrade breaks call sites that relied on silent-nil (e.g. optional from_message_id)	Audit all call sites: keep nil-on-absent semantics, only throw on parse-fail. Test at all 4 entry parsers.
New parseLimit accepts .double(20.0) — breaks any existing test asserting .double rejection	parseMaxMessages already accepts .double since f0203ac. No existing test asserts double rejection on limit. Verify via grep -n '.double.*limit' in tests.
Adding validateMaxMessagesCap to default-5000 path changes runtime behavior if 5000 ever exceeds cap	5000 < 10_000 currently. Future cap reductions are the intended trigger. Test asserts the invariant, not the current value.
Cluster scope creep — diagnosis covers 3 issues but unrelated 4th creeps in	Strict scope: helper layer + 4 caller swaps + cap-on-default + 6 tests. Sister concerns (#16/#19 fixed-TZ) explicitly excluded — separate clusters.

Residue

NSQL §4.6 residue — the 3-issue cluster's broader intent is "argument parsers should never silently fall back", which extends beyond limit / chat_id / max_messages:

1. from_message_id (L82) — int64ArgValue(args, "from_message_id") ?? 0 — same silent-fallback shape. Inside cluster scope (will be addressed by int64ArgValueStrict upgrade — 0 default preserved when absent, but type errors now throw).
2. output_path (output_path traversal hardening for dump_chat_to_markdown #21) — string-arg parser has its own silent-trust issue (no traversal validation). Out of scope — different shape (string validation vs numeric strictness), tracked separately as output_path traversal hardening for dump_chat_to_markdown #21.
3. CLI parser (telegram-all argparse) — Swift ArgumentParser already enforces types at parse time. No analogous gap.

Cluster ships consistent numeric-arg parsing; #21's string-path concern stays its own issue.

Vagueness Pre-check

• V1: 2 — WHAT is explicit in all 3 issue bodies: exact file paths (L78-79, L136, L433/446/503/511), exact error message text wanted, expected helper signature shown in code blocks.
• V4: 2 — Acceptance criteria specified per issue (chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22 "chat_id must be an integer; got X"; parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23 belt-and-suspenders code shape; Apply parseMaxMessages pattern to limit and other numeric option args #25 parseLimit helper pattern modeled on parseMaxMessages).
• Triggered: no — both axes ≤ 3

Sister Concerns Filed (mid-diagnose, IC_R011 #528)

Sister-pattern scan results (per Strategy + Residue analysis above):

1. from_message_id silent-fallback — same shape as chat_id/limit, addressed inside cluster scope via int64ArgValueStrict upgrade. Skip filing: in-scope (category: covered-by-this-cluster).
2. output_path traversal hardening — string-arg silent-trust at parseDumpChatToMarkdownArgs L43. Skip filing: already-tracked as output_path traversal hardening for dump_chat_to_markdown #21 (category: already-filed).
3. CLI parser numeric-arg strictness — Swift ArgumentParser enforces types at parse time (statically typed CLI), no analogous gap. Skip filing: not-applicable (category: infeasible — no surface).

All three sister mentions resolved without new issues. Cluster scope strictly: HandlerArgs.swift helper layer + Server.swift caller swaps + ServerHandlerLogicTests test additions.

[kiki830621]

Implementation Plan

Cluster: #22 + #23 + #25 — parser-consistency (silent-fallback / type-strictness).

Branch: idd/22-23-25-parser-consistency. PR path enforced (user cluster intent).

Helper Layer (HandlerArgs.swift)

• Upgrade int64ArgValue → throwing variant (chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22): in-place rename to int64ArgValueStrict; throws on .string(s) parse-fail with "\(key) must be an integer; got \"\(s)\""; preserves nil-on-absent semantics; whole-.double(d) accepted (parity with parseMaxMessages)
• Update call sites (4): L78 / L82 (parseGetChatHistoryArgs chat_id + from_message_id), L130 (parseDumpChatToMarkdownArgs chat_id)
• Add parseLimit(_ args:, default defaultValue: Int) throws -> Int (Apply parseMaxMessages pattern to limit and other numeric option args #25) — modeled on parseMaxMessages: accepts .int / .string(numeric) / .double(whole); throws on type-mismatch + zero/negative
• Fix parseDumpChatToMarkdownArgs:136 default-5000 cap bypass (parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23): belt-and-suspenders explicit try validateMaxMessagesCap(maxMessages) after ?? fallback

Caller Layer

• HandlerArgs.swift:81 — parseGetChatHistoryArgs limit: args["limit"]?.intValue ?? 50 → try parseLimit(args, default: 50)
• Server.swift:433 — get_chats limit (default 50)
• Server.swift:446 — search_chats limit (default 20)
• Server.swift:503 — search_messages limit (default 50)
• Server.swift:511 — get_chat_members limit (default 200)

5 callsites total (1 in HandlerArgs + 4 in Server), not 4 as scoped in diagnosis — verified via grep args\["limit"\]\?\.intValue.

Test Layer (ServerHandlerLogicTests.swift)

• Update testChatIdAsStringInvalidThrows (currently asserts "chat_id is required" — the BUG behavior) → assert "chat_id must be an integer; got \"not-a-number\"" (locks chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22 fix)
• Add testFromMessageIdAsStringInvalidThrows — parity check for from_message_id
• Add testParseDumpDefaultMaxMessagesRespectsCap — regression guard for parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23; assert default-5000 path still calls validateMaxMessagesCap (mutation: imagine future cap=1000, default-5000 would throw)
• Add testLimitRejectsInvalidString — parseGetChatHistoryArgs with "limit": .string("xyz") throws
• Add testLimitAcceptsWholeNumberDouble — parseGetChatHistoryArgs with "limit": .double(20.0) accepted (parity with parseMaxMessages)
• Add testLimitRejectsZeroOrNegative — parseLimit rejects ≤ 0

Build & Verify

• cd che-telegram-all-mcp && swift build -c release passes
• swift test --skip E2ETests all pass (includes existing 50+ tests + 6 new + 1 updated)

Commit / PR

• Each logical chunk commits with (#22, #23, #25 cluster) footer
• Push branch + gh pr create referencing all 3 issues
• PR title: fix(all-mcp): parser-consistency cluster — int64ArgValueStrict + parseLimit + dump default cap (#22, #23, #25)

[kiki830621]

Implementation Complete

Checklist (synced from TaskList)

Helper Layer (HandlerArgs.swift)

• int64ArgValueStrict (throwing variant) added → commit 77bf279
• Call site updates in parseGetChatHistoryArgs (L78, L82) + parseDumpChatToMarkdownArgs (L130) → commit 77bf279
• parseLimit helper added → commit 77bf279
• parseDumpChatToMarkdownArgs:136 belt-and-suspenders cap on default-5000 path → commit 77bf279

Caller Layer (Server.swift)

• HandlerArgs.swift:81 (parseGetChatHistoryArgs limit) → commit 77bf279
• Server.swift:433 (get_chats, default 50) → commit 5ae4a8e
• Server.swift:446 (search_chats, default 20) → commit 5ae4a8e
• Server.swift:503 (search_messages, default 50) → commit 5ae4a8e
• Server.swift:511 (get_chat_members, default 200) → commit 5ae4a8e

Test Layer (ServerHandlerLogicTests.swift)

• Update testChatIdAsStringInvalidThrows (locks chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22 fix) → commit 63b74df
• testFromMessageIdAsStringInvalidThrows (new) → commit 63b74df
• testDumpChatIdAsStringInvalidThrows (new, parity check) → commit 63b74df
• testParseDumpDefaultMaxMessagesRespectsCap (parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23 invariant guard, new) → commit 63b74df
• testLimitRejectsInvalidString (new) → commit 63b74df
• testLimitAcceptsWholeNumberDouble (new) → commit 63b74df
• testLimitRejectsZeroOrNegative (new) → commit 63b74df
• testLimitAcceptsNumericString (new — added during impl, not in original plan) → commit 63b74df

Build & Verify

• swift build -c release passes (clean, only pre-existing TDLibFramework linker warnings about macOS version mismatch)
• swift test --skip E2ETests → 212 tests, 0 failures (was 204 baseline = +8 net: 7 new tests + 1 added during impl)

Commit / PR

• PR opened: #32 — fix(all-mcp): parser-consistency cluster — int64ArgValueStrict + parseLimit + dump default cap (#22, #23, #25)

Legend: - [x] done · - [~] skipped (may revisit) · - [-] won't fix (scope)
idd-close will refuse to close if any - [ ] remains in this checklist.

Changes

• 77bf279 feat: add int64ArgValueStrict + parseLimit + dump default cap (chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22, parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23, Apply parseMaxMessages pattern to limit and other numeric option args #25)
• 5ae4a8e refactor: swap limit callers to parseLimit + use errorResultFromParse in catch-all (chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22, parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23, Apply parseMaxMessages pattern to limit and other numeric option args #25)
• 63b74df test: cover parser-consistency cluster (chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22, parseDumpChatToMarkdownArgs default 5000 bypasses validateMaxMessagesCap (latent invariant) #23, Apply parseMaxMessages pattern to limit and other numeric option args #25)

Files Changed

 Sources/CheTelegramAllMCPCore/HandlerArgs.swift              |  75 +++++++++++++-
 Sources/CheTelegramAllMCPCore/Server.swift                   |  17 +++-
 Tests/CheTelegramAllMCPTests/ServerHandlerLogicTests.swift   | 113 ++++++++++++++++++++-
 3 files changed, 193 insertions(+), 12 deletions(-)

Drive-by

One drive-by improvement included in commit 5ae4a8e and called out in the commit body + PR description:

• handleToolCall catch-all (L591): errorResult(error.localizedDescription) → errorResultFromParse(error). Required for try parseLimit throws to surface as user-friendly messages via MCP transport (without this, HandlerArgError.description wouldn't reach the client through the catch-all path). Net: HandlerArgError + DateParseError get their description; other errors still fall through to localizedDescription inside errorResultFromParse. No behavior change for existing throw paths (those go through dedicated case-arm do/catch blocks that already use errorResultFromParse).

Sister Bugs Filed (mid-impl, v2.44.0+ #526)

One sister bug surfaced during reproduction analysis + filed per IC_R011 file-by-default:

• [all-mcp] Apply int64ArgValueStrict to Server.swift direct callsites (sister to #22) #33 — Apply int64ArgValueStrict to Server.swift direct callsites (sister to #22). The cluster scoped chat_id type-mismatch error message misleads (silently shows 'is required' instead of type error) #22 to parser-layer callsites (3 in HandlerArgs.swift). The remaining 20+ direct callsites in Server.swift (get_user, send_message, edit_message, etc.) have the same int64ArgValue + return errorResult("X is required") bug shape. New issue documents the 3 viable fix approaches (Option A/B/C in body) for follow-up diagnose.

Scope Compliance

• ✓ All 3 cluster issues' Strategy items covered (parser layer + caller layer + test layer)
• ✓ One drive-by (catch-all improvement) documented in commit body + PR
• ✓ One sister concern filed as [all-mcp] Apply int64ArgValueStrict to Server.swift direct callsites (sister to #22) #33 (not implemented in this PR)
• ✓ No unrelated code touched

Next: verification pending

/issue-driven-dev:idd-verify --pr 32

[kiki830621]

Verify (via PR #32)

Result: 🔴 FAIL — 5 P1 blockers + 3 P2 in-scope fixes

Full report: #32 (comment)

This issue's slice

• F4 (P1): parseLimit lacks upper cap (parseMaxMessages has 10_000; parseLimit only > 0). Inconsistency vs stated "parity with MCP handler 型別強制 + CLI parity: max_messages string bypass + CLI date behavior 不一致 #8 A1" goal. Caller can pass .int(999_999_999) straight to TDLib.
• F1 (P1): commit body has auto-close trap (Closes #25).
• F7 (P2): per-handler limit tests missing (only parseGetChatHistoryArgs directly covered; get_chats / search_chats / search_messages / get_chat_members not directly tested).
• F6 (P2): CHANGELOG [Unreleased] entry missing.

Next: add validateLimitCap (shared 10_000 or per-handler) → re-verify.