fix(config): support timeout_sec in config set

[Yukibei]

Description

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• make test passes locally
• Manual testing (describe below)

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly (if applicable)
• I have signed the CLA

Related Issues

[CLAassistant]

All committers have signed the CLA.

[github-actions]

✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

[Qiyuanqiii]

The implementation itself looks good to me. There is one user-facing inconsistency I think should be fixed before merge: the configuration docs still explicitly say that timeout_sec is not supported by ocr config set and instruct users to edit config.json manually. Since enabling that exact CLI path is the purpose of this PR, the timeout documentation should be updated alongside the code.
One small robustness suggestion: parseTimeoutSeconds currently checks integer parsing and non-negativity, while the resolver also rejects values that overflow time.Duration. It would be cleaner if config set applied the same bound so it cannot successfully persist a value that the resolver will later reject.
Other than that, the setter paths and tests look good. I'd be happy to re-review once the docs are updated and CI has run.

[Qiyuanqiii]

Quick correction to my previous review: I was a bit too strong in using Request changes here.

The docs mismatch and the timeout overflow edge case are still worth addressing, but after reconsidering the scope and impact, I don't think either should be treated as a merge blocker for this PR. Please treat my earlier review as regular review feedback rather than a request to hold the PR.

The implementation itself looks good to me. I'll wait for CI to run before giving a final approval.

[lizhengfeng101]

LGTM

[lizhengfeng101]

parseTimeoutSeconds here checks for non-negative but skips the overflow guard that validateTimeoutSec in the resolver enforces, so a value between ~9.2e9 and MaxInt64 sails through config set and only blows up later at resolve time — could we mirror that upper bound here (ideally by reusing the resolver's check) so the two ends stay in sync?

[Qiyuanqiii]

Even though the PR has been approved by a maintainer, I still want to keep my earlier point on record: the "timeout_sec" overflow validation is worth fixing so that "config set" and the resolver enforce the same bounds.

That said, I agree this is not a merge blocker for the current PR.

[Qiyuanqiii]

Thanks for the follow-up. The timeout validation now reuses the resolver's existing bounds check, and the related documentation has been updated as well.

This addresses my previous concerns. LGTM from my side.

[Yukibei]

Implemented in eba0978.

• Reused the resolver's timeout validation for ocr config set.
• config set and endpoint resolution now share the same non-negative
  and time.Duration overflow checks.
• Added coverage for values above the maximum representable timeout.
• Updated timeout configuration documentation in all supported
  languages.

[Yukibei]

The requested overflow validation and documentation updates have been implemented in eba0978. All checks are passing. Thank you for the review.

[lizhengfeng101]

One thing to fix before merge: timeout_sec should also be listed in isAuxiliaryProviderField (cmd/opencodereview/config_cmd.go:819) — otherwise ocr config set custom_providers.brandnew.timeout_sec 900 silently creates a protocol-less entry ({"custom_providers":{"brandnew":{"timeout_sec":900}}}) that the resolver will later reject, whereas retry_codes in the same position correctly errors with "set a core field first".

[Yukibei]

Fixed in a6af684.

Added timeout_sec to isAuxiliaryProviderField so setting timeout_sec
on an unconfigured provider now returns the same “set a core field
first” error as retry_codes, without creating an incomplete provider
entry. Added regression coverage for both provider namespaces. Focused
tests pass.

[lizhengfeng101]

LGTM