Standardize timeout parameter naming to clarify units#304
Merged
Conversation
Duration values previously travelled under bare names like `timeout` (seconds in the CLI/IPC layer, milliseconds in the core client) plus a mix of `Ms`/`_MS` suffixes. Rename all internal identifiers to a single convention: `...Secs`/`...Millis` for variables and fields, `..._SECS`/`..._MILLIS` for constants. External surfaces are unchanged: the `--timeout` flag, the `timeout` field in mcp.json / sessions.json (ServerConfig), the ping command's `durationMs` JSON output, MCP SDK request options, Node/library options, and wire-format fields (x402 `maxTimeoutSeconds`, OAuth `expires_in`). Already unit-explicit `...Seconds` names that mirror protocol vocabulary are also left as-is. The convention is documented in CLAUDE.md. Note: the CLI→bridge IPC request field is renamed `timeout` → `timeoutSecs`; a mixed-version CLI/bridge pair would fall back to the default request timeout until the bridge restarts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DndLCCyhJFcYsLP1EDZDms
Follow-up sweep beyond `timeout`: `expiryOverride` → `expiryOverrideSecs` (x402 signer input, holds seconds), `parseDuration()` → `parseDurationMillis()`, `sleep(ms)`/`formatElapsed(ms)` → `millis`, local `durationMs`/`expiresIn` → `durationMillis`/`expiresInSecs`, and `fileAge` → `fileMtimeMillis` (it holds an mtime timestamp, not an age). Kept: the persisted keychain token field `expiresIn`, the ping command's `durationMs` JSON output field, and the e2e server's `LATENCY_MS` env var — all externally visible names. Refs #304 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DndLCCyhJFcYsLP1EDZDms
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
3 participants
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.
Internal duration values travelled under bare names like
timeout— seconds in the CLI/IPC layer, milliseconds in the core client — so the unit had to be inferred from context. This unifies all internal duration names under one convention:...Secs/...Millisfor variables and fields,..._SECS/..._MILLISfor constants, converting units at the boundary where external values enter. No user-visible behavior change.--timeoutflag,timeoutfield in mcp.json/sessions.json, ping'sdurationMsJSON output, SDK/Node/library option names, wire-format fields (x402maxTimeoutSeconds, OAuthexpires_in)...Secondsnames mirroring protocol vocabulary are left as-istimeout→timeoutSecs; a mixed-version CLI/bridge pair falls back to the default request timeout until the bridge restartsVerified with lint, build, all 862 unit tests, and a live smoke test of
connect/tools-callwith--timeoutagainst the e2e server.https://claude.ai/code/session_01DndLCCyhJFcYsLP1EDZDms