2.5.1 — Round 2+3 fix bundle (24 bugs)#20
Merged
chenliuyun merged 34 commits intomainfrom Apr 20, 2026
Merged
Conversation
added 30 commits
April 20, 2026 19:18
MCP tool-call errors were collapsing to a plain-text message through the SDK's generic error wrapper, losing subKind / transient / hint / retryAfterMs / errorClass / retryable. Agents had to parse English strings to branch on device-offline vs auth-failed. Enrich mcpError() to emit the full ErrorPayload shape under structuredContent.error. Add apiErrorToMcpError() helper that routes any thrown error through buildErrorPayload(). Wire into the three tool handlers that previously rethrew (send_command, describe_device) or ran unprotected (run_scene). Tests: 3 new MCP tests asserting structuredContent.error shape for ApiError codes 161 / 401 / 190 through each of the three tools.
…nion Follow-up to 7fae5c2. subKind and errorClass were typed as wide strings, losing compile-time catches for typos at direct call sites. Import the canonical types from utils/output and reuse them.
devices batch was stripping subKind and verification from per-device results — the very IR unverifiability signal 2.4.0 introduced. An 8-device AC batch would emit zero unverifiability signal. Mirror the single-device IR annotation in BatchResult.succeeded[] and add summary.unverifiableCount aggregate. Tests: 2 new cases covering IR (attached) vs physical (absent).
Rotating credentials or switching profiles was still serving the
prior session's inventory because devices.json lived at a fixed
disk path. In multi-tenant automation this looked like account-A
data bleeding into account-B.
When an active profile is set, cache files now live under
~/.switchbot/cache/<sha256(profile):8>/{devices,status}.json.
Unnamed/default profile keeps the legacy ~/.switchbot/devices.json
path for backwards compatibility. --config-path override unchanged.
Tests: 4 new cases covering default, scoped, per-profile isolation,
and status-cache parity. Also fixes name-resolver.test.ts partial
flags mock to include getProfile (required by getActiveProfile).
… serve (bug #37) Follow-up to bebc1d7. Disk was scoped by profile but the module-level _listCache / _statusCache globals were not — so mcp serve, which rotates profiles per request via withRequestContext, would return the first profile's inventory on subsequent requests regardless of the active profile. Replace the two singletons with Map<profile, Cache> keyed by getActiveProfile() (with '__default__' sentinel for the unnamed profile). resetListCache/resetStatusCache still clear everything. Also restore the JSDoc comment on DEFAULT_STATUS_GC_TTL_MS dropped in bebc1d7. Tests: 1 new case asserting no leak across profile switches in a single process.
2.4.0 accepted --fields id / --fields name as aliases for deviceId and deviceName; the 2.5.0 list-alias refactor dropped them. Restore the mapping so scripts calling `devices list --fields id,name` keep working.
Code 190 is SwitchBot's generic "internal error" — it fires for invalid deviceIds, unsupported parameters, AND non-device endpoints like `webhook query` with no webhook configured. The "device-busy" subKind and device-specific hint were misleading for webhook. Rename subKind device-busy → device-internal-error and rewrite the hint to reflect the real semantics.
…#31) `scenes execute <bogus>` returned ok:true because the upstream API doesn't validate sceneIds. `scenes describe` already guards against this — port the same check to execute so agents can't silently burn quota on nonexistent scenes.
Round-2 report: `cache clear --status` was rejected with "unknown option"; users had to know the more verbose --key status form. Both shorthands now work; using them with --key or together errors with a clear UsageError.
Nothing stopped two devices from carrying the same alias — --name resolution against a duplicated alias was undefined. Reject duplicate aliases with a clear error naming the existing holder; --force reassigns (clears the old holder's alias) with a log line.
Help text showed `(default: [])`, implying optional; the command actually required it and threw a custom error. Switch to Commander's .requiredOption so `--help` says "required" and the error message matches other required options in the CLI.
Follow-up to 419556e. The reject paths for (a) --status combined with --list and (b) --status combined with --key were implemented in src but not exercised by tests. Add two small UsageError assertions to close the coverage gap.
Code 3005 "invalid value" is the API's catch-all for model-specific command rejections (e.g., Fan speed commands on stock IR remotes that only work under --type customize). Previously surfaced as unknown-api-error with no actionable hint.
Agents chaining validate → run were surprised that plans with bogus deviceNames or sceneIds passed validation. Make the scope explicit in the description and point users to `plan run --dry-run` for semantic checks.
…#34) Operators who `touch` cache files to force-refresh were surprised the CLI ignored mtime. One-line note in help text.
…bug #40) The local metadata system (devices meta set --alias …) was undiscoverable: not in agent-bootstrap, not in capabilities, not in the devices help footer. Add the four subcommands to COMMAND_META and a 'meta' entry to QUICK_REFERENCE so agents find it on first bootstrap.
The .json file in ~/.switchbot/device-history/ was never documented. It's the 100-entry ring buffer read by MCP get_device_history; the .jsonl is the append-only source of truth for history range and aggregate.
Round-2 smoke-test response: 13 bug fixes across 18 commits. See CHANGELOG.md for the per-bug breakdown and responses to the two false positives and two deferred feature requests.
The VERSION assertion compared against both package.json and a hardcoded '2.5.0' string. The hardcoded form silently went stale on every version bump; the package.json read already proves the real contract. Remove the redundant literal so future bumps don't require a test edit.
Extend parseDurationToMs regex to (ms|s|m|h|d|w)? and update durationArg error message to list all supported units. Previously --cache 1d, --for 2w, etc. returned null and silently fell back to defaults. Unsupported units (y, year, month) still reject but now with a hint listing the six supported suffixes.
The --name-strategy help text hard-coded the strategy list in three places, which drifted from the source of truth (ALL_STRATEGIES in name-resolver.ts). Export ALL_STRATEGIES and generate the help text from it. Also expose the list as a top-level 'nameStrategies' field in agent-bootstrap so agents can discover all six strategies (exact, prefix, substring, fuzzy, first, require-unique) without grepping help text. Fixes bug #51.
Empty queries caused search_catalog to scan the entire catalog, which both hid the agent's intent (was the empty string a bug, or deliberate 'list all'?) and produced unnecessarily large MCP payloads. Reject with a usage error and suggest list_catalog_types for the enumerate-everything case. Fixes bug #57.
…refix The 2.4.0 release notes and user-facing docs referred to `--idempotency-key`, but the actual flag has always been `--idempotency-key-prefix` (the prefix gets the deviceId appended to form per-device keys). Accepting both spellings removes the documentation/implementation mismatch without breaking existing scripts. Supplying both forms is rejected to keep the intent unambiguous. Fixes bug #30.
Adds an opt-in --skip-offline flag that checks the local status cache before dispatching commands. Devices whose cached body shows onlineStatus === 'offline' are recorded under result.skipped[] (with reason: 'offline') instead of being hit. Cache miss falls through to the normal send path, so the flag never introduces new network calls for the preflight itself. Default remains off — patch release must not change existing batch behavior. Fixes bug #33.
…, etc. Commander treated negative numbers like '-1' as unknown option tokens and rejected the whole invocation with 'error: unknown option -1'. Enable allowUnknownOption on the 'devices command' subcommand so negative numbers flow through as the parameter positional; they reach the API (and are rejected by device validators if out of range) instead of failing at argv parsing. Trade-off: unknown flag typos on this subcommand (e.g. '--dryrun' instead of '--dry-run') are now silently ignored instead of erroring. Acceptable here because the subcommand's surface area is small and the parameter validator still catches obviously malformed values. Fixes bug #53.
Adds --for <duration> to 'devices watch', 'events tail', and 'events mqtt-tail' as a time-based stop condition (vs --max which counts ticks/events). When both are provided, whichever limit hits first wins. Implemented via a setTimeout that aborts the existing AbortController — the timer is cleared in the cleanup/finally paths so long-running loops that exit via other routes don't leak handles. Fixes bug #52.
Previously, `events mqtt-tail --json` only emitted JSON after the MQTT
broker connected. If credential fetch or broker connect failed (or the
process exited early via --max/--for), downstream JSONL consumers saw
zero lines and could not distinguish "never ran" from "ran but no events".
Now an opening `__session_start` envelope is emitted immediately when
--json is set, carrying { type, at, eventId, state:'connecting' }. It
always appears as the first JSON line, before any credential fetch,
mirroring the existing __connect/__disconnect control record pattern.
…L (bug #39) The previous --filter only recognized key=value clauses with implicit substring semantics (except for category, which did exact match). Users couldn't explicitly request substring or regex, and there was no way to pattern-match device types beyond trivial prefix overlap. --filter now parses each comma-separated clause as one of: key=value — current behavior (substring; exact for category) key~value — explicit case-insensitive substring key=/pattern/ — case-insensitive regex; invalid regex exits 2 Supported keys (type, name, category, room) and multi-clause AND composition are unchanged.
…em (bug #SYS-1)
Previously every error under --json landed on stderr, while every success
landed on stdout — a pipeline like `cli --json devices status X | jq ...`
saw nothing on the error path, making the structured envelope useless
for automation. handleError and all bespoke JSON error emitters now go
through emitJsonError(), which writes the envelope on stdout and mirrors
a short red message on stderr only when stderr is a TTY.
Consolidated ~15 ad-hoc `console.error(JSON.stringify({ error: … }))`
call sites across batch/config/devices/expand/history/mcp/format into
the new emitJsonError helper so the contract stays consistent.
added 3 commits
April 20, 2026 22:31
send_command with dryRun:true now validates the deviceId against the local device cache before returning the wouldSend envelope. Fabricated IDs (e.g. 'DEADBEEF') now return a usage error with subKind 'device-not-found' instead of silently echoing back a plausible-looking preview. Dry-run is a validation surface; silently accepting arbitrary input defeated the point. Existing regression tests that exercise dryRun with specific IDs now seed the cache explicitly.
CHANGELOG: rework [2.5.1] section — expand scope statement from "Round-2 only" to "Round-2 + Round-3", add three new subsections covering the 10 extra commits (2 🔴 contract bugs SYS-1/SYS-3, 3 round-2 leftovers, 5 DX polish items), and revise "Not included" to list the three items actually deferred (parallel-status profiling, watch --json doc wording, meta import/export). README: document the three --filter operators (=, ~, =/regex/), --skip-offline and --idempotency-key alias under devices batch, --for on watch / events tail / events mqtt-tail, and a note that negative positional parameters (setBrightness -1) reach the validation layer.
…r (bug #SYS-1 followup) Canary testing on 2.5.1 uncovered a bespoke DeviceNotFoundError handler in `devices describe` that was writing plain text to stderr under --json, bypassing the emitJsonError helper added in bc473b0. Clients piping `devices describe <unknown> --json | jq` saw an empty stdout and couldn't distinguish this from a silent failure. Route the error through emitJsonError when isJsonMode() so the schemaVersion envelope reaches stdout with exit 1; keep the human message on stderr for TTY users. Also ignore tmp/ in .gitignore so the canary harness stays out of the repo.
chenliuyun
changed the title
Before: three independent parsers with mismatched grammars:
- devices list: =/~/=/regex/ (substring, category exact)
- devices batch: = (exact) / ~= (substring)
- events tail: = (exact) only
Now all three share one grammar: `key=value` (case-insensitive
substring; exact only for `category`), `key~value` (explicit
case-insensitive substring), `key=/pattern/` (case-insensitive
regex; invalid regex is a usage error). Clauses AND-ed. Each
command still exposes its own key set.
BREAKING CHANGE: `devices batch --filter 'type=Bot'` previously
required an exact match and now treats `Bot` as a substring
(matches `Bot Plus` too). `devices batch --filter 'type~=X'`
(the `~=` spelling) is removed — use `~` instead. `events tail
--filter 'deviceId=ABC'` is now a substring match (previously
exact). See CHANGELOG §"Changed (BREAKING)" and README
§"Filter expressions — per-command reference" for the full
per-command table and migration.
Implementation:
- src/utils/filter.ts: add parseFilterExpr(expr, allowedKeys) +
matchClause(candidate, clause, { exactKeys }); keep legacy
parseFilter/applyFilter exports so src/commands/batch.ts
needs no change.
- src/commands/events.ts: switch to shared parser with
EVENT_FILTER_KEYS=['deviceId','type']; FilterClause[]|null
replaces the old ad-hoc {deviceId?,type?} shape.
- src/commands/devices.ts (list): already on the new grammar
since 2.5.1 bug #39 — unchanged.
- Tests rewritten for new shape; added 'Bot Plus' fixture and
a regex-alternation case to prove the substring switch.
Verification: 959/959 vitest green; 516/516 canary green at 50
workers (48s); real-account smoke on list/batch/events tail for
all three operators; `type~=X` rejected with a hint that points
to `type~X`.
Collaborator
Author
Follow-up commit: unify
|
| Surface | Before | After |
|---|---|---|
devices list |
= (sub, category exact) / ~ (sub) / =/regex/ |
unchanged |
devices batch |
= (exact) / ~= (sub) |
= (sub; category exact) / ~ (sub) / =/regex/ |
events tail |
= (exact) only |
= (sub) / ~ (sub) / =/regex/ |
Keys stay per-command: list {type,name,category,room}, batch {type,family,room,category}, events {deviceId,type}.
Breaking (documented under CHANGELOG §"Changed (BREAKING)")
devices batch --filter 'type=Bot'is now a substring match — matchesBot Plustoo. Pair=with a more specific value or filter post-hoc if exact was load-bearing.devices batch --filter 'type~=X'(the~=spelling) is removed. Migration hint is printed by the parser:"type~=X" — "~=" is no longer supported. Use "type~X" instead.events tail --filter 'deviceId=ABC'is now a substring match (previously exact).
2.5.1 remains a patch version per maintainer call; the breaking surface is narrow (two flags on one feature) and CHANGELOG is explicit. Version unification is the durable fix — shipping documented on one fence rather than three independent ones.
Verification
npm test— 959/959 (filter.test rewritten; events.test updated forFilterClause[]signature)tmp/canary/par.sh— 516/516 at 50-way concurrency, 48s- Real-account smoke on list/batch/events tail for all three operators;
type~=Xrejects with the migration hint;category=physreturns 0 (exact preserved on category)
Files: src/utils/filter.ts (parser+matcher), src/commands/events.ts (parseFilter/matchFilter switched to shared), tests, README (new per-command reference table), CHANGELOG.
Ready for re-review on the new commit.
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.
Summary
2.5.1 patch release responding to the OpenClaw smoke-test reports (Round 2 + Round 3, 460 cases across both). Closes 24 bugs — all fixes, no breaking changes, no new required config.
Fixed (Round 3 contract bugs)
--jsonerrors now emit on stdout socli | jqpipelines see the structured envelope for both success and failure paths (bug #SYS-1 + follow-up fordevices describeDeviceNotFoundError)send_command { dryRun:true }preflights deviceId against the local cache; unknown IDs returndevice-not-foundinstead of silently accepting fabricated input (bug #SYS-3)Fixed (Round 2 leftovers)
devices batch --idempotency-keyaccepted as alias for--idempotency-key-prefix(bug #30)--filterDSL supportskey~substringandkey=/regex/alongside the existingkey=value(bug #39)Added
devices batch --skip-offline(default off) skips cached-offline devices with each skip recorded undersummary.skipped(bug #33)--for <duration>alias ondevices watch,events tail,events mqtt-tail— stop after elapsed time instead of tick count (bug #52)dandwin addition toms/s/m/h(bug #54)events mqtt-tail --jsonemits a__session_startenvelope immediately so downstream tools can distinguish "connecting" from "never connected" (bug #56)Polish
--name-strategyhelp andagent-bootstrapoutput list all six strategies (bug #51)search_catalogrejects empty queries (bug #57)--formatstrictly validated against the supported list; unknown values exit 2 (bug #59)setBrightness -1) reach the validation layer instead of being swallowed by Commander (bug #53)Round 2 fixes (earlier commits on this branch)
#22, #27, #28, #29, #31, #32, #34, #35, #36, #37, #38, #40, #41, #42, #43 + MCP type narrowing
Test plan
npm test— 945/945 greentmp/canary/par.sh, 50 workers against a real SwitchBot account) — 516/516 pass across C1–C12 + regressions + live readsdevices list --filter 'type=/Hub.*/' --json,send_command {dryRun:true}preflight,--format csvrejection,--for 500msclean exitkey=valuefilter queries — confirmed against a 25-device live account:category=physical→ 15,category=ir→ 10,type=Hub(substring) → 5,name=Hub→ 3,category=physical,name=Hub(AND) → 3,type=Air Conditioner(substring with space) → 8, unknown key rejected exit 2 with identical message shape. Code diff: neweq-op path is byte-identical to old matcher (category exact, others lowercased substring).[2.5.1]maps to a commit on the branch; "Not included" section matches the plan (feat: 2.5.0 — history aggregate + v2.4.0 report bug fixes #19 false positive; #58/#55/naming/meta import-export deferred to 2.6.0). README additions (3 filter operators,--skip-offline,--idempotency-keyalias,--foron watch+events, negative-positional note) match the shipped code exactly;--forgoes throughparseDurationToMsso the claimedms/s/m/h/d/wgrammar holds.