feat(qbit): add speed limit tool

[koinsaari]

Adds qbit_speed_limits — a single tool for reading and setting qBittorrent global speed limits and the built-in alternative-rate scheduler.

What it does

Read (no set argument): returns current state — dl/ul limits, alt limits, whether alt mode is active, and full scheduler config.

Write (pass a set object with only the fields you want to change):

• download_limit / upload_limit — global limits in KiB/s, 0 = unlimited
• alt_download_limit / alt_upload_limit — alternative rate limits in KiB/s
• use_alt_limits — switch to alt limits immediately (read-then-toggle, no-op if already in desired state)
• schedule_enabled, schedule_from_hour, schedule_to_hour, schedule_days — configure the automatic time-based switch

schedule_days supports all 10 qBittorrent API values: all, weekdays, weekends, monday–sunday.

Validation

• Speed limits reject negative values and non-numbers
• Hours validated 0–23
• schedule_enabled validated as bool before forwarding to API (previously any type would pass through)
• Unknown schedule_days values return a descriptive error listing valid options

Tests

16 new test cases covering read, write, toggle behaviour, all day mappings, and all validation error paths.

🤖 Co-authored with Claude Sonnet 4.6

[github-actions]

Claude finished @koinsaari's task in 2m 23s —— View job

---

Review: Add qbit_speed_limits tool

• Read REVIEW.md and CLAUDE.md
• Review changed files
• Post review findings
• Set commit status

---

🔴 Important

Partial write on invalid use_alt_limits — internal/tools/qbittorrent.go:772-775

The use_alt_limits type-check happens after SetPreferencesCtx is called (line 767). If a request includes both preference fields and an invalid use_alt_limits (e.g., "yes" instead of true), the preferences are successfully applied, then the handler returns an error. The caller sees an error response and has no way to know that some fields were already written.

Move the use_alt_limits type assertion up into the validation block (before SetPreferencesCtx) so all input is validated before any downstream API call:

// validate use_alt_limits early, alongside the other field checks
var toggleAlt *bool
if v, ok := setMap["use_alt_limits"]; ok {
    desired, ok := v.(bool)
    if !ok {
        return mcp.NewToolResultError("use_alt_limits must be a bool"), nil
    }
    toggleAlt = &desired
}

// ... existing SetPreferencesCtx call ...

// then use toggleAlt instead of re-reading from setMap
if toggleAlt != nil {
    current, err := qc.GetAlternativeSpeedLimitsModeCtx(ctx)
    // ...
}

Fix this →

---

🟡 Nit

Fractional hours silently truncated — internal/tools/qbittorrent.go:740-752

schedule_from_hour and schedule_to_hour accept fractional values (e.g., 22.5) which are silently truncated to 22 via int(h). Consider adding h != math.Trunc(h) to the validation condition to reject non-integer values.

---

Summary

One blocking issue: use_alt_limits validation runs after preferences are already written, so a type error causes a partial write that the caller can't detect. All other checks pass — tool is registered in server.go, errors return mcp.NewToolResultError (no panics), no credentials in response content, HTTP client uses the shared qBit client, all inputs are validated before use, and test coverage is thorough across read/write/toggle/validation paths. Clean, well-structured implementation.

[github-actions]

Claude finished @koinsaari's task in 2m 17s —— View job

---

• Read REVIEW.md and CLAUDE.md
• Review changed files
• Post review findings
• Set commit status

---

Review: Add qbit_speed_limits tool (re-review after fix)

No blocking issues.

Previous findings

Both findings from the prior review have been addressed:

• use_alt_limits partial write (was 🔴 Important) — Fixed in e45a119. Validation now runs early via a *bool variable before SetPreferencesCtx is called. The new TestQBitSpeedLimits_UseAltLimitsWrongType test explicitly asserts that SetPreferences is never called when the type check fails.
• Fractional hours silently truncated (was 🟡 Nit) — Fixed. schedule_from_hour and schedule_to_hour now reject non-integers with h != math.Trunc(h), and TestQBitSpeedLimits_FractionalHourRejected covers the path.

Checklist

Check	Status
Errors return mcp.NewToolResultError(...), no panics	✅ All 12 error paths return MCP errors
No credentials/secrets in response content	✅ Response contains only speed limits, scheduler config, mode
HTTP client has explicit timeout	✅ Uses shared QBitClient (no direct HTTP calls)
Input validated before downstream API calls	✅ All fields validated; use_alt_limits checked before SetPreferencesCtx
Tool registered in server.go	✅ s.AddTool(tools.QBitSpeedLimits(qc))

🟡 Nit

Unknown set fields silently ignored but echoed as "applied" — internal/tools/qbittorrent.go:804

If a caller passes a misspelled field (e.g. "donwload_limit"), it's silently skipped during processing but still appears in the "applied" map in the response. The caller sees {"mode":"write","applied":{"donwload_limit":100}} and may believe the field took effect. Not a correctness bug — just potentially misleading for AI agents that don't cross-check.

Summary

Clean implementation. The previous review's blocking issue (partial write on invalid use_alt_limits) is fixed and verified by a dedicated test. All CLAUDE.md invariants hold. 16 test cases cover read, write, toggle, day mappings, and all validation error paths. Ready to merge.

[github-actions]

Claude finished @koinsaari's task in 3m 16s —— View job

---

• Read REVIEW.md and CLAUDE.md
• Review changed files
• Post review findings
• Set commit status

---

Review: Add qbit_speed_limits tool

No blocking issues.

Previous findings

All three findings from prior reviews have been addressed:

• use_alt_limits partial write (was 🔴 Important) — Fixed in e45a119. Validation now runs early via a *bool before SetPreferencesCtx. Test TestQBitSpeedLimits_UseAltLimitsWrongType asserts SetPreferences is never called on invalid type.
• Fractional hours silently truncated (was 🟡 Nit) — Fixed. Hours now reject non-integers with h != math.Trunc(h). Test TestQBitSpeedLimits_FractionalHourRejected covers this.
• Unknown fields echoed as applied (was 🟡 Nit) — Fixed in 15df0f7. The applied map is now built from prefsUpdate (recognized fields only) + toggleAlt, not from the raw input. Test TestQBitSpeedLimits_UnknownFieldNotEchoed covers this.

Checklist

Check	Status
Errors return mcp.NewToolResultError(...), no panics	✅ All error paths return MCP errors
No credentials/secrets in response content	✅ Response contains only speed limits, scheduler config, mode
HTTP client has explicit timeout	✅ Uses shared QBitClient (no direct HTTP calls)
Input validated before downstream API calls	✅ All fields validated; use_alt_limits checked before SetPreferencesCtx
Tool registered in server.go	✅ s.AddTool(tools.QBitSpeedLimits(qc))

🟡 Nit

applied response uses internal API keys and byte values — internal/tools/qbittorrent.go:793-799

The applied map echoes the qBittorrent API's internal key names and raw byte values rather than the tool's user-facing field names and KiB/s values. A caller that sends {"download_limit": 1024} gets back {"applied": {"dl_limit": 1048576}} — both the key and value differ from what was submitted. For AI agent callers, a response that mirrors the input schema would be easier to correlate.

Summary

Clean implementation. All three findings from prior reviews are fixed and verified by dedicated tests. 19 test cases cover read, write, toggle, day mappings, validation errors, SDK errors, and the unknown-field edge case. Ready to merge.

[github-actions]

Claude finished @koinsaari's task in 2m 15s —— View job

---

• Read REVIEW.md and CLAUDE.md
• Review changed files
• Post review findings
• Set commit status

---

Review: Add qbit_speed_limits tool

No blocking issues.

Previous findings

All three findings from prior reviews have been addressed:

• use_alt_limits partial write (was 🔴 Important) — Fixed in e45a119. Validation now runs early via a *bool before SetPreferencesCtx. Test TestQBitSpeedLimits_UseAltLimitsWrongType asserts SetPreferences is never called on invalid type.
• Fractional hours silently truncated (was 🟡 Nit) — Fixed. Hours now reject non-integers with h != math.Trunc(h). Test TestQBitSpeedLimits_FractionalHourRejected covers this.
• Unknown fields echoed as applied (was 🟡 Nit) — Fixed in 1e808c1. The applied map is now built by iterating a knownFields list against setMap, so unrecognised keys are excluded. Test TestQBitSpeedLimits_AppliedEchoesUserFacingKeys verifies user-facing keys are echoed and internal keys are not.

Checklist

Check	Status
Errors return mcp.NewToolResultError(...), no panics	✅ All 12 error paths return MCP errors
No credentials/secrets in response content	✅ Response contains only speed limits, scheduler config, mode
HTTP client has explicit timeout	✅ Uses shared QBitClient (no direct HTTP calls)
Input validated before downstream API calls	✅ All fields validated; use_alt_limits checked before SetPreferencesCtx
Tool registered in server.go	✅ s.AddTool(tools.QBitSpeedLimits(qc))

🟡 Nit

Toggle race between read and toggle — internal/tools/qbittorrent.go:781-790

The use_alt_limits toggle does a read-then-conditional-toggle: it calls GetAlternativeSpeedLimitsModeCtx, checks if the current state differs from the desired state, then calls ToggleAlternativeSpeedLimitsCtx. If another client toggles the mode between the read and the toggle, the result is the opposite of what was requested. This is inherent to the qBittorrent API (no "set" endpoint, only toggle), so there's no fix — just noting the limitation.

Summary

Clean implementation. All three findings from prior reviews are fixed and covered by dedicated tests. 19 test cases cover read, write, toggle, day mappings, validation errors, type errors, SDK errors, and the unknown-field edge case. All CLAUDE.md invariants hold. Ready to merge.