diff --git a/.gitignore b/.gitignore index c8026823..87f911f5 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,4 @@ CLAUDE.md # Init transcript 2026-04-10-155920-command-messageinitcommand-message.txt +tmp/ diff --git a/CHANGELOG.md b/CHANGELOG.md index ee5969ad..8a0b8e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,203 @@ All notable changes to `@switchbot/openapi-cli` are documented in this file. The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.5.1] - 2026-04-20 + +Round-2 + Round-3 smoke-test response: 24 bugs closed across three groups โ€” +Round-2 correctness (13), Round-2 leftovers (3), and Round-3 contract & DX +(8). Sources: `switchbot-cli-v2.5.0-round2-report.md` and +`switchbot-cli-v2.5.0-round3-report.md`. + +The release was cut initially against the Round-2 report; the Round-3 report +arrived shortly after and is folded into the same patch so consumers of +2.5.1 get the full fix set in one version bump. The two Round-3 ๐Ÿ”ด items +(`#SYS-1`, `#SYS-3`) are contract bugs that break agent pipelines and could +not wait. + +This version also contains one **breaking change** โ€” the `--filter` grammar +is now unified across `devices list`, `devices batch`, and +`events tail` / `mqtt-tail`. `devices batch` and `events tail` keys that +used to require exact matches are now substrings. See +**Changed (BREAKING)** below for the migration. + +### Changed (BREAKING) + +- **`--filter` grammar unified across three surfaces** โ€” `devices list`, + `devices batch`, and `events tail` / `mqtt-tail` now share one DSL: + `key=value` (case-insensitive substring; exact only for `category`), + `key~value` (explicit case-insensitive substring), and + `key=/pattern/` (case-insensitive regex; invalid regex returns a usage + error). Each command still exposes its own key set โ€” see README + ยง"Filter expressions โ€” per-command reference". (bug #39) + - **Breaking**: `devices batch --filter 'type=Bot'` previously required + an exact match and now treats `Bot` as a substring (matches `Bot Plus` + too). Pair `=` with a more specific value, or filter post-hoc, if + exact match was load-bearing. + - **Breaking**: `devices batch --filter 'type~=...'` (the `~=` spelling) + is removed. Use `~` instead: `type~Light`. + - **Breaking**: `events tail --filter 'deviceId=ABC'` is now a substring + match (previously exact). + +### Fixed (correctness & safety) + +- **`devices command --dry-run --json` no longer emits empty stdout** โ€” + the single-device write path was hitting `handleError`'s silent + `DryRunSignal` exit before the JSON serializer ran. Now mirrors the MCP + `send_command {dryRun:true}` shape: + `{schemaVersion:"1.1", data:{dryRun:true, wouldSend:{deviceId,command,parameter,commandType}}}`. + Batch and plan dry-run paths were already correct. (bug #36) +- **MCP tool-call errors preserve structure** โ€” `send_command` / + `describe_device` / `run_scene` were letting `ApiError`s escape to the + SDK's generic `createToolError`, collapsing `{code, subKind, transient, + hint, retryAfterMs, errorClass}` to a plain-text string. Errors now + return `structuredContent.error` alongside `isError:true` so agents can + branch on `subKind` instead of parsing English. Also narrowed the + `mcpError()` option types so `subKind` / `errorClass` are compile-time + checked. (bug #38) +- **`devices batch` propagates `verification` + `subKind` for IR devices** โ€” + a batch over IR remotes was emitting zero unverifiability signal, the + exact contract 2.4.0 was released to establish. `succeeded[]` entries + now include `subKind:'ir-no-feedback'` and the verification object for + IR devices, plus `summary.unverifiableCount`. (bug #28) +- **Device & status cache scoped per profile** โ€” `devices.json` and + `status.json` lived at a fixed disk path, so rotating credentials or + switching profiles served the *prior* session's inventory. Cache files + now live under `~/.switchbot/cache//` when a profile + is active; unnamed/default profile keeps the legacy `~/.switchbot/` + path (backwards compatible). A follow-up fix also keys the in-memory + hot cache (`_listCache` / `_statusCache`) by profile so `mcp serve` + request-scoped profile switches do not leak either. (bug #37) +- **API code 190 reclassified `device-internal-error`** โ€” 190 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. Renamed + subKind + rewrote hint to cover all three causes. (bug #27) +- **API code 3005 mapped to `command-not-supported`** โ€” 3005 "invalid + value" is the API's catch-all for model-specific command rejections + (e.g., Fan `lowSpeed/middleSpeed/highSpeed` on stock IR remotes that + only work under `--type customize`). Now returns a useful subKind + hint + pointing to `devices commands ` and `--type customize`. (bug #29) +- **`scenes execute` pre-validates sceneId** โ€” `scenes execute ` + returned `ok:true` because the API does not validate sceneIds. + `scenes describe` already guarded against this via `scene_not_found` โ€” + port the same check so agents do not silently burn quota. (bug #31) +- **`devices meta set --alias` enforces uniqueness** โ€” nothing stopped + two devices from carrying the same alias; `--name ` behavior + 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. (bug #41) + +### Fixed (UX & docs) + +- **`--fields id` / `--fields name` aliases restored on `devices list`** โ€” + the 2.5.0 alias-map refactor dropped the short forms that 2.4.0 + accepted, breaking scripts. `id โ†’ deviceId` is back alongside + `name โ†’ deviceName`. (bug #22) +- **`cache clear --status` and `--list` shorthand aliases** โ€” the old + `--key status` form still works, but the shorter flags no longer + error with `unknown option`. Using them with `--key` or together + raises `UsageError`. (bug #35) +- **`history aggregate --metric` marked `requiredOption`** โ€” help text + said `(default: [])` implying optional; the command actually required + at least one metric and threw a custom error. Now Commander enforces + it and `--help` says `required`. (bug #42) +- **`plan validate` help text clarifies scope** โ€” now says "structural + only; does not verify device or scene existence" and points to + `plan run --dry-run` for semantic checks. (bug #32) +- **`cache` help text documents TTL behavior** โ€” the cache TTL is computed + from the `lastUpdated` field *inside* the JSON, not file mtime. + Operators who `touch`ed cache files to force a refresh were surprised. + One-line note added to `cache show --help`. (bug #34) +- **`devices meta` surfaced in agent-bootstrap and capabilities** โ€” the + local metadata system was completely undiscoverable in 2.5.0. `meta set + / get / list / clear` now appear in `capabilities` with correct safety + tiers, and `agent-bootstrap`'s `quickReference` gains a `meta` entry. + (bug #40) +- **`~/.switchbot/device-history/.json` companion file documented** โ€” + the 100-entry ring buffer read by MCP `get_device_history` had no docs, + while only the append-only `.jsonl` was mentioned. `docs/agent-guide.md` + now describes both files and `__control.jsonl`. (bug #43) + +### Fixed (Round 3 contract bugs โ€” ๐Ÿ”ด) + +- **`--json` errors now emit on stdout instead of stderr** โ€” piped + consumers (`cli --json ... | jq`) could not decode failure envelopes + because `handleError` wrote them to stderr. The JSON envelope + `{schemaVersion, error:{...}}` now lands on stdout for both success + and failure; TTY users still get a colored human-readable summary on + stderr, non-TTY invocations get silence on stderr. 15+ bespoke JSON + error sites across `batch`, `config`, `devices`, `expand`, `history`, + `mcp`, and `format` were consolidated through a new `emitJsonError` + helper. (bug #SYS-1) +- **MCP `send_command { dryRun:true }` validates deviceId against the + local cache** โ€” dryRun previously accepted any string and echoed back + a plausible-looking preview, defeating the whole point of a + validation surface. Unknown IDs now return `subKind:'device-not-found'` + with a hint to run `list_devices` first. Happy path unchanged for + cached IDs. (bug #SYS-3) + +### Fixed (Round 2 leftovers) + +- **`devices batch --idempotency-key`** accepted as alias for + `--idempotency-key-prefix`. Still uses prefix semantics internally + (auto-appends `-` per step). (bug #30) + +### Added (Round 2/3 features) + +- **`devices batch --skip-offline`** (default off) skips devices whose + cached status is offline, with each skip recorded under + `summary.skipped` with `skippedReason:'offline'`. Reads the local + status cache only โ€” no new API calls. Off by default preserves 2.5.0 + behavior. (bug #33) +- **`--for ` alias** on `devices watch`, `events tail`, and + `events mqtt-tail` โ€” stops after elapsed time instead of tick/event + count. Accepts the same duration grammar as `--since` (ms/s/m/h/d/w). + When both `--for` and `--max` are set, the first limit to hit wins. + (bug #52) +- **Duration parser accepts `d` (days) and `w` (weeks)** in addition + to `ms/s/m/h`. Unsupported units like `1y` / `1month` now produce a + usage error that lists the supported unit set. (bug #54) +- **`events mqtt-tail --json` emits a `__session_start` envelope** + immediately on invocation (before the broker connect), so downstream + tools can distinguish "connecting" from "never connected" and get an + eventId to correlate with subsequent `__connect` / `__disconnect` + events. (bug #56) + +### Polish (Round 3 DX) + +- **`--name-strategy` help + `agent-bootstrap` list all six + strategies** โ€” `exact`, `prefix`, `substring`, `fuzzy`, `first`, + `require-unique`. `ALL_STRATEGIES` in `name-resolver.ts` is the + single source of truth; help text is generated from it. (bug #51) +- **MCP `search_catalog` rejects empty queries** with a usage error + pointing to `list_catalog_types` for enumeration. Silent + "return everything" behavior was surprising and agent-hostile. + (bug #57) +- **Negative positional parameters reach the validation layer** โ€” + `setBrightness -1` was being swallowed by Commander as "unknown + option `-1`". `devices command` now uses `.passThroughOptions()` so + negative numeric positionals are forwarded to the command-specific + validator, where they can be accepted or range-rejected as + appropriate. (bug #53) + +### Not included (response to reports) + +- **Report bug #19 (MCP strict schema not enforced) โ€” false positive.** + All 11 MCP tools already have `.strict()` on their Zod input schemas + and the SDK enforces it via `safeParseAsync` โ†’ JSON-RPC `-32602`. + Could not reproduce the reported behavior; the existing test suite + exercises the full JSON-RPC path. +- **Deferred to 2.6.0:** + - Report bug #58 (parallel `devices status` outlier) โ€” needs + profiling to separate CLI-side latency from API-side, and the fix + likely involves a concurrency knob rather than a single flip. + - Report bug #55 (`devices watch --json` rewording) โ€” already works + via the global `--json` flag; pure doc rewording scheduled with + other doc sweeps. + - MCP / CLI naming alignment (`live` vs `includeStatus`, `metric` vs + `metrics`) flagged in Round-3 ยง4. + - `devices meta import/export` (Round-2 #40 follow-up). + ## [2.5.0] - 2026-04-20 ### Added diff --git a/README.md b/README.md index 71bfc52e..9afe076d 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,12 @@ switchbot devices list --filter category=physical switchbot devices list --filter type=Bot switchbot devices list --filter name=living,category=physical +# Filter operators: = (substring; exact for `category`), ~ (substring), +# =/regex/ (case-insensitive regex). Clauses are AND-ed. +switchbot devices list --filter 'name~living' +switchbot devices list --filter 'type=/Hub.*/' +switchbot devices list --filter 'name~office,type=/Bulb|Strip/' + # Filter by family / room (family & room info requires the 'src: OpenClaw' # header, which this CLI sends on every request) switchbot devices list --json | jq '.deviceList[] | select(.familyName == "Home")' @@ -255,6 +261,21 @@ switchbot devices commands "Smart Lock" switchbot devices commands curtain # Case-insensitive, substring match ``` +#### Filter expressions โ€” per-command reference + +Three commands accept `--filter`. They share one three-operator grammar, +but each exposes its own key set: + +| Command | Operators | Supported keys | +|-------------------------------------|-----------------------------------------------------------------------------------------------|---------------------------------------| +| `devices list` | `=` (substring; **exact** for `category`), `~` (substring), `=/regex/` (case-insensitive regex) | `type`, `name`, `category`, `room` | +| `devices batch` | same | `type`, `family`, `room`, `category` | +| `events tail` / `events mqtt-tail` | same (tail only; mqtt-tail uses `--topic` instead) | `deviceId`, `type` | + +Clauses are comma-separated and AND-ed. No OR across clauses โ€” use regex +alternation (`=/A|B/`) for that. `category` is the one key that stays exact +under `=` to preserve `category=physical` / `category=ir` semantics. + #### Parameter formats `parameter` is optional โ€” omit it for commands like `turnOn`/`turnOff` (auto-defaults to `"default"`). @@ -280,6 +301,8 @@ Generic parameter shapes (which one applies is decided by the device โ€” see the Parameters for `setAll` (Air Conditioner), `setPosition` (Curtain / Blind Tilt), and `setMode` (Relay Switch) are validated client-side before the request โ€” malformed shapes, out-of-range values, and JSON for CSV fields all fail fast with exit 2. Command names are also case-normalized against the catalog (e.g. `turnon` is auto-corrected to `turnOn` with a stderr warning); unknown names still exit 2 with the supported-commands list. +Negative numeric parameters (e.g. `setBrightness -1` for a probe) are passed through to the command validator instead of being swallowed by the flag parser as an unknown option. + For the complete per-device command reference, see the [SwitchBot API docs](https://github.com/OpenWonderLabs/SwitchBotAPI#send-device-control-commands). #### `devices expand` โ€” named flags for packed parameters @@ -333,7 +356,7 @@ Stores local annotations (alias, hidden flag, notes) in `~/.switchbot/device-met ```bash # Send the same command to every device matching a filter switchbot devices batch turnOff --filter 'type=Bot' -switchbot devices batch setBrightness 50 --filter 'type~=Light,family=Living' +switchbot devices batch setBrightness 50 --filter 'type~Light,family=Living' # Explicit device IDs (comma-separated) switchbot devices batch turnOn --ids ID1,ID2,ID3 @@ -343,9 +366,18 @@ switchbot devices list --format=id --filter 'type=Bot' | switchbot devices batch # Destructive commands require --yes switchbot devices batch unlock --filter 'type=Smart Lock' --yes + +# Skip devices whose cached status is offline (default: off) +switchbot devices batch turnOn --ids ID1,ID2 --skip-offline + +# --idempotency-key is an alias for --idempotency-key-prefix; both append - +switchbot devices batch turnOn --ids ID1,ID2 --idempotency-key morning-lights ``` -Sends the same command to many devices in one run. Uses the same `--filter` expressions as `devices list`. Destructive commands (Smart Lock unlock, Garage Door Opener, etc.) require `--yes` to prevent accidents. +Sends the same command to many devices in one run. Filter grammar matches `devices list` (`=` substring, `~` substring, `=/regex/` regex โ€” clauses AND-ed); supported keys here are `type`, `family`, `room`, `category`. Destructive commands (Smart Lock unlock, Garage Door Opener, etc.) require `--yes` to prevent accidents. + +`--skip-offline` reads from the local status cache only (no new API calls); +skipped devices appear under `summary.skipped` with `skippedReason:'offline'`. ### `scenes` โ€” run manual scenes @@ -390,6 +422,9 @@ switchbot events tail --filter deviceId=ABC123 # Stop after 5 matching events switchbot events tail --filter 'type=WoMeter' --max 5 +# Stop after 10 minutes regardless of event count +switchbot events tail --for 10m + # Custom port / path switchbot events tail --port 8080 --path /hook --json ``` @@ -401,7 +436,7 @@ Output (one JSON line per matched event): { "t": "2024-01-01T12:00:00.000Z", "remote": "1.2.3.4:54321", "path": "/", "body": {...}, "matched": true } ``` -Filter keys: `deviceId=`, `type=` (comma-separated for AND logic). +Filter keys: `deviceId`, `type`. Operators: `=` (substring), `~` (substring), `=/regex/` (case-insensitive regex). Clauses comma-separated and AND-ed. #### `events mqtt-tail` โ€” real-time MQTT stream @@ -414,6 +449,9 @@ switchbot events mqtt-tail --topic 'switchbot/#' # Stop after 10 events switchbot events mqtt-tail --max 10 --json + +# Stop after a fixed duration (emits __session_start under --json before connect) +switchbot events mqtt-tail --for 30s --json ``` Connects to the SwitchBot MQTT service automatically using the same credentials configured for the REST API (`SWITCHBOT_TOKEN` + `SWITCHBOT_SECRET`). No additional MQTT configuration is required โ€” the client certificates are provisioned on first use. @@ -514,9 +552,12 @@ switchbot devices watch # Custom interval; emit every tick even when nothing changed switchbot devices watch --interval 10s --include-unchanged --json + +# Time-bounded: stop after 5 minutes instead of a fixed tick count +switchbot devices watch --for 5m ``` -Output is a JSONL stream of status-change events (with `--json`) or a refreshed table. Use `--max ` to stop after N ticks. +Output is a JSONL stream of status-change events (with `--json`) or a refreshed table. Use `--max ` to stop after N ticks, or `--for ` to stop after an elapsed wall-clock window (e.g. `30s`, `1h`, `2d`). When both are set, whichever limit trips first wins. ### `mcp` โ€” Model Context Protocol server diff --git a/docs/agent-guide.md b/docs/agent-guide.md index 0bb5ad83..a8dc3885 100644 --- a/docs/agent-guide.md +++ b/docs/agent-guide.md @@ -89,6 +89,18 @@ Reads `~/.switchbot/device-history/.json` written by `events mqtt-tail **Workflow**: run `switchbot events mqtt-tail` in the background (e.g. with pm2) to keep the history files fresh; then call `get_device_history` from any MCP session without consuming REST quota. +#### Device-history directory layout + +After `events mqtt-tail` runs on a device, `~/.switchbot/device-history/` contains up to three companion files per device: + +| File | Description | +|------|-------------| +| `.jsonl` | Append-only, authoritative event log. Source of truth for `history range` and `history aggregate`. Rotated at ~50 MB (up to 3 segments). | +| `.json` | Latest 100-entry ring buffer. Written on every MQTT event. Read by MCP `get_device_history` for fast, zero-quota retrieval. | +| `__control.jsonl` | MQTT connection lifecycle events (heartbeat, connect, disconnect). Not a device log; used for diagnostics. | + +The `.json` file is **not** the source of truth for historical queries โ€” use `.jsonl` (via `history range` or `history aggregate`) when you need a complete, time-bounded record. The `.json` file is optimised for "what is the latest state?" lookups. + ### MCP resource: `switchbot://events` Read-only snapshot of recent MQTT shadow-update events from the ring buffer. Returns `{state, count, events[]}`. diff --git a/package-lock.json b/package-lock.json index e2d62225..eef6365d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@switchbot/openapi-cli", - "version": "2.5.0", + "version": "2.5.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@switchbot/openapi-cli", - "version": "2.5.0", + "version": "2.5.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", diff --git a/package.json b/package.json index c6367f63..1c179bcf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@switchbot/openapi-cli", - "version": "2.5.0", + "version": "2.5.1", "description": "SwitchBot smart home CLI โ€” control devices, run scenes, stream real-time events, and integrate AI agents via MCP. Full API v1.1 coverage.", "keywords": [ "switchbot", diff --git a/src/commands/agent-bootstrap.ts b/src/commands/agent-bootstrap.ts index 80f572c3..a7d65d0a 100644 --- a/src/commands/agent-bootstrap.ts +++ b/src/commands/agent-bootstrap.ts @@ -4,6 +4,7 @@ import { loadCache } from '../devices/cache.js'; import { getEffectiveCatalog } from '../devices/catalog.js'; import { readProfileMeta } from '../config.js'; import { todayUsage, DAILY_QUOTA } from '../utils/quota.js'; +import { ALL_STRATEGIES } from '../utils/name-resolver.js'; import { createRequire } from 'node:module'; const require = createRequire(import.meta.url); @@ -29,6 +30,7 @@ const QUICK_REFERENCE = { safety: ['--dry-run', '--idempotency-key ', '--audit-log', '--no-quota'], observability: ['doctor --json', 'quota status', 'cache status', 'events mqtt-tail'], history: ['history range --since 7d', 'history stats '], + meta: ['devices meta set --alias ', 'devices meta list', 'devices meta get '], }; interface BootstrapOptions { @@ -122,6 +124,7 @@ Examples: identity: IDENTITY, quickReference: QUICK_REFERENCE, safetyTiers: SAFETY_TIERS, + nameStrategies: [...ALL_STRATEGIES], profile: meta ? { label: meta.label ?? null, diff --git a/src/commands/batch.ts b/src/commands/batch.ts index 8ecfcdb2..59112f62 100644 --- a/src/commands/batch.ts +++ b/src/commands/batch.ts @@ -1,7 +1,7 @@ import { Command } from 'commander'; import type { AxiosInstance } from 'axios'; import { intArg, enumArg, stringArg } from '../utils/arg-parsers.js'; -import { printJson, isJsonMode, handleError, buildErrorPayload, type ErrorPayload } from '../utils/output.js'; +import { printJson, isJsonMode, handleError, buildErrorPayload, UsageError, emitJsonError, type ErrorPayload } from '../utils/output.js'; import { fetchDeviceList, executeCommand, @@ -12,7 +12,7 @@ import { createClient } from '../api/client.js'; import { parseFilter, applyFilter, FilterSyntaxError } from '../utils/filter.js'; import { isDryRun } from '../utils/flags.js'; import { DryRunSignal } from '../api/client.js'; -import { getCachedTypeMap } from '../devices/cache.js'; +import { getCachedTypeMap, getCachedDevice, loadStatusCache } from '../devices/cache.js'; interface BatchStepTiming { startedAt: string; @@ -22,14 +22,25 @@ interface BatchStepTiming { } interface BatchResult { - succeeded: Array<{ deviceId: string; result: unknown } & BatchStepTiming>; + succeeded: Array<{ + deviceId: string; + result: unknown; + subKind?: 'ir-no-feedback'; + verification?: { + verifiable: false; + reason: string; + suggestedFollowup: string; + }; + } & BatchStepTiming>; failed: Array<{ deviceId: string; error: ErrorPayload } & BatchStepTiming>; + skipped?: Array<{ deviceId: string; reason: 'offline' }>; summary: { total: number; ok: number; failed: number; skipped: number; durationMs: number; + unverifiableCount: number; dryRun?: boolean; schemaVersion?: string; maxConcurrent?: number; @@ -147,6 +158,8 @@ export function registerBatchCommand(devices: Command): void { .option('--type ', '"command" (default) or "customize" for user-defined IR buttons', enumArg('--type', COMMAND_TYPES), 'command') .option('--stdin', 'Read deviceIds from stdin, one per line (same as trailing "-")') .option('--idempotency-key-prefix ', 'Client-supplied prefix for idempotency keys (key per device: -). process-local 60s window; cache is per Node process (MCP session, batch run, plan run). Independent CLI invocations do not share cache.', stringArg('--idempotency-key-prefix')) + .option('--idempotency-key ', 'Alias for --idempotency-key-prefix.', stringArg('--idempotency-key')) + .option('--skip-offline', 'Skip devices whose cached status is offline (no API call; cache miss โ†’ send as usual).') .addHelpText('after', ` Targets are resolved in this priority order: 1. --ids when present (explicit deviceIds) @@ -201,12 +214,22 @@ Examples: type: string; stdin?: boolean; idempotencyKeyPrefix?: string; + idempotencyKey?: string; + skipOffline?: boolean; }, commandObj: Command ) => { // Trailing "-" sentinel selects stdin mode. const extra = commandObj.args ?? []; const readStdin = Boolean(options.stdin) || extra.includes('-'); + // Accept --idempotency-key as alias; reject when both forms are supplied. + if (options.idempotencyKey !== undefined && options.idempotencyKeyPrefix !== undefined) { + handleError(new UsageError('Use either --idempotency-key or --idempotency-key-prefix, not both.')); + return; + } + if (options.idempotencyKey !== undefined && options.idempotencyKeyPrefix === undefined) { + options.idempotencyKeyPrefix = options.idempotencyKey; + } let client: AxiosInstance | undefined; const getClient = (): AxiosInstance => (client ??= createClient()); @@ -220,7 +243,7 @@ Examples: } catch (error) { if (error instanceof FilterSyntaxError) { if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: error.message } })); + emitJsonError({ code: 2, kind: 'usage', message: error.message }); } else { console.error(`Error: ${error.message}`); } @@ -228,7 +251,7 @@ Examples: } if (error instanceof Error && error.message.startsWith('No target devices')) { if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: error.message } })); + emitJsonError({ code: 2, kind: 'usage', message: error.message }); } else { console.error(`Error: ${error.message}`); } @@ -241,7 +264,7 @@ Examples: const out: BatchResult = { succeeded: [], failed: [], - summary: { total: 0, ok: 0, failed: 0, skipped: 0, durationMs: 0 }, + summary: { total: 0, ok: 0, failed: 0, skipped: 0, durationMs: 0, unverifiableCount: 0 }, }; if (isJsonMode()) printJson(out); else console.log('No devices matched โ€” nothing to do.'); @@ -252,6 +275,24 @@ Examples: | 'command' | 'customize'; + // --skip-offline: preflight using the status cache (no network). Cache + // miss = send as usual; only definite "offline" cached entries skip. + const preSkipped: Array<{ deviceId: string; reason: 'offline' }> = []; + if (options.skipOffline && resolved.ids.length > 0) { + const statusCache = loadStatusCache(); + const kept: string[] = []; + for (const id of resolved.ids) { + const entry = statusCache.entries[id]; + const online = entry?.body?.onlineStatus; + if (online === 'offline') { + preSkipped.push({ deviceId: id, reason: 'offline' }); + } else { + kept.push(id); + } + } + resolved = { ...resolved, ids: kept }; + } + // Pre-flight: identify destructive targets before spending API calls. const blockedForDestructive: Array<{ deviceId: string; reason: string }> = []; for (const id of resolved.ids) { @@ -267,15 +308,13 @@ Examples: if (blockedForDestructive.length > 0 && !options.yes) { if (isJsonMode()) { const deviceIds = blockedForDestructive.map((b) => b.deviceId); - console.error(JSON.stringify({ - error: { - code: 2, - kind: 'guard', - message: `Destructive command "${cmd}" requires --yes to run on ${blockedForDestructive.length} device(s).`, - hint: 'Re-issue the call with --yes to proceed.', - context: { command: cmd, deviceIds }, - }, - })); + emitJsonError({ + code: 2, + kind: 'guard', + message: `Destructive command "${cmd}" requires --yes to run on ${blockedForDestructive.length} device(s).`, + hint: 'Re-issue the call with --yes to proceed.', + context: { command: cmd, deviceIds }, + }); } else { console.error( `Refusing to run destructive command "${cmd}" on ${blockedForDestructive.length} device(s) without --yes:` @@ -416,14 +455,26 @@ Examples: }>; const result: BatchResult = { - succeeded: succeeded.map((s) => ({ - deviceId: s.deviceId, - result: s.result, - startedAt: s.startedAt, - finishedAt: s.finishedAt, - durationMs: s.durationMs, - replayed: s.replayed, - })), + succeeded: succeeded.map((s) => { + const isIr = getCachedDevice(s.deviceId)?.category === 'ir'; + const entry: BatchResult['succeeded'][number] = { + deviceId: s.deviceId, + result: s.result, + startedAt: s.startedAt, + finishedAt: s.finishedAt, + durationMs: s.durationMs, + replayed: s.replayed, + }; + if (isIr) { + entry.subKind = 'ir-no-feedback'; + entry.verification = { + verifiable: false, + reason: 'IR transmission is unidirectional; no receipt acknowledgment is possible.', + suggestedFollowup: 'Confirm visible change manually or via a paired state sensor.', + }; + } + return entry; + }), failed: failed.map((f) => ({ deviceId: f.deviceId, error: f.error, @@ -431,12 +482,14 @@ Examples: finishedAt: f.finishedAt, durationMs: f.durationMs, })), + ...(preSkipped.length > 0 ? { skipped: preSkipped } : {}), summary: { - total: resolved.ids.length, + total: resolved.ids.length + preSkipped.length, ok: succeeded.length, failed: failed.length, - skipped: dryRunned.length, + skipped: dryRunned.length + preSkipped.length, durationMs: Date.now() - startedAt, + unverifiableCount: succeeded.filter((s) => getCachedDevice(s.deviceId)?.category === 'ir').length, schemaVersion: '1.1', maxConcurrent: concurrency, staggerMs, diff --git a/src/commands/cache.ts b/src/commands/cache.ts index 7bf65c3c..3b45151f 100644 --- a/src/commands/cache.ts +++ b/src/commands/cache.ts @@ -53,6 +53,10 @@ Examples: .command('show') .alias('status') .description('Summarize the cache files (paths, ages, entry counts)') + .addHelpText('after', ` +Cache TTL is computed from the 'lastUpdated' field inside the JSON, not the file mtime. +touch does not invalidate; use 'cache clear' to force a refresh. +`) .action(() => { const summary = describeCache(); if (isJsonMode()) { @@ -92,9 +96,20 @@ Examples: .command('clear') .description('Delete cache files') .option('--key ', 'Which cache to clear: "list" | "status" | "all" (default)', enumArg('--key', CACHE_KEYS), 'all') - .action((options: { key: string }) => { + .option('--status', 'Shorthand for --key status') + .option('--list', 'Shorthand for --key list') + .action((options: { key: string; status?: boolean; list?: boolean }) => { try { - const key = options.key; + if (options.status && options.list) { + throw new UsageError('--status and --list are mutually exclusive.'); + } + if ((options.status || options.list) && options.key !== 'all') { + throw new UsageError('--status / --list cannot be combined with --key.'); + } + let key = options.key; + if (options.status) key = 'status'; + if (options.list) key = 'list'; + if (!['list', 'status', 'all'].includes(key)) { throw new UsageError(`Unknown --key "${key}". Expected: list, status, all.`); } diff --git a/src/commands/capabilities.ts b/src/commands/capabilities.ts index d49e7b60..3c52f484 100644 --- a/src/commands/capabilities.ts +++ b/src/commands/capabilities.ts @@ -40,6 +40,11 @@ const COMMAND_META: Record = { 'devices types': { mutating: false, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'read', verifiability: 'local', typicalLatencyMs: 20 }, 'devices commands': { mutating: false, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'read', verifiability: 'local', typicalLatencyMs: 20 }, 'devices watch': { mutating: false, consumesQuota: true, idempotencySupported: false, agentSafetyTier: 'read', verifiability: 'local', typicalLatencyMs: 500 }, + // devices meta (local metadata โ€” no quota, no API call) + 'devices meta set': { mutating: true, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'action', verifiability: 'local', typicalLatencyMs: 5 }, + 'devices meta get': { mutating: false, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'read', verifiability: 'local', typicalLatencyMs: 5 }, + 'devices meta list': { mutating: false, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'read', verifiability: 'local', typicalLatencyMs: 5 }, + 'devices meta clear': { mutating: true, consumesQuota: false, idempotencySupported: false, agentSafetyTier: 'action', verifiability: 'local', typicalLatencyMs: 5 }, // devices: actions 'devices command': { mutating: true, consumesQuota: true, idempotencySupported: true, agentSafetyTier: 'action', verifiability: 'deviceDependent', typicalLatencyMs: 800 }, 'devices batch': { mutating: true, consumesQuota: true, idempotencySupported: true, agentSafetyTier: 'action', verifiability: 'deviceDependent', typicalLatencyMs: 1200 }, diff --git a/src/commands/config.ts b/src/commands/config.ts index 304228b3..c1188000 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -5,7 +5,7 @@ import { execFileSync } from 'node:child_process'; import { stringArg } from '../utils/arg-parsers.js'; import { intArg } from '../utils/arg-parsers.js'; import { saveConfig, showConfig, listProfiles, readProfileMeta } from '../config.js'; -import { isJsonMode, printJson } from '../utils/output.js'; +import { isJsonMode, printJson, emitJsonError } from '../utils/output.js'; import chalk from 'chalk'; function parseEnvFile(file: string): { token?: string; secret?: string } { @@ -164,7 +164,7 @@ Files are written with mode 0600. Profiles live under ~/.switchbot/profiles/', 'Freeform notes shown in "devices describe"', stringArg('--notes')) - .action((deviceId: string, options: { alias?: string; hide?: boolean; show?: boolean; notes?: string }) => { + .option('--force', 'Reassign alias even if it already belongs to another device') + .action((deviceId: string, options: { alias?: string; hide?: boolean; show?: boolean; notes?: string; force?: boolean }) => { try { if (options.hide && options.show) { throw new UsageError('--hide and --show cannot be used together.'); @@ -32,6 +34,27 @@ export function registerDevicesMetaCommand(devices: Command): void { throw new UsageError('Specify at least one of: --alias, --hide, --show, --notes'); } + // Enforce alias uniqueness across devices + if (options.alias !== undefined) { + const meta = loadDeviceMeta(); + const holder = Object.entries(meta.devices).find( + ([id, m]) => m.alias === options.alias && id !== deviceId, + ); + if (holder) { + if (!options.force) { + throw new UsageError( + `Alias "${options.alias}" is already assigned to device ${holder[0]}. Use --force to reassign.`, + ); + } + // --force: clear the alias from the previous holder + meta.devices[holder[0]] = { ...meta.devices[holder[0]], alias: undefined }; + saveDeviceMeta(meta); + if (!isJsonMode()) { + console.log(`(reassigned alias from ${holder[0]})`); + } + } + } + const patch: Record = {}; if (options.alias !== undefined) patch.alias = options.alias; if (options.notes !== undefined) patch.notes = options.notes; diff --git a/src/commands/devices.ts b/src/commands/devices.ts index 76a672d5..f15c7e4c 100644 --- a/src/commands/devices.ts +++ b/src/commands/devices.ts @@ -1,11 +1,11 @@ import { Command } from 'commander'; import { enumArg, stringArg } from '../utils/arg-parsers.js'; -import { printTable, printKeyValue, printJson, isJsonMode, handleError, UsageError } from '../utils/output.js'; +import { printTable, printKeyValue, printJson, isJsonMode, handleError, UsageError, emitJsonError } from '../utils/output.js'; import { resolveFormat, resolveFields, renderRows } from '../utils/format.js'; import { findCatalogEntry, getEffectiveCatalog, DeviceCatalogEntry } from '../devices/catalog.js'; import { getCachedDevice } from '../devices/cache.js'; import { loadDeviceMeta } from '../devices/device-meta.js'; -import { resolveDeviceId, NameResolveStrategy } from '../utils/name-resolver.js'; +import { resolveDeviceId, NameResolveStrategy, ALL_STRATEGIES } from '../utils/name-resolver.js'; import { fetchDeviceList, fetchDeviceStatus, @@ -25,6 +25,7 @@ import { registerExplainCommand } from './explain.js'; import { registerExpandCommand } from './expand.js'; import { registerDevicesMetaCommand } from './device-meta.js'; import { isDryRun } from '../utils/flags.js'; +import { DryRunSignal } from '../api/client.js'; export function registerDevicesCommand(program: Command): void { const COMMAND_TYPES = ['command', 'customize'] as const; @@ -87,10 +88,12 @@ Examples: $ switchbot devices list --filter type="Air Conditioner" $ switchbot devices list --filter category=ir $ switchbot devices list --filter name=living,category=physical + $ switchbot devices list --filter 'name~living' # explicit substring + $ switchbot devices list --filter 'type=/Air.*/' # regex (case-insensitive) `) .option('--wide', 'Show all columns (controlType, family, roomID, room, hub, cloud)') .option('--show-hidden', 'Include devices hidden via "devices meta set --hide"') - .option('--filter ', 'Filter devices: "type=X", "name=X", "category=physical|ir", "room=X" (comma-separated key=value pairs)', stringArg('--filter')) + .option('--filter ', 'Filter devices: comma-separated clauses. Each clause is "key=value" (substring; exact for category), "key~value" (explicit substring), or "key=/regex/" (case-insensitive regex). Supported keys: type, name, category, room.', stringArg('--filter')) .action(async (options: { wide?: boolean; showHidden?: boolean; filter?: string }) => { try { const body = await fetchDeviceList(); @@ -100,34 +103,86 @@ Examples: const hubLocation = buildHubLocationMap(deviceList); - // Parse --filter into a simple predicate map - interface ListFilter { type?: string; name?: string; category?: string; room?: string; } - let listFilter: ListFilter | null = null; + // Parse --filter into a list of clauses. Each comma-separated pair is + // one of three shapes: + // key=value โ€” current behavior (substring; exact for category) + // key~value โ€” explicit case-insensitive substring + // key=/pattern/ โ€” case-insensitive regex + interface FilterClause { + key: 'type' | 'name' | 'category' | 'room'; + op: 'eq' | 'sub' | 'regex'; + raw: string; + regex?: RegExp; + } + const SUPPORTED_KEYS = ['type', 'name', 'category', 'room'] as const; + let listClauses: FilterClause[] | null = null; if (options.filter) { - listFilter = {}; + listClauses = []; for (const pair of options.filter.split(',')) { - const eq = pair.indexOf('='); - if (eq === -1) throw new UsageError(`Invalid --filter pair "${pair.trim()}". Expected key=value.`); - const k = pair.slice(0, eq).trim(); - const v = pair.slice(eq + 1).trim(); - if (!['type', 'name', 'category', 'room'].includes(k)) { - throw new UsageError(`Unknown --filter key "${k}". Supported: type, name, category, room.`); + const trimmed = pair.trim(); + if (!trimmed) continue; + const regexMatch = /^([^=~]+)=\/(.*)\/$/.exec(trimmed); + const tildeIdx = trimmed.indexOf('~'); + const eqIdx = trimmed.indexOf('='); + let key: string; + let op: 'eq' | 'sub' | 'regex'; + let raw: string; + let regex: RegExp | undefined; + if (regexMatch) { + key = regexMatch[1].trim(); + op = 'regex'; + raw = regexMatch[2]; + try { + regex = new RegExp(raw, 'i'); + } catch (err) { + throw new UsageError( + `Invalid regex in --filter "${trimmed}": ${(err as Error).message}`, + ); + } + } else if (tildeIdx !== -1 && (eqIdx === -1 || tildeIdx < eqIdx)) { + key = trimmed.slice(0, tildeIdx).trim(); + op = 'sub'; + raw = trimmed.slice(tildeIdx + 1).trim().toLowerCase(); + } else if (eqIdx !== -1) { + key = trimmed.slice(0, eqIdx).trim(); + op = 'eq'; + raw = trimmed.slice(eqIdx + 1).trim().toLowerCase(); + } else { + throw new UsageError( + `Invalid --filter pair "${trimmed}". Expected key=value, key~value, or key=/regex/.`, + ); + } + if (!(SUPPORTED_KEYS as readonly string[]).includes(key)) { + throw new UsageError( + `Unknown --filter key "${key}". Supported: ${SUPPORTED_KEYS.join(', ')}.`, + ); } - (listFilter as Record)[k] = v.toLowerCase(); + listClauses.push({ key: key as FilterClause['key'], op, raw, regex }); } } const matchesFilter = (entry: { type: string; name: string; category: 'physical' | 'ir'; room: string }) => { - if (!listFilter) return true; - if (listFilter.type && !entry.type.toLowerCase().includes(listFilter.type)) return false; - if (listFilter.name && !entry.name.toLowerCase().includes(listFilter.name)) return false; - if (listFilter.category && entry.category !== listFilter.category) return false; - if (listFilter.room && !entry.room.toLowerCase().includes(listFilter.room)) return false; + if (!listClauses || listClauses.length === 0) return true; + for (const c of listClauses) { + const fieldVal = (entry as Record)[c.key] ?? ''; + const lower = fieldVal.toLowerCase(); + let ok: boolean; + if (c.op === 'regex') { + ok = c.regex!.test(fieldVal); + } else if (c.op === 'sub') { + ok = lower.includes(c.raw); + } else if (c.key === 'category') { + ok = lower === c.raw; + } else { + ok = lower.includes(c.raw); + } + if (!ok) return false; + } return true; }; if (fmt === 'json' && process.argv.includes('--json')) { - if (listFilter) { + if (listClauses) { const filteredDeviceList = deviceList.filter((d) => matchesFilter({ type: d.deviceType || '', name: d.deviceName, category: 'physical', room: d.roomName || '' }) ); @@ -186,20 +241,20 @@ Examples: } if (rows.length === 0 && fmt === 'table') { - console.log(listFilter ? 'No devices matched the filter.' : 'No devices found'); + console.log(listClauses ? 'No devices matched the filter.' : 'No devices found'); return; } const defaultFields = options.wide ? undefined : narrowHeaders; // Accept API field names and short aliases alongside canonical column names const DEVICE_LIST_ALIASES: Record = { - name: 'deviceName', deviceType: 'type', type: 'type', + id: 'deviceId', name: 'deviceName', deviceType: 'type', type: 'type', roomName: 'room', familyName: 'family', hubDeviceId: 'hub', enableCloudService: 'cloud', }; renderRows(wideHeaders, rows, fmt, userFields ?? defaultFields, DEVICE_LIST_ALIASES); if (fmt === 'table') { - const totalLabel = listFilter + const totalLabel = listClauses ? `${rows.length} match(es) (${deviceList.length} physical + ${infraredRemoteList.length} IR before filter)` : `${deviceList.length} physical device(s), ${infraredRemoteList.length} IR remote device(s)`; console.log(`\nTotal: ${totalLabel}`); @@ -216,7 +271,7 @@ Examples: .description('Query the real-time status of a specific device') .argument('[deviceId]', 'Device ID from "devices list" (or use --name or --ids)') .option('--name ', 'Resolve device by fuzzy name instead of deviceId', stringArg('--name')) - .option('--name-strategy ', 'Name match strategy: exact|prefix|substring|fuzzy|first|require-unique (default: fuzzy)', stringArg('--name-strategy')) + .option('--name-strategy ', `Name match strategy: ${ALL_STRATEGIES.join('|')} (default: fuzzy)`, stringArg('--name-strategy')) .option('--name-type ', 'Narrow --name by device type (e.g. "Bot", "Color Bulb")', stringArg('--name-type')) .option('--name-category ', 'Narrow --name by category: physical|ir', enumArg('--name-category', ['physical', 'ir'] as const)) .option('--name-room ', 'Narrow --name by room name (substring match)', stringArg('--name-room')) @@ -316,9 +371,10 @@ Examples: .description('Send a control command to a device') .argument('[deviceId]', 'Target device ID (or use --name)') .argument('[cmd]', 'Command name, e.g. turnOn, turnOff, setColor, setBrightness, setAll, startClean') - .argument('[parameter]', 'Command parameter. Omit for commands like turnOn/turnOff (defaults to "default"). Format depends on the command (see below).') + .argument('[parameter]', 'Command parameter. Omit for commands like turnOn/turnOff (defaults to "default"). Format depends on the command (see below). Negative numbers like -1 are accepted as-is (use `--` before them only if Commander mis-parses in your shell).') + .allowUnknownOption() .option('--name ', 'Resolve device by fuzzy name instead of deviceId', stringArg('--name')) - .option('--name-strategy ', 'Name match strategy: exact|prefix|substring|fuzzy|first|require-unique (default for command: require-unique)', stringArg('--name-strategy')) + .option('--name-strategy ', `Name match strategy: ${ALL_STRATEGIES.join('|')} (default for command: require-unique)`, stringArg('--name-strategy')) .option('--name-type ', 'Narrow --name by device type (e.g. "Bot", "Color Bulb")', stringArg('--name-type')) .option('--name-category ', 'Narrow --name by category: physical|ir', enumArg('--name-category', ['physical', 'ir'] as const)) .option('--name-room ', 'Narrow --name by room name (substring match)', stringArg('--name-room')) @@ -371,6 +427,10 @@ Examples: $ switchbot devices command unlock --yes `) .action(async (deviceIdArg: string | undefined, cmdArg: string | undefined, parameter: string | undefined, options: { name?: string; nameStrategy?: string; nameType?: string; nameCategory?: 'physical' | 'ir'; nameRoom?: string; type: string; yes?: boolean; idempotencyKey?: string }) => { + // Declared outside try so the DryRunSignal catch branch can reference them. + let _deviceId: string | undefined; + let _cmd: string | undefined; + let _parsedParam: unknown; try { // BUG-FIX: When --name is provided, Commander fills positionals left-to-right // starting at [deviceId]. Shift them back to their semantic slots. @@ -404,6 +464,7 @@ Examples: category: options.nameCategory, room: options.nameRoom, }); + _deviceId = deviceId; if (!getCachedDevice(deviceId)) { console.error( `Note: device ${deviceId} is not in the local cache โ€” run 'switchbot devices list' first to enable command validation.`, @@ -416,7 +477,7 @@ Examples: const obj: Record = { code: 2, kind: 'usage', message: err.message }; if (err.hint) obj.hint = err.hint; obj.context = { validationKind: err.kind }; - console.error(JSON.stringify({ error: obj })); + emitJsonError(obj); } else { console.error(`Error: ${err.message}`); if (err.hint) console.error(err.hint); @@ -451,14 +512,12 @@ Examples: const paramCheck = validateParameter(cachedForParam.type, cmd, parameter); if (!paramCheck.ok) { if (isJsonMode()) { - console.error(JSON.stringify({ - error: { - code: 2, - kind: 'usage', - message: paramCheck.error, - context: { command: cmd, deviceType: cachedForParam.type, deviceId }, - }, - })); + emitJsonError({ + code: 2, + kind: 'usage', + message: paramCheck.error, + context: { command: cmd, deviceType: cachedForParam.type, deviceId }, + }); } else { console.error(`Error: ${paramCheck.error}`); } @@ -476,17 +535,15 @@ Examples: const typeLabel = cachedForGuard?.type ?? 'unknown'; const reason = getDestructiveReason(cachedForGuard?.type, cmd, options.type); if (isJsonMode()) { - console.error(JSON.stringify({ - error: { - code: 2, - kind: 'guard', - message: `"${cmd}" on ${typeLabel} is destructive and requires --yes.`, - hint: reason - ? `Re-run with --yes to confirm. Reason: ${reason}` - : 'Re-run with --yes to confirm, or --dry-run to preview without sending.', - context: { command: cmd, deviceType: typeLabel, deviceId, ...(reason ? { destructiveReason: reason } : {}) }, - }, - })); + emitJsonError({ + code: 2, + kind: 'guard', + message: `"${cmd}" on ${typeLabel} is destructive and requires --yes.`, + hint: reason + ? `Re-run with --yes to confirm. Reason: ${reason}` + : 'Re-run with --yes to confirm, or --dry-run to preview without sending.', + context: { command: cmd, deviceType: typeLabel, deviceId, ...(reason ? { destructiveReason: reason } : {}) }, + }); } else { console.error( `Refusing to run destructive command "${cmd}" on ${typeLabel} without --yes.` @@ -513,6 +570,9 @@ Examples: // keep as string } } + // Capture for DryRunSignal catch branch (which runs after executeCommand throws). + _cmd = cmd; + _parsedParam = parsedParam; const body = await executeCommand( deviceId, @@ -558,6 +618,16 @@ Examples: // Re-throw mock process.exit signals (Vitest intercepts process.exit as thrown // Error('__exit__')) so they aren't double-handled and the exit code is preserved. if (error instanceof Error && error.message === '__exit__') throw error; + if (error instanceof DryRunSignal) { + const commandType = (options.type ?? 'command') as string; + const wouldSend = { deviceId: _deviceId, command: _cmd, parameter: _parsedParam, commandType }; + if (isJsonMode()) { + printJson({ dryRun: true, wouldSend }); + } else { + console.log(`[dry-run] Would POST devices/${_deviceId}/commands with ${JSON.stringify({ command: _cmd, parameter: _parsedParam, commandType })}`); + } + return; + } handleError(error); } }); @@ -648,7 +718,7 @@ Examples: .description('Describe a device by ID: metadata + supported commands + status fields (1 API call)') .argument('[deviceId]', 'Target device ID (or use --name)') .option('--name ', 'Resolve device by fuzzy name instead of deviceId', stringArg('--name')) - .option('--name-strategy ', 'Name match strategy: exact|prefix|substring|fuzzy|first|require-unique (default: fuzzy)', stringArg('--name-strategy')) + .option('--name-strategy ', `Name match strategy: ${ALL_STRATEGIES.join('|')} (default: fuzzy)`, stringArg('--name-strategy')) .option('--name-type ', 'Narrow --name by device type', stringArg('--name-type')) .option('--name-category ', 'Narrow --name by category: physical|ir', enumArg('--name-category', ['physical', 'ir'] as const)) .option('--name-room ', 'Narrow --name by room name (substring match)', stringArg('--name-room')) @@ -757,8 +827,19 @@ Examples: } } catch (error) { if (error instanceof DeviceNotFoundError) { - console.error(error.message); - console.error(`Try 'switchbot devices list' to see the full list.`); + const message = `${error.message} Try 'switchbot devices list' to see the full list.`; + if (isJsonMode()) { + emitJsonError({ + code: 1, + kind: 'runtime', + message, + errorClass: 'runtime', + transient: false, + }); + } else { + console.error(error.message); + console.error(`Try 'switchbot devices list' to see the full list.`); + } process.exit(1); } handleError(error); diff --git a/src/commands/events.ts b/src/commands/events.ts index 46913d09..0e18c28d 100644 --- a/src/commands/events.ts +++ b/src/commands/events.ts @@ -2,7 +2,9 @@ import { Command } from 'commander'; import http from 'node:http'; import crypto from 'node:crypto'; import { printJson, isJsonMode, handleError, UsageError } from '../utils/output.js'; -import { intArg, stringArg } from '../utils/arg-parsers.js'; +import { intArg, stringArg, durationArg } from '../utils/arg-parsers.js'; +import { parseDurationToMs } from '../utils/flags.js'; +import { parseFilterExpr, matchClause, FilterSyntaxError, type FilterClause } from '../utils/filter.js'; import { SwitchBotMqttClient } from '../mqtt/client.js'; import { fetchMqttCredential } from '../mqtt/credential.js'; import { tryLoadConfig } from '../config.js'; @@ -40,54 +42,47 @@ interface EventRecord { function matchFilter( body: unknown, - filter: { deviceId?: string; type?: string } | null, + clauses: FilterClause[] | null, ): boolean { - if (!filter) return true; + if (!clauses || clauses.length === 0) return true; if (!body || typeof body !== 'object') return false; const b = body as Record; const ctx = (b.context ?? b) as Record; - if (filter.deviceId && ctx.deviceMac !== filter.deviceId && ctx.deviceId !== filter.deviceId) { - return false; - } - if (filter.type && ctx.deviceType !== filter.type) { - return false; + for (const c of clauses) { + let candidate: string; + if (c.key === 'deviceId') { + const mac = ctx.deviceMac; + const id = ctx.deviceId; + candidate = String( + typeof mac === 'string' && mac ? mac : typeof id === 'string' ? id : '', + ); + } else { + const t = ctx.deviceType; + candidate = typeof t === 'string' ? t : ''; + } + if (!matchClause(candidate, c)) return false; } return true; } -function parseFilter(flag: string | undefined): { deviceId?: string; type?: string } | null { +const EVENT_FILTER_KEYS = ['deviceId', 'type'] as const; + +function parseFilter(flag: string | undefined): FilterClause[] | null { if (!flag) return null; - const allowed = new Set(['deviceId', 'type']); - const out: { deviceId?: string; type?: string } = {}; - for (const pair of flag.split(',')) { - const eq = pair.indexOf('='); - if (eq === -1 || eq === 0) { - throw new UsageError( - `Invalid --filter pair "${pair.trim()}". Expected "key=value". Supported keys: deviceId, type.` - ); - } - const k = pair.slice(0, eq).trim(); - const v = pair.slice(eq + 1).trim(); - if (!v) { - throw new UsageError( - `Empty value for --filter key "${k}". Expected "key=value". Supported keys: deviceId, type.` - ); - } - if (!allowed.has(k)) { - throw new UsageError( - `Unknown --filter key "${k}". Supported keys: deviceId, type.` - ); + try { + return parseFilterExpr(flag, EVENT_FILTER_KEYS); + } catch (e) { + if (e instanceof FilterSyntaxError) { + throw new UsageError(e.message); } - if (k === 'deviceId') out.deviceId = v; - else if (k === 'type') out.type = v; + throw e; } - return out; } export function startReceiver( port: number, pathMatch: string, - filter: { deviceId?: string; type?: string } | null, + filter: FilterClause[] | null, onEvent: (ev: EventRecord) => void, ): http.Server { const server = http.createServer((req, res) => { @@ -154,8 +149,9 @@ export function registerEventsCommand(program: Command): void { .description('Run a local HTTP receiver and print incoming webhook events as JSONL') .option('--port ', `Local port to listen on (default ${DEFAULT_PORT})`, intArg('--port', { min: 1, max: 65535 }), String(DEFAULT_PORT)) .option('--path

', `HTTP path to match (default "${DEFAULT_PATH}"; use "*" for all paths)`, stringArg('--path'), DEFAULT_PATH) - .option('--filter ', 'Filter events, e.g. "deviceId=ABC123" or "type=Bot" (comma-separated)', stringArg('--filter')) + .option('--filter ', 'Filter events by deviceId / type. Grammar: "key=value" (substring), "key~value" (substring), "key=/regex/" (regex). Comma-separated clauses are AND-ed.', stringArg('--filter')) .option('--max ', 'Stop after N matching events (default: run until Ctrl-C)', intArg('--max', { min: 1 })) + .option('--for ', 'Stop after elapsed time (e.g. "5m", "30s"). Combines with --max: first limit wins.', durationArg('--for')) .addHelpText( 'after', ` @@ -170,17 +166,23 @@ Output (JSONL, one event per line): { "t": "", "remote": "", "path": "/", "body": , "matched": true } -Filter grammar: comma-separated "key=value" pairs. Supported keys: - deviceId= match by context.deviceMac / context.deviceId - type= match by context.deviceType (e.g. "Bot", "WoMeter") +Filter grammar: comma-separated clauses (AND-ed). Each clause is one of + key=value โ€” case-insensitive substring + key~value โ€” explicit case-insensitive substring + key=/regex/ โ€” case-insensitive regex + +Supported keys: + deviceId match by context.deviceMac / context.deviceId + type match by context.deviceType (e.g. "Bot", "WoMeter") Examples: $ switchbot events tail --port 3000 $ switchbot events tail --port 3000 --filter deviceId=ABC123 - $ switchbot events tail --filter 'type=WoMeter' --max 5 --json + $ switchbot events tail --filter 'type~Meter' --max 5 --json + $ switchbot events tail --filter 'type=/Bot|Meter/' `, ) - .action(async (options: { port: string; path: string; filter?: string; max?: string }) => { + .action(async (options: { port: string; path: string; filter?: string; max?: string; for?: string }) => { try { const port = Number(options.port); if (!Number.isInteger(port) || port <= 0 || port > 65535) { @@ -190,10 +192,14 @@ Examples: if (maxMatched !== null && (!Number.isFinite(maxMatched) || maxMatched < 1)) { throw new UsageError(`Invalid --max "${options.max}". Must be a positive integer.`); } + const forMs = options.for ? parseDurationToMs(options.for) : null; const filter = parseFilter(options.filter); let matchedCount = 0; const ac = new AbortController(); + const forTimer = forMs !== null && forMs > 0 + ? setTimeout(() => ac.abort(), forMs) + : null; await new Promise((resolve, reject) => { let server: http.Server | null = null; try { @@ -220,6 +226,7 @@ Examples: if (!isJsonMode()) console.error(startMsg); const cleanup = () => { + if (forTimer) clearTimeout(forTimer); server?.close(); resolve(); }; @@ -237,6 +244,7 @@ Examples: .description('Subscribe to SwitchBot MQTT shadow events and stream them as JSONL') .option('--topic ', 'MQTT topic filter (default: SwitchBot shadow topic from credential)', stringArg('--topic')) .option('--max ', 'Stop after N events (default: run until Ctrl-C)', intArg('--max', { min: 1 })) + .option('--for ', 'Stop after elapsed time (e.g. "5m", "30s"). Combines with --max: first limit wins.', durationArg('--for')) .option( '--sink ', 'Output sink: stdout (default), file, webhook, openclaw, telegram, homeassistant (repeatable)', @@ -265,6 +273,7 @@ Output (JSONL, one event per line): { "t": "", "eventId": "", "topic": "", "payload": } Control records (interleaved, no "payload" field โ€” use type-prefix to filter): + { "type": "__session_start", "at": "", "eventId": "", "state": "connecting" } before credential fetch (JSON mode only) { "type": "__connect", "at": "", "eventId": "" } first successful connect { "type": "__reconnect", "at": "", "eventId": "" } connect after a disconnect { "type": "__disconnect", "at": "", "eventId": "" } reconnecting or failed @@ -300,6 +309,7 @@ Examples: .action(async (options: { topic?: string; max?: string; + for?: string; sink: string[]; sinkFile?: string; webhookUrl?: string; @@ -318,6 +328,7 @@ Examples: if (maxEvents !== null && (!Number.isInteger(maxEvents) || maxEvents < 1)) { throw new UsageError(`Invalid --max "${options.max}". Must be a positive integer.`); } + const forMs = options.for ? parseDurationToMs(options.for) : null; const loaded = tryLoadConfig(); if (!loaded) { @@ -376,11 +387,25 @@ Examples: if (!isJsonMode()) { console.error('Fetching MQTT credentials from SwitchBot serviceโ€ฆ'); } + // Emit a __session_start envelope immediately (before any credential + // fetch) so JSON consumers can distinguish "connecting" from "never + // connected" even when mqtt-tail exits before the broker connects. + if (isJsonMode()) { + printJson({ + type: '__session_start', + at: new Date().toISOString(), + eventId: crypto.randomUUID(), + state: 'connecting', + }); + } const credential = await fetchMqttCredential(loaded.token, loaded.secret); const topic = options.topic ?? credential.topics.status; let eventCount = 0; const ac = new AbortController(); + const forTimer = forMs !== null && forMs > 0 + ? setTimeout(() => ac.abort(), forMs) + : null; const client = new SwitchBotMqttClient( credential, () => fetchMqttCredential(loaded.token, loaded.secret), @@ -472,6 +497,7 @@ Examples: await new Promise((resolve) => { const cleanup = () => { + if (forTimer) clearTimeout(forTimer); process.removeListener('SIGINT', cleanup); process.removeListener('SIGTERM', cleanup); unsub(); diff --git a/src/commands/expand.ts b/src/commands/expand.ts index 8e26c878..3431ebbc 100644 --- a/src/commands/expand.ts +++ b/src/commands/expand.ts @@ -1,6 +1,6 @@ import { Command } from 'commander'; import { intArg, stringArg } from '../utils/arg-parsers.js'; -import { handleError, isJsonMode, printJson, UsageError } from '../utils/output.js'; +import { handleError, isJsonMode, printJson, UsageError, emitJsonError } from '../utils/output.js'; import { getCachedDevice } from '../devices/cache.js'; import { executeCommand, isDestructiveCommand, getDestructiveReason } from '../lib/devices.js'; import { isDryRun } from '../utils/flags.js'; @@ -115,10 +115,12 @@ Examples: if (!options.yes && !isDryRun() && isDestructiveCommand(deviceType, command, 'command')) { const reason = getDestructiveReason(deviceType, command, 'command'); if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'guard', + emitJsonError({ + code: 2, + kind: 'guard', message: `"${command}" on ${deviceType || 'device'} is destructive and requires --yes.`, hint: reason ? `Re-run with --yes. Reason: ${reason}` : 'Re-run with --yes to confirm.', - }})); + }); } else { console.error(`Refusing to run destructive command "${command}" without --yes.`); if (reason) console.error(`Reason: ${reason}`); diff --git a/src/commands/history.ts b/src/commands/history.ts index 88066952..84a54f7c 100644 --- a/src/commands/history.ts +++ b/src/commands/history.ts @@ -2,7 +2,7 @@ import { Command } from 'commander'; import path from 'node:path'; import os from 'node:os'; import { intArg, stringArg } from '../utils/arg-parsers.js'; -import { printJson, isJsonMode, handleError, UsageError } from '../utils/output.js'; +import { printJson, isJsonMode, handleError, UsageError, emitJsonError } from '../utils/output.js'; import { readAudit, verifyAudit, type AuditEntry } from '../utils/audit.js'; import { executeCommand } from '../lib/devices.js'; import { @@ -88,7 +88,7 @@ Examples: if (!Number.isInteger(idx) || idx < 1 || idx > entries.length) { const msg = `Invalid index ${indexArg}. Log has ${entries.length} entries.`; if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: msg } })); + emitJsonError({ code: 2, kind: 'usage', message: msg }); } else { console.error(msg); } @@ -98,7 +98,7 @@ Examples: if (entry.kind !== 'command') { const msg = `Entry ${idx} is not a command (kind=${entry.kind}).`; if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: msg } })); + emitJsonError({ code: 2, kind: 'usage', message: msg }); } else { console.error(msg); } @@ -280,7 +280,7 @@ Examples: .option('--since ', 'Relative window ending now, e.g. "1h", "7d" (mutually exclusive with --from/--to)', stringArg('--since')) .option('--from ', 'Range start (ISO-8601)', stringArg('--from')) .option('--to ', 'Range end (ISO-8601)', stringArg('--to')) - .option('--metric ', 'Payload field to aggregate (repeat for multiple)', (v: string, acc: string[] = []) => acc.concat(v), [] as string[]) + .requiredOption('--metric ', 'Payload field to aggregate (repeat for multiple; required)', (v: string, acc: string[] = []) => acc.concat(v)) .option('--agg ', 'Comma-separated aggregation functions (count,min,max,avg,sum,p50,p95)', stringArg('--agg')) .option('--bucket ', 'Bucket width, e.g. "15m", "1h", "1d"', stringArg('--bucket')) .option('--max-bucket-samples ', 'Max samples per bucket for quantiles (1โ€“100000)', intArg('--max-bucket-samples', { min: 1, max: 100_000 })) @@ -289,9 +289,6 @@ Examples: options: { since?: string; from?: string; to?: string; metric?: string[]; agg?: string; bucket?: string; maxBucketSamples?: string }, ) => { const metrics: string[] = options.metric ?? []; - if (metrics.length === 0) { - handleError(new UsageError('at least one --metric is required.')); - } if (options.since && (options.from || options.to)) { handleError(new UsageError('--since is mutually exclusive with --from/--to.')); diff --git a/src/commands/mcp.ts b/src/commands/mcp.ts index 1c32cb2d..241bd568 100644 --- a/src/commands/mcp.ts +++ b/src/commands/mcp.ts @@ -4,7 +4,7 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import { z } from 'zod'; import { intArg, stringArg } from '../utils/arg-parsers.js'; -import { handleError, isJsonMode } from '../utils/output.js'; +import { handleError, isJsonMode, buildErrorPayload, emitJsonError, type ErrorPayload, type ErrorSubKind } from '../utils/output.js'; import { VERSION } from '../version.js'; import { fetchDeviceList, @@ -50,18 +50,48 @@ function mcpError( kind: McpErrorKind, code: number, message: string, - options?: { hint?: string; retryable?: boolean; context?: Record }, + options?: { + hint?: string; + retryable?: boolean; + context?: Record; + subKind?: ErrorSubKind; + errorClass?: NonNullable; + transient?: boolean; + retryAfterMs?: number; + }, ) { const obj: Record = { code, kind, message }; if (options?.hint) obj.hint = options.hint; if (options?.retryable) obj.retryable = true; if (options?.context) obj.context = options.context; + if (options?.subKind !== undefined) obj.subKind = options.subKind; + if (options?.errorClass !== undefined) obj.errorClass = options.errorClass; + if (options?.transient !== undefined) obj.transient = options.transient; + if (options?.retryAfterMs !== undefined) obj.retryAfterMs = options.retryAfterMs; return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify({ error: obj }, null, 2) }], + structuredContent: { error: obj }, }; } +/** + * Convert any thrown error into a structured MCP tool-error response, + * preserving all ErrorPayload fields (subKind, transient, hint, etc.). + */ +function apiErrorToMcpError(err: unknown) { + const payload = buildErrorPayload(err); + return mcpError(payload.kind, payload.code, payload.message, { + hint: payload.hint, + retryable: payload.retryable, + context: payload.context, + subKind: payload.subKind, + errorClass: payload.errorClass, + transient: payload.transient, + retryAfterMs: payload.retryAfterMs, + }); +} + export function createSwitchBotMcpServer(options?: { eventManager?: EventSubscriptionManager }): McpServer { const eventManager = options?.eventManager; const server = new McpServer( @@ -316,8 +346,19 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, async ({ deviceId, command, parameter, commandType, confirm, idempotencyKey, dryRun }) => { const effectiveType = commandType ?? 'command'; - // dryRun early-return โ€” no API call, no validation against live device list + // dryRun early-return โ€” no API call. We still preflight the deviceId + // against the local cache so fabricated IDs don't silently pass + // validation (bug #SYS-3). Dry-run is meant to catch bad inputs; a + // dry-run that accepts anything is worse than no dry-run at all. if (dryRun) { + const cached = getCachedDevice(deviceId); + if (!cached) { + return mcpError('usage', 2, `Device "${deviceId}" not found in local cache.`, { + subKind: 'device-not-found', + hint: "Run 'list_devices' first to warm the cache, then retry with dryRun:true.", + context: { deviceId }, + }); + } const wouldSend = { deviceId, command, @@ -400,7 +441,7 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, }, }); } - throw err; + return apiErrorToMcpError(err); } const isIr = getCachedDevice(deviceId)?.category === 'ir'; const structured: { @@ -460,7 +501,11 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, structuredContent: structured, }; } - await executeScene(sceneId); + try { + await executeScene(sceneId); + } catch (err) { + return apiErrorToMcpError(err); + } const structured = { ok: true as const, sceneId }; return { content: [{ type: 'text', text: JSON.stringify(structured, null, 2) }], @@ -499,7 +544,7 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, 'Search the built-in device catalog by type name or alias. Returns matching entries with their commands, roles, destructive flags, and status fields. No API call.', _meta: { agentSafetyTier: 'read' }, inputSchema: z.object({ - query: z.string().describe('Search query (matches type and aliases, case-insensitive). Use empty string to list all.'), + query: z.string().describe('Search query (matches type and aliases, case-insensitive). Must be non-empty; use list_catalog_types to enumerate instead.'), limit: z.number().int().min(1).max(100).optional().default(20).describe('Max entries returned (default 20)'), }).strict(), outputSchema: { @@ -523,6 +568,16 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, }, }, async ({ query, limit }) => { + if (query.trim() === '') { + return mcpError( + 'usage', + 2, + 'search_catalog requires a non-empty query.', + { + hint: "Pass a search term like 'Bot' or 'Hub', or call list_catalog_types to enumerate all types without a query.", + }, + ); + } const hits = searchCatalog(query, limit); const structured = { results: hits as unknown as Array>, total: hits.length }; return { @@ -578,7 +633,7 @@ API docs: https://github.com/OpenWonderLabs/SwitchBotAPI`, context: { deviceId }, }); } - throw err; + return apiErrorToMcpError(err); } } ); @@ -807,7 +862,7 @@ Inspect locally: if (!Number.isFinite(port) || port < 1 || port > 65535) { const msg = `Invalid --port "${options.port}". Must be 1-65535.`; if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: msg } })); + emitJsonError({ code: 2, kind: 'usage', message: msg }); } else { console.error(msg); } @@ -824,7 +879,7 @@ Inspect locally: if (!isLocalhost && !authToken) { const msg = 'Refusing to listen on 0.0.0.0 without --auth-token. Pass --auth-token or bind to localhost (default).'; if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: msg } })); + emitJsonError({ code: 2, kind: 'usage', message: msg }); } else { console.error(msg); } diff --git a/src/commands/plan.ts b/src/commands/plan.ts index 5621b51f..16e79f61 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -251,8 +251,13 @@ Workflow: plan .command('validate') - .description('Validate a plan file (or stdin) against the schema') + .description('Validate a plan file (or stdin) against the schema (structural only; does not verify device or scene existence)') .argument('[file]', 'Path to plan.json, or "-" / omit to read stdin') + .addHelpText('after', ` +To check semantic validity (e.g., that deviceIds and sceneIds actually exist), +use 'plan run --dry-run' which exercises name resolution and device lookup +against the live API without executing any mutations. +`) .action(async (file: string | undefined) => { let raw: unknown; try { diff --git a/src/commands/scenes.ts b/src/commands/scenes.ts index 47bf6f74..fc1859f8 100644 --- a/src/commands/scenes.ts +++ b/src/commands/scenes.ts @@ -58,6 +58,15 @@ Example: `) .action(async (sceneId: string) => { try { + const sceneList = await fetchScenes(); + const found = sceneList.find((s) => s.sceneId === sceneId); + if (!found) { + throw new StructuredUsageError(`scene not found: ${sceneId}`, { + error: 'scene_not_found', + sceneId, + candidates: sceneList.map((s) => ({ sceneId: s.sceneId, sceneName: s.sceneName })), + }); + } await executeScene(sceneId); if (isJsonMode()) { printJson({ ok: true, sceneId }); diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 1192dd84..a2869488 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -81,6 +81,7 @@ export function registerWatchCommand(devices: Command): void { '30s', ) .option('--max ', 'Stop after N ticks (default: run until Ctrl-C)', intArg('--max', { min: 1 })) + .option('--for ', 'Stop after elapsed time (e.g. "5m", "30s"). Combines with --max: first limit wins.', durationArg('--for')) .option('--include-unchanged', 'Emit a tick even when no field changed') .addHelpText( 'after', @@ -106,6 +107,7 @@ Examples: name?: string; interval: string; max?: string; + for?: string; includeUnchanged?: boolean; }, ) => { @@ -133,12 +135,17 @@ Examples: maxTicks = Math.floor(n); } + const forMs = options.for ? parseDurationToMs(options.for) : null; + const fields: string[] | null = getFields() ?? null; const ac = new AbortController(); const onSig = () => ac.abort(); process.on('SIGINT', onSig); process.on('SIGTERM', onSig); + const forTimer = forMs !== null && forMs > 0 + ? setTimeout(() => ac.abort(), forMs) + : null; try { const prev = new Map>(); @@ -196,6 +203,7 @@ Examples: } catch (err) { handleError(err); } finally { + if (forTimer) clearTimeout(forTimer); process.off('SIGINT', onSig); process.off('SIGTERM', onSig); } diff --git a/src/devices/cache.ts b/src/devices/cache.ts index e428ef09..1a4c9943 100644 --- a/src/devices/cache.ts +++ b/src/devices/cache.ts @@ -1,7 +1,30 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; +import { createHash } from 'node:crypto'; import { getConfigPath } from '../utils/flags.js'; +import { getActiveProfile } from '../lib/request-context.js'; + +/** + * Returns the directory where cache files should be stored. + * + * - If a profile is active, scopes into a per-profile sub-directory so that + * rotating credentials or switching profiles never serves stale inventory + * from a prior session (Bug #37). + * - If no profile is active (unnamed / default), returns `baseDir` unchanged + * so the existing legacy path (~/.switchbot/devices.json) is preserved. + * + * Only called when `getConfigPath()` returns undefined โ€” the --config-path + * override takes full precedence and bypasses this helper entirely. + */ +function scopedCacheDir(baseDir: string): string { + const profile = getActiveProfile(); + if (profile === undefined) return baseDir; + const hash = createHash('sha256').update(profile).digest('hex').slice(0, 8); + const dir = path.join(baseDir, 'cache', hash); + if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); + return dir; +} /** GC cutoff for status entries: evict anything older than this. */ const DEFAULT_STATUS_GC_TTL_MS = 24 * 60 * 60 * 1000; // 24 h @@ -48,42 +71,50 @@ function cacheFilePath(): string { const override = getConfigPath(); const dir = override ? path.dirname(path.resolve(override)) - : path.join(os.homedir(), '.switchbot'); + : scopedCacheDir(path.join(os.homedir(), '.switchbot')); return path.join(dir, 'devices.json'); } -// In-memory hot-cache: undefined = not yet loaded, null = loaded but empty. -let _listCache: DeviceCache | null | undefined = undefined; -let _statusCache: StatusCache | undefined = undefined; +// In-memory hot-cache keyed by active profile (or '__default__' for no profile). +// Using Maps instead of module-level singletons ensures that mcp serve, which +// rotates profiles per request via withRequestContext, never leaks inventory +// across profiles within the same process (Bug #37). +const _listCacheByProfile = new Map(); +const _statusCacheByProfile = new Map(); + +function cacheKey(): string { + return getActiveProfile() ?? '__default__'; +} /** Force the next loadCache() call to re-read from disk. Used in tests. */ export function resetListCache(): void { - _listCache = undefined; + _listCacheByProfile.clear(); } /** Force the next loadStatusCache() call to re-read from disk. Used in tests. */ export function resetStatusCache(): void { - _statusCache = undefined; + _statusCacheByProfile.clear(); } export function loadCache(): DeviceCache | null { - if (_listCache !== undefined) return _listCache; + const key = cacheKey(); + if (_listCacheByProfile.has(key)) return _listCacheByProfile.get(key)!; const file = cacheFilePath(); if (!fs.existsSync(file)) { - _listCache = null; + _listCacheByProfile.set(key, null); return null; } try { const raw = fs.readFileSync(file, 'utf-8'); const cache = JSON.parse(raw) as DeviceCache; if (!cache || typeof cache.devices !== 'object' || cache.devices === null) { - _listCache = null; + _listCacheByProfile.set(key, null); return null; } - _listCache = cache; + _listCacheByProfile.set(key, cache); return cache; } catch { - _listCache = null; + _listCacheByProfile.set(key, null); return null; } } @@ -152,7 +183,7 @@ export function updateCacheFromDeviceList(body: DeviceListBodyShape): void { const dir = path.dirname(file); if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); fs.writeFileSync(file, JSON.stringify(cache, null, 2), { mode: 0o600 }); - _listCache = cache; + _listCacheByProfile.set(cacheKey(), cache); } catch { // Cache write failures must not break the command that triggered them. } @@ -161,7 +192,7 @@ export function updateCacheFromDeviceList(body: DeviceListBodyShape): void { export function clearCache(): void { const file = cacheFilePath(); if (fs.existsSync(file)) fs.unlinkSync(file); - _listCache = null; + _listCacheByProfile.set(cacheKey(), null); } // ---- Device list freshness ------------------------------------------------- @@ -205,34 +236,38 @@ function statusCacheFilePath(): string { const override = getConfigPath(); const dir = override ? path.dirname(path.resolve(override)) - : path.join(os.homedir(), '.switchbot'); + : scopedCacheDir(path.join(os.homedir(), '.switchbot')); return path.join(dir, 'status.json'); } export function loadStatusCache(): StatusCache { - if (_statusCache !== undefined) return _statusCache; + const key = cacheKey(); + if (_statusCacheByProfile.has(key)) return _statusCacheByProfile.get(key)!; const file = statusCacheFilePath(); if (!fs.existsSync(file)) { - _statusCache = { entries: {} }; - return _statusCache; + const empty = { entries: {} }; + _statusCacheByProfile.set(key, empty); + return empty; } try { const raw = fs.readFileSync(file, 'utf-8'); const parsed = JSON.parse(raw) as StatusCache; if (!parsed || typeof parsed.entries !== 'object' || parsed.entries === null) { - _statusCache = { entries: {} }; - return _statusCache; + const empty = { entries: {} }; + _statusCacheByProfile.set(key, empty); + return empty; } - _statusCache = parsed; + _statusCacheByProfile.set(key, parsed); return parsed; } catch { - _statusCache = { entries: {} }; - return _statusCache; + const empty = { entries: {} }; + _statusCacheByProfile.set(key, empty); + return empty; } } function saveStatusCache(cache: StatusCache): void { - _statusCache = cache; + _statusCacheByProfile.set(cacheKey(), cache); try { const file = statusCacheFilePath(); const dir = path.dirname(file); @@ -287,7 +322,7 @@ export function setCachedStatus( export function clearStatusCache(): void { const file = statusCacheFilePath(); if (fs.existsSync(file)) fs.unlinkSync(file); - _statusCache = { entries: {} }; + _statusCacheByProfile.set(cacheKey(), { entries: {} }); } /** Summary for `switchbot cache show`. */ diff --git a/src/utils/arg-parsers.ts b/src/utils/arg-parsers.ts index d2cdbca4..13284891 100644 --- a/src/utils/arg-parsers.ts +++ b/src/utils/arg-parsers.ts @@ -48,7 +48,8 @@ export function durationArg(flagName: string): (value: string) => string { const ms = parseDurationToMs(value); if (ms === null) { throw new InvalidArgumentError( - `${flagName} must look like "30s", "1m", "500ms", "1h" (got "${value}")`, + `${flagName} must look like "30s", "1m", "500ms", "1h", "7d", "2w" ` + + `(supported units: ms, s, m, h, d, w โ€” got "${value}")`, ); } return value; diff --git a/src/utils/filter.ts b/src/utils/filter.ts index b85755d4..f55e293c 100644 --- a/src/utils/filter.ts +++ b/src/utils/filter.ts @@ -2,12 +2,21 @@ import type { Device, InfraredDevice } from '../lib/devices.js'; /** * A parsed filter clause. Each clause is an (op, key, value) triple that runs - * against a candidate device. All clauses from a single expression are AND-ed. + * against a candidate string. All clauses from a single expression are AND-ed. + * + * Three operators (shared across `devices list`, `devices batch`, + * `events tail` / `mqtt-tail`): + * key=value โ€” case-insensitive substring (exact for `category`) + * key~value โ€” explicit case-insensitive substring + * key=/pattern/ โ€” case-insensitive regex */ +export type FilterOp = 'eq' | 'sub' | 'regex'; + export interface FilterClause { - key: 'type' | 'family' | 'room' | 'category'; - op: '=' | '~='; - value: string; + key: string; + op: FilterOp; + raw: string; + regex?: RegExp; } export class FilterSyntaxError extends Error { @@ -17,49 +26,128 @@ export class FilterSyntaxError extends Error { } } -const VALID_KEYS: FilterClause['key'][] = ['type', 'family', 'room', 'category']; - /** - * Parse a filter expression like "type=Bot,family=Home" into discrete clauses. + * Parse a comma-separated filter expression into discrete clauses. * - * Grammar: - * expr := clause ("," clause)* - * clause := KEY OP VALUE - * KEY := type | family | room | category - * OP := "=" | "~=" - * VALUE := any non-empty string (no comma โ€” split at the clause boundary) + * Grammar (per clause, recognition order): + * 1. key=/pattern/ โ†’ regex (case-insensitive); invalid regex throws. + * 2. key~value โ†’ substring (case-insensitive). + * 3. key=value โ†’ 'eq' op (substring; caller decides whether to treat + * as exact for specific keys via matchClause's + * `exactKeys` option). * - * Whitespace around keys / values is trimmed. Empty expressions return []. + * `allowedKeys` is command-specific: `devices list` uses + * {type,name,category,room}; `devices batch` uses {type,family,room,category}; + * `events tail` uses {deviceId,type}. */ -export function parseFilter(expr: string | undefined): FilterClause[] { +export function parseFilterExpr( + expr: string | undefined, + allowedKeys: readonly string[], +): FilterClause[] { if (!expr) return []; const parts = expr.split(',').map((p) => p.trim()).filter((p) => p.length > 0); const clauses: FilterClause[] = []; for (const part of parts) { - const m = /^([a-zA-Z_]+)\s*(~=|=)\s*(.+)$/.exec(part); - if (!m) { + const regexMatch = /^([^=~]+)=\/(.*)\/$/.exec(part); + const tildeIdx = part.indexOf('~'); + const eqIdx = part.indexOf('='); + + let key: string; + let op: FilterOp; + let raw: string; + let regex: RegExp | undefined; + + if (regexMatch) { + key = regexMatch[1].trim(); + op = 'regex'; + raw = regexMatch[2]; + try { + regex = new RegExp(raw, 'i'); + } catch (err) { + throw new FilterSyntaxError( + `Invalid regex in --filter "${part}": ${(err as Error).message}`, + ); + } + } else if (tildeIdx !== -1 && (eqIdx === -1 || tildeIdx < eqIdx)) { + key = part.slice(0, tildeIdx).trim(); + op = 'sub'; + raw = part.slice(tildeIdx + 1).trim(); + if (raw.startsWith('=')) { + throw new FilterSyntaxError( + `Invalid filter clause "${part}" โ€” "~=" is no longer supported. Use "${key}~${raw.slice(1)}" instead.`, + ); + } + } else if (eqIdx !== -1) { + key = part.slice(0, eqIdx).trim(); + op = 'eq'; + raw = part.slice(eqIdx + 1).trim(); + } else { throw new FilterSyntaxError( - `Invalid filter clause "${part}" โ€” expected "=" or "~="` + `Invalid filter clause "${part}" โ€” expected "=", "~", or "=//"`, ); } - const key = m[1] as FilterClause['key']; - const op = m[2] as FilterClause['op']; - const value = m[3].trim(); - if (!VALID_KEYS.includes(key)) { - throw new FilterSyntaxError( - `Unknown filter key "${key}" โ€” supported: ${VALID_KEYS.join(', ')}` - ); + + if (!key) { + throw new FilterSyntaxError(`Empty key in filter clause "${part}"`); } - if (!value) { + if (!raw) { throw new FilterSyntaxError(`Empty value for filter clause "${part}"`); } - clauses.push({ key, op, value }); + if (!allowedKeys.includes(key)) { + throw new FilterSyntaxError( + `Unknown filter key "${key}" โ€” supported: ${allowedKeys.join(', ')}`, + ); + } + + clauses.push({ key, op, raw, regex }); } return clauses; } +/** + * Match a single candidate string against a clause. + * + * - `regex` โ†’ RegExp.test against the candidate (case-insensitive by construction). + * - `sub` โ†’ case-insensitive substring. + * - `eq` โ†’ case-insensitive substring, except for keys listed in + * `exactKeys`, which get case-insensitive exact comparison. + * Default `exactKeys` is `['category']` to preserve the existing + * list/batch behavior for that key. + */ +export function matchClause( + candidate: string | undefined, + clause: FilterClause, + options?: { exactKeys?: readonly string[] }, +): boolean { + if (candidate === undefined) return false; + if (clause.op === 'regex') { + return clause.regex!.test(candidate); + } + const cLower = candidate.toLowerCase(); + const vLower = clause.raw.toLowerCase(); + if (clause.op === 'sub') { + return cLower.includes(vLower); + } + const exactKeys = options?.exactKeys ?? ['category']; + if (exactKeys.includes(clause.key)) { + return cLower === vLower; + } + return cLower.includes(vLower); +} + +const BATCH_KEYS = ['type', 'family', 'room', 'category'] as const; + +/** + * Back-compat narrow signature: parses with the batch key set. Callers that + * need a different key set (list, events tail) should call parseFilterExpr + * directly. + */ +export function parseFilter(expr: string | undefined): FilterClause[] { + return parseFilterExpr(expr, BATCH_KEYS); +} + interface FilterableDevice { deviceId: string; type: string; @@ -72,7 +160,7 @@ interface FilterableDevice { function toFilterable( d: Device | InfraredDevice, isPhysical: boolean, - hubLocation?: Map + hubLocation?: Map, ): FilterableDevice { if (isPhysical) { const p = d as Device; @@ -95,35 +183,30 @@ function toFilterable( }; } -function matches(d: FilterableDevice, clause: FilterClause): boolean { - const candidate: string | undefined = - clause.key === 'type' - ? d.type - : clause.key === 'family' - ? d.family - : clause.key === 'room' - ? d.room - : d.category; - if (candidate === undefined) return false; - - if (clause.op === '=') return candidate.toLowerCase() === clause.value.toLowerCase(); - - // '~=' โ€” case-insensitive substring match on the candidate. - return candidate.toLowerCase().includes(clause.value.toLowerCase()); +function candidateFor(d: FilterableDevice, key: string): string | undefined { + switch (key) { + case 'type': + return d.type; + case 'family': + return d.family; + case 'room': + return d.room; + case 'category': + return d.category; + default: + return undefined; + } } /** * Apply the parsed clauses to a mixed list of physical devices + IR remotes. - * Returns the deviceIds of the entries that satisfy every clause. - * - * `hubLocation` (optional) allows family/room filters to match IR remotes by - * the Hub-inherited location. + * Returns the filterable entries that satisfy every clause. */ export function applyFilter( clauses: FilterClause[], deviceList: Device[], infraredRemoteList: InfraredDevice[], - hubLocation?: Map + hubLocation?: Map, ): FilterableDevice[] { const candidates: FilterableDevice[] = [ ...deviceList.map((d) => toFilterable(d, true)), @@ -131,5 +214,7 @@ export function applyFilter( ]; if (clauses.length === 0) return candidates; - return candidates.filter((c) => clauses.every((clause) => matches(c, clause))); + return candidates.filter((c) => + clauses.every((clause) => matchClause(candidateFor(c, clause.key), clause)), + ); } diff --git a/src/utils/flags.ts b/src/utils/flags.ts index 28876d7d..2e42e817 100644 --- a/src/utils/flags.ts +++ b/src/utils/flags.ts @@ -112,7 +112,7 @@ export interface CacheMode { const DEFAULT_LIST_TTL_MS = 60 * 60 * 1000; function parseDurationToMs(v: string): number | null { - const m = /^(\d+)(ms|s|m|h)?$/.exec(v.trim().toLowerCase()); + const m = /^(\d+)(ms|s|m|h|d|w)?$/.exec(v.trim().toLowerCase()); if (!m) return null; const n = Number(m[1]); if (!Number.isFinite(n) || n < 0) return null; @@ -122,6 +122,8 @@ function parseDurationToMs(v: string): number | null { case 's': return n * 1000; case 'm': return n * 60 * 1000; case 'h': return n * 60 * 60 * 1000; + case 'd': return n * 24 * 60 * 60 * 1000; + case 'w': return n * 7 * 24 * 60 * 60 * 1000; default: return null; } } diff --git a/src/utils/format.ts b/src/utils/format.ts index ff2e6e36..e44a731e 100644 --- a/src/utils/format.ts +++ b/src/utils/format.ts @@ -1,4 +1,4 @@ -import { printTable, printJson, isJsonMode, UsageError } from './output.js'; +import { printTable, printJson, isJsonMode, UsageError, emitJsonError } from './output.js'; import { getFormat, getFields } from './flags.js'; import { dump as yamlDump } from 'js-yaml'; @@ -18,7 +18,7 @@ export function parseFormat(flag: string | undefined): OutputFormat { default: { const msg = `Unknown --format "${flag}". Expected: table, json, jsonl, tsv, yaml, id, markdown.`; if (isJsonMode()) { - console.error(JSON.stringify({ error: { code: 2, kind: 'usage', message: msg } })); + emitJsonError({ code: 2, kind: 'usage', message: msg }); } else { console.error(msg); } diff --git a/src/utils/name-resolver.ts b/src/utils/name-resolver.ts index 0ed87fed..34f1b98a 100644 --- a/src/utils/name-resolver.ts +++ b/src/utils/name-resolver.ts @@ -29,9 +29,9 @@ export type NameResolveResult = | { ok: false; ambiguous: true; candidates: NameMatch[] } | { ok: false; ambiguous: false }; -const ALL_STRATEGIES: NameResolveStrategy[] = [ +export const ALL_STRATEGIES: readonly NameResolveStrategy[] = [ 'exact', 'prefix', 'substring', 'fuzzy', 'first', 'require-unique', -]; +] as const; export function isValidStrategy(s: string): s is NameResolveStrategy { return (ALL_STRATEGIES as string[]).includes(s); diff --git a/src/utils/output.ts b/src/utils/output.ts index aa50433e..b4d0428a 100644 --- a/src/utils/output.ts +++ b/src/utils/output.ts @@ -14,6 +14,26 @@ export function printJson(data: unknown): void { console.log(JSON.stringify({ schemaVersion: SCHEMA_VERSION, data }, null, 2)); } +/** + * Emit a structured JSON error envelope on stdout. + * + * Bug #SYS-1: Under `--json`, both success and error payloads must share + * the same output channel (stdout) so a single `cli --json ... | jq` pipe + * can decode either shape. Use this helper everywhere that previously + * called `console.error(JSON.stringify({ error: ... }))` in --json mode. + * + * The envelope is always `{ schemaVersion, error }` โ€” callers pass only the + * error payload. Also emits a brief human-readable line on stderr when a + * TTY is attached, so interactive runs still see the failure. + */ +export function emitJsonError(errorPayload: Record): void { + console.log(JSON.stringify({ schemaVersion: SCHEMA_VERSION, error: errorPayload })); + if (process.stderr.isTTY) { + const msg = typeof errorPayload.message === 'string' ? errorPayload.message : 'Error'; + console.error(chalk.red(msg)); + } +} + function escapeMarkdownCell(s: string): string { // Pipes break markdown table layout; backslash-escape them. Collapse // newlines into
so each row stays on one line. @@ -138,7 +158,7 @@ export type ErrorSubKind = | 'command-not-supported' | 'auth-failed' | 'quota-exceeded' - | 'device-busy' + | 'device-internal-error' | 'unknown-api-error'; export interface ErrorPayload { @@ -151,7 +171,7 @@ export interface ErrorPayload { context?: Record; retryAfterMs?: number; transient?: boolean; - errorClass?: 'network' | 'api' | 'device-offline' | 'device-busy' | 'guard' | 'usage'; + errorClass?: 'network' | 'api' | 'device-offline' | 'device-internal-error' | 'guard' | 'usage'; } export class StructuredUsageError extends Error { @@ -164,11 +184,12 @@ export class StructuredUsageError extends Error { function classifyApiError(code: number): ErrorSubKind { switch (code) { case 151: - case 160: return 'command-not-supported'; + case 160: + case 3005: return 'command-not-supported'; case 152: return 'device-not-found'; case 161: case 171: return 'device-offline'; - case 190: return 'device-busy'; + case 190: return 'device-internal-error'; case 401: return 'auth-failed'; case 429: return 'quota-exceeded'; default: return 'unknown-api-error'; @@ -255,7 +276,16 @@ export function handleError(error: unknown): never { const payload = buildErrorPayload(error); if (isJsonMode()) { - console.error(JSON.stringify({ schemaVersion: SCHEMA_VERSION, error: payload })); + // Bug #SYS-1: Under --json, route the structured envelope to stdout so + // `cli --json ... | jq` pipelines can decode the error shape exactly + // the same way they decode success. Previously it went to stderr, which + // silently broke every error-path pipeline. TTY users still get a + // terse human-readable line on stderr so interactive runs don't look + // like the process simply exited. + console.log(JSON.stringify({ schemaVersion: SCHEMA_VERSION, error: payload })); + if (process.stderr.isTTY) { + console.error(chalk.red(payload.message)); + } process.exit(payload.code === 2 ? 2 : 1); } @@ -291,11 +321,13 @@ function errorHint(code: number): string | null { case 171: return 'The Hub itself is offline โ€” check its power and Wi-Fi.'; case 190: - return "Often means the deviceId is wrong or the command/parameter is invalid for this device. Double-check with 'switchbot devices list' and 'switchbot devices describe '. Use --verbose to see the raw API response."; + return 'SwitchBot API code 190 is a generic internal error. Common causes: invalid deviceId, unsupported command/parameter, or the endpoint does not apply (e.g., "webhook query" with no webhook configured). Verify with --verbose.'; case 401: return "Re-run 'switchbot config set-token ', or verify SWITCHBOT_TOKEN / SWITCHBOT_SECRET."; case 429: return 'Daily quota is 10,000 requests/account โ€” retry after midnight UTC.'; + case 3005: + return "SwitchBot rejected the command as invalid for this specific device model. For IR remotes, this often means the command works only on --type customize (user-learned buttons). Try 'switchbot devices commands ' or check the device's capabilities."; default: return null; } diff --git a/tests/commands/agent-bootstrap.test.ts b/tests/commands/agent-bootstrap.test.ts index 1a8290a1..b0cd1f42 100644 --- a/tests/commands/agent-bootstrap.test.ts +++ b/tests/commands/agent-bootstrap.test.ts @@ -66,6 +66,10 @@ describe('agent-bootstrap', () => { expect(data.identity).toBeDefined(); expect(data.safetyTiers).toBeDefined(); expect(data.quickReference).toBeDefined(); + expect(Array.isArray(data.nameStrategies)).toBe(true); + expect(data.nameStrategies).toEqual([ + 'exact', 'prefix', 'substring', 'fuzzy', 'first', 'require-unique', + ]); expect(Array.isArray(data.devices)).toBe(true); expect((data.devices as unknown[]).length).toBe(1); expect(data.catalog).toBeDefined(); diff --git a/tests/commands/batch.test.ts b/tests/commands/batch.test.ts index 2f22fb56..4e406973 100644 --- a/tests/commands/batch.test.ts +++ b/tests/commands/batch.test.ts @@ -28,6 +28,7 @@ vi.mock('../../src/api/client.js', () => ({ // Cache: keep deterministic across tests. const cacheMock = vi.hoisted(() => ({ map: new Map(), + statusMap: new Map }>(), getCachedDevice: vi.fn((id: string) => cacheMock.map.get(id) ?? null), getCachedTypeMap: vi.fn((ids?: Iterable) => { const out = new Map(); @@ -39,6 +40,7 @@ const cacheMock = vi.hoisted(() => ({ return out; }), updateCacheFromDeviceList: vi.fn(), + loadStatusCache: vi.fn(() => ({ entries: Object.fromEntries(cacheMock.statusMap) })), })); vi.mock('../../src/devices/cache.js', () => ({ getCachedDevice: cacheMock.getCachedDevice, @@ -52,7 +54,7 @@ vi.mock('../../src/devices/cache.js', () => ({ setCachedStatus: vi.fn(), clearStatusCache: vi.fn(), resetStatusCache: vi.fn(), - loadStatusCache: vi.fn(() => ({ entries: {} })), + loadStatusCache: cacheMock.loadStatusCache, describeCache: vi.fn(() => ({ list: { path: '', exists: false }, status: { path: '', exists: false, entryCount: 0 }, @@ -116,6 +118,7 @@ describe('devices batch', () => { apiMock.__instance.post.mockReset(); apiMock.createClient.mockClear(); cacheMock.map.clear(); + cacheMock.statusMap.clear(); cacheMock.getCachedDevice.mockClear(); cacheMock.getCachedTypeMap.mockClear(); flagsMock.dryRun = false; @@ -401,4 +404,157 @@ describe('devices batch', () => { expect(parsed.data.plan.stepCount).toBe(2); expect(parsed.data.plan.steps.map((s: { deviceId: string }) => s.deviceId).sort()).toEqual(['BOT1', 'BOT2']); }); + + it('--idempotency-key alias sets the same prefix as --idempotency-key-prefix', async () => { + flagsMock.dryRun = true; + apiMock.__instance.get.mockResolvedValue({ data: { statusCode: 100, body: DEVICE_LIST_BODY } }); + + const result = await runCli(registerDevicesCommand, [ + '--json', + 'devices', + 'batch', + 'turnOn', + '--filter', + 'type=Bot', + '--plan', + '--idempotency-key', + 'foo', + ]); + + expect(result.exitCode).toBeNull(); + const parsed = JSON.parse(result.stdout[0]); + const keys = parsed.data.plan.steps.map((s: { idempotencyKey?: string }) => s.idempotencyKey).sort(); + expect(keys).toEqual(['foo-BOT1', 'foo-BOT2']); + }); + + it('rejects when both --idempotency-key and --idempotency-key-prefix are supplied', async () => { + flagsMock.dryRun = true; + apiMock.__instance.get.mockResolvedValue({ data: { statusCode: 100, body: DEVICE_LIST_BODY } }); + + const result = await runCli(registerDevicesCommand, [ + 'devices', + 'batch', + 'turnOn', + '--filter', + 'type=Bot', + '--plan', + '--idempotency-key', + 'foo', + '--idempotency-key-prefix', + 'bar', + ]); + + expect(result.exitCode).toBe(2); + expect(result.stderr.join('\n')).toMatch(/either --idempotency-key or --idempotency-key-prefix/); + }); + + it('--skip-offline skips devices whose cached status is offline', async () => { + cacheMock.map.set('BOT1', { type: 'Bot', name: 'Kitchen', category: 'physical' }); + cacheMock.map.set('BOT2', { type: 'Bot', name: 'Office', category: 'physical' }); + cacheMock.statusMap.set('BOT2', { + fetchedAt: new Date().toISOString(), + body: { onlineStatus: 'offline', power: 'off' }, + }); + apiMock.__instance.post.mockResolvedValue({ data: { statusCode: 100, body: {} } }); + + const result = await runCli(registerDevicesCommand, [ + '--json', + 'devices', + 'batch', + 'turnOn', + '--ids', + 'BOT1,BOT2', + '--skip-offline', + ]); + + expect(result.exitCode).toBeNull(); + expect(apiMock.__instance.post).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(result.stdout[0]); + expect(parsed.data.summary.ok).toBe(1); + expect(parsed.data.summary.total).toBe(2); + expect(parsed.data.summary.skipped).toBe(1); + expect(parsed.data.skipped).toEqual([{ deviceId: 'BOT2', reason: 'offline' }]); + expect(parsed.data.succeeded[0].deviceId).toBe('BOT1'); + }); + + it('without --skip-offline, offline-cached devices are still sent', async () => { + cacheMock.map.set('BOT1', { type: 'Bot', name: 'Kitchen', category: 'physical' }); + cacheMock.map.set('BOT2', { type: 'Bot', name: 'Office', category: 'physical' }); + cacheMock.statusMap.set('BOT2', { + fetchedAt: new Date().toISOString(), + body: { onlineStatus: 'offline', power: 'off' }, + }); + apiMock.__instance.post.mockResolvedValue({ data: { statusCode: 100, body: {} } }); + + const result = await runCli(registerDevicesCommand, [ + '--json', + 'devices', + 'batch', + 'turnOn', + '--ids', + 'BOT1,BOT2', + ]); + + expect(result.exitCode).toBeNull(); + expect(apiMock.__instance.post).toHaveBeenCalledTimes(2); + const parsed = JSON.parse(result.stdout[0]); + expect(parsed.data.skipped).toBeUndefined(); + }); + + it('bug28: batch over IR devices attaches subKind + verification and sets summary.unverifiableCount', async () => { + cacheMock.map.set('IR1', { type: 'Air Conditioner', name: 'Living Room AC', category: 'ir' }); + cacheMock.map.set('IR2', { type: 'Air Conditioner', name: 'Bedroom AC', category: 'ir' }); + apiMock.__instance.post.mockResolvedValue({ data: { statusCode: 100, body: {} } }); + + const result = await runCli(registerDevicesCommand, [ + '--json', + 'devices', + 'batch', + 'turnOff', + '--ids', + 'IR1,IR2', + ]); + + expect(result.exitCode).toBeNull(); + const parsed = JSON.parse(result.stdout[0]); + expect(parsed.data.summary.ok).toBe(2); + expect(parsed.data.summary.unverifiableCount).toBe(2); + + for (const s of parsed.data.succeeded) { + expect(s.subKind).toBe('ir-no-feedback'); + expect(s.verification).toBeDefined(); + expect(s.verification.verifiable).toBe(false); + expect(s.verification.reason).toBe( + 'IR transmission is unidirectional; no receipt acknowledgment is possible.' + ); + expect(s.verification.suggestedFollowup).toBe( + 'Confirm visible change manually or via a paired state sensor.' + ); + } + }); + + it('bug28: batch over physical devices does NOT attach subKind/verification and unverifiableCount is 0', async () => { + cacheMock.map.set('BOT1', { type: 'Bot', name: 'Kitchen', category: 'physical' }); + cacheMock.map.set('BOT2', { type: 'Bot', name: 'Office', category: 'physical' }); + apiMock.__instance.post.mockResolvedValue({ data: { statusCode: 100, body: {} } }); + + const result = await runCli(registerDevicesCommand, [ + '--json', + 'devices', + 'batch', + 'turnOn', + '--ids', + 'BOT1,BOT2', + ]); + + expect(result.exitCode).toBeNull(); + const parsed = JSON.parse(result.stdout[0]); + expect(parsed.data.summary.ok).toBe(2); + expect(parsed.data.summary.unverifiableCount).toBe(0); + + for (const s of parsed.data.succeeded) { + expect(s.subKind).toBeUndefined(); + expect(s.verification).toBeUndefined(); + } + }); }); diff --git a/tests/commands/cache.test.ts b/tests/commands/cache.test.ts index 15471282..b61ea3b2 100644 --- a/tests/commands/cache.test.ts +++ b/tests/commands/cache.test.ts @@ -163,4 +163,44 @@ describe('cache clear', () => { expect(result.exitCode).toBeNull(); expect(result.stdout.join('\n')).toMatch(/Cleared/); }); + + it('--status shorthand clears only status cache (bug #35)', async () => { + updateCacheFromDeviceList(SAMPLE_BODY); + setCachedStatus('BOT1', { power: 'on' }); + + const listFile = path.join(tmpHome, '.switchbot', 'devices.json'); + const statusFile = path.join(tmpHome, '.switchbot', 'status.json'); + + const result = await runCli(registerCacheCommand, ['cache', 'clear', '--status']); + expect(result.exitCode).toBeNull(); + expect(fs.existsSync(listFile)).toBe(true); + expect(fs.existsSync(statusFile)).toBe(false); + expect(result.stdout.join('\n')).toMatch(/Cleared:.*status/); + }); + + it('--list shorthand clears only list cache (bug #35)', async () => { + updateCacheFromDeviceList(SAMPLE_BODY); + setCachedStatus('BOT1', { power: 'on' }); + + const listFile = path.join(tmpHome, '.switchbot', 'devices.json'); + const statusFile = path.join(tmpHome, '.switchbot', 'status.json'); + + const result = await runCli(registerCacheCommand, ['cache', 'clear', '--list']); + expect(result.exitCode).toBeNull(); + expect(fs.existsSync(listFile)).toBe(false); + expect(fs.existsSync(statusFile)).toBe(true); + expect(result.stdout.join('\n')).toMatch(/Cleared:.*list/); + }); + + it('--status + --list together exits 2 (bug #35)', async () => { + const result = await runCli(registerCacheCommand, ['cache', 'clear', '--status', '--list']); + expect(result.exitCode).toBe(2); + expect(result.stderr.join('\n')).toMatch(/mutually exclusive/i); + }); + + it('--status combined with --key exits 2 (bug #35)', async () => { + const result = await runCli(registerCacheCommand, ['cache', 'clear', '--status', '--key', 'status']); + expect(result.exitCode).toBe(2); + expect(result.stderr.join('\n')).toMatch(/cannot be combined with --key/i); + }); }); diff --git a/tests/commands/capabilities.test.ts b/tests/commands/capabilities.test.ts index fa96722b..e0012d5d 100644 --- a/tests/commands/capabilities.test.ts +++ b/tests/commands/capabilities.test.ts @@ -19,6 +19,12 @@ function makeProgram(): Command { const describe = devices.command('describe').description('Show full device info'); describe.argument('', 'Device ID'); describe.option('--json', 'JSON output'); + // devices meta subcommands (bug #40) + const meta = devices.command('meta').description('Manage local device metadata'); + meta.command('set').description('Set metadata for a device'); + meta.command('get').description('Get metadata for a device'); + meta.command('list').description('List all device metadata'); + meta.command('clear').description('Clear metadata for a device'); const history = p.command('history').description('Device history and aggregation'); history.command('aggregate').description('Aggregate device history'); @@ -224,4 +230,13 @@ describe('capabilities B3/B4', () => { const mcp = (out.surfaces as Record).mcp; expect(mcp.tools).toContain('aggregate_device_history'); }); + + it('devices meta set appears in compact capabilities output (bug #40)', async () => { + const out = await runCapabilitiesWith(['--compact']); + const cmds = out.commands as Array<{ name: string; agentSafetyTier: string; mutating: boolean }>; + const metaSet = cmds.find((c) => c.name === 'devices meta set'); + expect(metaSet).toBeDefined(); + expect(metaSet!.agentSafetyTier).toBe('action'); + expect(metaSet!.mutating).toBe(true); + }); }); diff --git a/tests/commands/device-meta.test.ts b/tests/commands/device-meta.test.ts index b15e53ef..6d2561cb 100644 --- a/tests/commands/device-meta.test.ts +++ b/tests/commands/device-meta.test.ts @@ -79,4 +79,29 @@ describe('devices meta', () => { const res = await runCli(registerDevicesCommand, ['devices', 'meta', 'get', 'LAMP-1']); expect(res.stdout.join('\n')).toContain('No local metadata'); }); + + it('setting alias on device B (without --force) when device A already holds it โ†’ exit 2 mentioning device A (bug #41)', async () => { + await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-1', '--alias', 'myAlias']); + const res = await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-2', '--alias', 'myAlias']); + expect(res.exitCode).toBe(2); + expect(res.stderr.join('\n')).toContain('LAMP-1'); + }); + + it('--force reassigns alias from device A to device B and clears A (bug #41)', async () => { + await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-1', '--alias', 'myAlias']); + const res = await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-2', '--alias', 'myAlias', '--force']); + expect(res.exitCode).toBeNull(); + // LAMP-1 should have no alias now + const lamp1 = await runCli(registerDevicesCommand, ['devices', 'meta', 'get', 'LAMP-1']); + expect(lamp1.stdout.join('\n')).not.toContain('myAlias'); + // LAMP-2 should hold the alias + const lamp2 = await runCli(registerDevicesCommand, ['devices', 'meta', 'get', 'LAMP-2']); + expect(lamp2.stdout.join('\n')).toContain('myAlias'); + }); + + it('re-asserting the same alias on the same device is a no-op (no conflict with self, bug #41)', async () => { + await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-1', '--alias', 'myAlias']); + const res = await runCli(registerDevicesCommand, ['devices', 'meta', 'set', 'LAMP-1', '--alias', 'myAlias']); + expect(res.exitCode).toBeNull(); + }); }); diff --git a/tests/commands/devices.test.ts b/tests/commands/devices.test.ts index 231b8d29..156a988a 100644 --- a/tests/commands/devices.test.ts +++ b/tests/commands/devices.test.ts @@ -374,6 +374,16 @@ describe('devices command', () => { expect(lines[1]).not.toContain('Living Lamp'); }); + it('--fields id,name aliases resolve to deviceId/deviceName columns (bug #22)', async () => { + apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); + const res = await runCli(registerDevicesCommand, ['devices', 'list', '--format', 'tsv', '--fields', 'id,name']); + const lines = res.stdout.join('\n').split('\n'); + // Header row must show the resolved canonical column names + expect(lines[0]).toBe('deviceId\tdeviceName'); + // Data rows must contain the device id and name values + expect(lines[1]).toBe('ABC123\tLiving Lamp'); + }); + it('--format=id outputs one deviceId per line', async () => { apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); const res = await runCli(registerDevicesCommand, ['devices', 'list', '--format', 'id']); @@ -439,6 +449,39 @@ describe('devices command', () => { expect(out.data.deviceList).toHaveLength(3); expect(out.data.infraredRemoteList).toHaveLength(0); }); + + it('--filter name~Kitchen uses substring match (bug #39)', async () => { + apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); + const res = await runCli(registerDevicesCommand, ['devices', 'list', '--filter', 'name~Kitchen', '--json']); + const out = JSON.parse(res.stdout.join('\n')); + expect(out.data.deviceList).toHaveLength(1); + expect(out.data.deviceList[0].deviceId).toBe('BLE-001'); + }); + + it('--filter type=/regex/ uses case-insensitive regex (bug #39)', async () => { + apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); + const res = await runCli(registerDevicesCommand, ['devices', 'list', '--filter', 'type=/^Strip.*/', '--json']); + const out = JSON.parse(res.stdout.join('\n')); + expect(out.data.deviceList).toHaveLength(1); + expect(out.data.deviceList[0].deviceId).toBe('NOHUB-1'); + }); + + it('--filter with invalid regex exits 2 with UsageError (bug #39)', async () => { + apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); + const res = await runCli(registerDevicesCommand, ['devices', 'list', '--filter', 'name=/[unterminated/']); + expect(res.exitCode).toBe(2); + expect(res.stderr.join('\n')).toMatch(/Invalid regex/i); + }); + + it('--filter combines AND clauses across ops (bug #39)', async () => { + apiMock.__instance.get.mockResolvedValue({ data: { body: sampleBody } }); + const res = await runCli(registerDevicesCommand, [ + 'devices', 'list', '--filter', 'category=physical,name~Lamp', '--json', + ]); + const out = JSON.parse(res.stdout.join('\n')); + expect(out.data.deviceList).toHaveLength(1); + expect(out.data.deviceList[0].deviceId).toBe('ABC123'); + }); }); // ===================================================================== @@ -874,6 +917,15 @@ describe('devices command', () => { await runCmd('setBrightness', '100'); expectPost('setBrightness', 100); }); + it('setBrightness -1 (negative positional parameter reaches validation)', async () => { + // Regression for bug #53: Commander used to swallow "-1" as an unknown + // option token. With allowUnknownOption on the `command` subcommand + // negative numbers are now treated as the parameter positional and + // reach the API (the device then returns 190 for out-of-range values, + // but that's a device-layer concern, not a CLI parsing failure). + await runCmd('setBrightness', '-1'); + expectPost('setBrightness', -1); + }); it('setColorTemperature', async () => { await runCmd('setColorTemperature', '4000'); expectPost('setColorTemperature', 4000); @@ -1808,7 +1860,9 @@ describe('devices command', () => { '--json', 'devices', 'command', LOCK_ID, 'unlock', ]); expect(res.exitCode).toBe(2); - const parsed = JSON.parse(res.stderr.join('\n')); + // Bug #SYS-1: --json errors now go to stdout so piped consumers can + // decode them the same way as success envelopes. + const parsed = JSON.parse(res.stdout.join('\n')); expect(parsed.error.kind).toBe('guard'); expect(parsed.error.code).toBe(2); expect(parsed.error.context.deviceId).toBe(LOCK_ID); @@ -2061,4 +2115,47 @@ describe('devices command', () => { ); }); }); + + // ===================================================================== + // command โ€” dry-run structured output (bug #36) + // ===================================================================== + describe('command โ€” dry-run output', () => { + const DRY_ID = 'DRY-DEV-1'; + + beforeEach(() => { + // Make post throw DryRunSignal (simulates --dry-run interceptor) + apiMock.__instance.post.mockImplementation(async () => { + throw new apiMock.DryRunSignal('POST', `/v1.1/devices/${DRY_ID}/commands`); + }); + }); + + it('emits structured JSON with dryRun:true when --dry-run --json', async () => { + const res = await runCli(registerDevicesCommand, [ + '--dry-run', '--json', 'devices', 'command', DRY_ID, 'turnOff', + ]); + expect(res.exitCode).toBeNull(); + // post was called (and threw DryRunSignal โ€” that's the mechanism), but the API + // result was never used; we verify the structured output instead. + expect(apiMock.__instance.post).toHaveBeenCalledTimes(1); + // stdout must have valid JSON + const out = res.stdout.join('\n'); + expect(out).toBeTruthy(); + const parsed = JSON.parse(out); + expect(parsed.schemaVersion).toBe('1.1'); + expect(parsed.data.dryRun).toBe(true); + expect(parsed.data.wouldSend.deviceId).toBe(DRY_ID); + expect(parsed.data.wouldSend.command).toBe('turnOff'); + expect(parsed.data.wouldSend.commandType).toBe('command'); + }); + + it('emits human-readable dry-run message to stdout when --dry-run (no --json)', async () => { + const res = await runCli(registerDevicesCommand, [ + '--dry-run', 'devices', 'command', DRY_ID, 'turnOn', + ]); + expect(res.exitCode).toBeNull(); + const out = res.stdout.join('\n'); + expect(out).toMatch(/dry-run/i); + expect(out).toContain(DRY_ID); + }); + }); }); diff --git a/tests/commands/dry-run.test.ts b/tests/commands/dry-run.test.ts index 03acbc0a..1feb8cad 100644 --- a/tests/commands/dry-run.test.ts +++ b/tests/commands/dry-run.test.ts @@ -105,6 +105,7 @@ describe('dryRun support on mutating tools', () => { }); it('send_command dryRun:true with parameter and commandType mirrors the full request shape', async () => { + cacheMock.map.set('IR1', { type: 'IR TV', name: 'Living Room', category: 'ir' }); const { client } = await pair(); const res = await client.callTool({ diff --git a/tests/commands/events.test.ts b/tests/commands/events.test.ts index 3f5c7e40..cc29816d 100644 --- a/tests/commands/events.test.ts +++ b/tests/commands/events.test.ts @@ -6,6 +6,7 @@ import path from 'node:path'; import { once } from 'node:events'; import { AddressInfo } from 'node:net'; import { startReceiver, registerEventsCommand } from '../../src/commands/events.js'; +import type { FilterClause } from '../../src/utils/filter.js'; import { deviceHistoryStore } from '../../src/mcp/device-history.js'; import { runCli } from '../helpers/cli.js'; @@ -178,10 +179,11 @@ describe('events tail receiver', () => { it('marks events as unmatched when deviceId filter does not match', async () => { const port = await pickPort(); const received: Array<{ matched: boolean }> = []; + const filter: FilterClause[] = [{ key: 'deviceId', op: 'eq', raw: 'BOT1' }]; const server = startReceiver( port, '/', - { deviceId: 'BOT1' }, + filter, (ev) => received.push(ev as { matched: boolean }), ); await postJson(port, '/', { context: { deviceMac: 'BOT2', deviceType: 'Bot' } }); @@ -195,10 +197,11 @@ describe('events tail receiver', () => { it('type filter matches on context.deviceType', async () => { const port = await pickPort(); const received: Array<{ matched: boolean }> = []; + const filter: FilterClause[] = [{ key: 'type', op: 'eq', raw: 'WoMeter' }]; const server = startReceiver( port, '/', - { type: 'WoMeter' }, + filter, (ev) => received.push(ev as { matched: boolean }), ); await postJson(port, '/', { context: { deviceMac: 'X1', deviceType: 'Bot' } }); @@ -282,9 +285,12 @@ describe('events mqtt-tail', () => { const res = await runCli(registerEventsCommand, ['events', 'mqtt-tail', '--max', '1']); expect(res.exitCode).toBe(null); - const jsonLines = res.stdout.filter((l) => l.trim().startsWith('{')); - expect(jsonLines).toHaveLength(1); - const parsed = JSON.parse(jsonLines[0]) as { t: string; topic: string; payload: unknown }; + const jsonLines = res.stdout + .filter((l) => l.trim().startsWith('{')) + .map((l) => JSON.parse(l) as { type?: string; topic?: string; payload?: unknown; t?: string }); + const events = jsonLines.filter((j) => typeof j.type !== 'string' || !j.type.startsWith('__')); + expect(events).toHaveLength(1); + const parsed = events[0] as { t: string; topic: string; payload: unknown }; expect(parsed.topic).toBe('test/topic'); expect(parsed.payload).toEqual({ state: 'on' }); expect(typeof parsed.t).toBe('string'); @@ -295,11 +301,15 @@ describe('events mqtt-tail', () => { const res = await runCli(registerEventsCommand, ['--json', 'events', 'mqtt-tail', '--max', '1']); expect(res.exitCode).toBe(null); - const jsonLines = res.stdout.filter((l) => l.trim().startsWith('{')); - expect(jsonLines).toHaveLength(1); - const parsed = JSON.parse(jsonLines[0]) as { schemaVersion: string; data: { topic: string } }; - expect(parsed.schemaVersion).toBe('1.1'); - expect(parsed.data.topic).toBe('test/topic'); + const jsonLines = res.stdout + .filter((l) => l.trim().startsWith('{')) + .map((l) => JSON.parse(l) as { schemaVersion: string; data: { type?: string; topic?: string } }); + const events = jsonLines.filter( + (j) => typeof j.data?.type !== 'string' || !j.data.type.startsWith('__'), + ); + expect(events).toHaveLength(1); + expect(events[0].schemaVersion).toBe('1.1'); + expect(events[0].data.topic).toBe('test/topic'); }); it('exits 2 when --max is not a positive integer', async () => { @@ -349,6 +359,22 @@ describe('events mqtt-tail', () => { const disconnect = jsonLines.find((j) => (j as { type?: string }).type === '__disconnect'); expect(disconnect).toBeDefined(); }); + + it('emits __session_start envelope under --json before broker connect (bug #56)', async () => { + mqttMock.connectShouldFireMessage = true; + const res = await runCli(registerEventsCommand, ['--json', 'events', 'mqtt-tail', '--max', '1']); + const jsonLines = res.stdout + .filter((l) => l.trim().startsWith('{')) + .map((l) => JSON.parse(l) as { data: { type?: string; state?: string; at?: string; eventId?: string } }); + const sessionStart = jsonLines.find((j) => j.data?.type === '__session_start'); + expect(sessionStart).toBeDefined(); + expect(sessionStart!.data.state).toBe('connecting'); + expect(typeof sessionStart!.data.at).toBe('string'); + expect(typeof sessionStart!.data.eventId).toBe('string'); + // Must be the FIRST JSON line emitted so consumers see it even if broker + // never connects. + expect((jsonLines[0] as { data: { type?: string } }).data.type).toBe('__session_start'); + }); }); // --------------------------------------------------------------------------- diff --git a/tests/commands/explain.test.ts b/tests/commands/explain.test.ts index 3587d5bf..6f2796da 100644 --- a/tests/commands/explain.test.ts +++ b/tests/commands/explain.test.ts @@ -132,14 +132,15 @@ describe('devices explain', () => { expect(parsed.data.warnings).toHaveLength(0); }); - it('--json: device not found emits { error: { code:1, kind:"runtime" } } on stderr', async () => { + it('--json: device not found emits { error: { code:1, kind:"runtime" } } on stdout (bug #SYS-1)', async () => { devicesMock.describeDevice.mockRejectedValue(new devicesMock.DeviceNotFoundError('MISSING')); const res = await runExplain('--json', 'MISSING'); expect(res.exitCode).toBe(1); - expect(res.stdout).toHaveLength(0); - const parsed = JSON.parse(res.stderr[0]); + // Non-TTY: stderr stays clean so jq consumers aren't polluted. + expect(res.stderr).toHaveLength(0); + const parsed = JSON.parse(res.stdout[0]); expect(parsed.error.code).toBe(1); expect(parsed.error.kind).toBe('runtime'); expect(parsed.error.message).toContain('MISSING'); diff --git a/tests/commands/history.test.ts b/tests/commands/history.test.ts index e41c439f..8457113d 100644 --- a/tests/commands/history.test.ts +++ b/tests/commands/history.test.ts @@ -346,12 +346,13 @@ describe('history aggregate (D7)', () => { expect(parsed.data.buckets[0].metrics.temperature.avg).toBe(22); }); - it('exits 2 with UsageError when --metric is missing', async () => { - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((_code) => { throw new Error('process.exit'); }); + it('exits with error when --metric is missing (requiredOption enforcement, bug #42)', async () => { const res = await runCli(registerHistoryCommand, [ 'history', 'aggregate', 'DEV1', '--since', '1h', ]); - exitSpy.mockRestore(); - expect(res.exitCode).toBe(2); + expect(res.exitCode).not.toBeNull(); + expect(res.exitCode).not.toBe(0); + const errOut = res.stderr.join('\n'); + expect(errOut).toMatch(/--metric/); }); }); diff --git a/tests/commands/mcp.test.ts b/tests/commands/mcp.test.ts index f48106e4..2513a1c0 100644 --- a/tests/commands/mcp.test.ts +++ b/tests/commands/mcp.test.ts @@ -17,9 +17,21 @@ const apiMock = vi.hoisted(() => { vi.mock('../../src/api/client.js', () => ({ createClient: apiMock.createClient, ApiError: class ApiError extends Error { - constructor(message: string, public readonly code: number) { + public readonly retryable: boolean; + public readonly hint?: string; + public readonly retryAfterMs?: number; + public readonly transient: boolean; + constructor( + message: string, + public readonly code: number, + meta: { retryable?: boolean; hint?: string; retryAfterMs?: number; transient?: boolean } = {} + ) { super(message); this.name = 'ApiError'; + this.retryable = meta.retryable ?? false; + this.hint = meta.hint; + this.retryAfterMs = meta.retryAfterMs; + this.transient = meta.transient ?? false; } }, DryRunSignal: class DryRunSignal extends Error { @@ -60,6 +72,7 @@ vi.mock('../../src/devices/cache.js', () => ({ import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { createSwitchBotMcpServer } from '../../src/commands/mcp.js'; +import { ApiError } from '../../src/api/client.js'; /** Connect a fresh server + client pair and return both. */ async function pair() { @@ -182,6 +195,42 @@ describe('mcp server', () => { expect(apiMock.__instance.post).toHaveBeenCalledTimes(1); }); + it('send_command dryRun rejects unknown deviceId against local cache (bug #SYS-3)', async () => { + // Cache is empty โ€” no devices known. + const { client } = await pair(); + + const res = await client.callTool({ + name: 'send_command', + arguments: { deviceId: 'DEADBEEF', command: 'turnOff', dryRun: true }, + }); + + expect(res.isError).toBe(true); + const structured = res.structuredContent as { error?: { kind?: string; subKind?: string; context?: { deviceId?: string } } }; + expect(structured.error?.kind).toBe('usage'); + expect(structured.error?.subKind).toBe('device-not-found'); + expect(structured.error?.context?.deviceId).toBe('DEADBEEF'); + // Dry-run must not hit the network even for preflight. + expect(apiMock.__instance.post).not.toHaveBeenCalled(); + expect(apiMock.__instance.get).not.toHaveBeenCalled(); + }); + + it('send_command dryRun succeeds when deviceId is cached (bug #SYS-3 happy path)', async () => { + cacheMock.map.set('BULB1', { type: 'Color Bulb', name: 'Desk Lamp', category: 'physical' }); + const { client } = await pair(); + + const res = await client.callTool({ + name: 'send_command', + arguments: { deviceId: 'BULB1', command: 'turnOff', dryRun: true }, + }); + + expect(res.isError).toBeFalsy(); + const structured = res.structuredContent as { ok?: boolean; dryRun?: boolean; wouldSend?: { deviceId?: string; command?: string } }; + expect(structured.ok).toBe(true); + expect(structured.dryRun).toBe(true); + expect(structured.wouldSend?.deviceId).toBe('BULB1'); + expect(structured.wouldSend?.command).toBe('turnOff'); + }); + it('list_devices returns the raw API body and refreshes the cache', async () => { const body = { deviceList: [], infraredRemoteList: [] }; apiMock.__instance.get.mockResolvedValueOnce({ data: { statusCode: 100, body } }); @@ -274,6 +323,28 @@ describe('mcp server', () => { expect(parsed.some((e: { type: string }) => e.type === 'Strip Light')).toBe(true); }); + it('search_catalog rejects an empty query with a usage error', async () => { + const { client } = await pair(); + const res = await client.callTool({ + name: 'search_catalog', + arguments: { query: '' }, + }); + expect(res.isError).toBe(true); + const structured = (res as { structuredContent?: { error?: { kind?: string; message?: string; hint?: string } } }).structuredContent; + expect(structured?.error?.kind).toBe('usage'); + expect(structured?.error?.message).toMatch(/non-empty query/i); + expect(structured?.error?.hint).toMatch(/list_catalog_types/); + }); + + it('search_catalog rejects whitespace-only query', async () => { + const { client } = await pair(); + const res = await client.callTool({ + name: 'search_catalog', + arguments: { query: ' ' }, + }); + expect(res.isError).toBe(true); + }); + it('run_scene POSTs the scene execute endpoint', async () => { apiMock.__instance.post.mockResolvedValueOnce({ data: { statusCode: 100, body: {} } }); const { client } = await pair(); @@ -393,4 +464,73 @@ describe('mcp server', () => { fs.rmSync(tmpHome, { recursive: true, force: true }); } }); + + // --------------------------------------------------------------------------- + // Bug #38: structured error metadata preserved in MCP tool responses + // --------------------------------------------------------------------------- + + it('send_command preserves structured error metadata on ApiError (code 161 device-offline)', async () => { + cacheMock.map.set('BLE1', { type: 'Bot', name: 'BLE Bot', category: 'physical' }); + // Mock the POST to throw a device-offline ApiError + apiMock.__instance.post.mockRejectedValueOnce( + new ApiError('Device offline (check Wi-Fi / Bluetooth connection)', 161, { transient: false }) + ); + const { client } = await pair(); + + const res = await client.callTool({ + name: 'send_command', + arguments: { deviceId: 'BLE1', command: 'turnOn' }, + }); + + expect(res.isError).toBe(true); + const sc = (res as { structuredContent?: unknown }).structuredContent as + | { error?: { code?: number; subKind?: string; transient?: boolean; hint?: string } } + | undefined; + expect(sc?.error?.code).toBe(161); + expect(sc?.error?.subKind).toBe('device-offline'); + expect(sc?.error?.transient).toBe(false); + expect(sc?.error?.hint).toMatch(/Hub/); + // content[0].text must still be a JSON string (backwards compat) + const text = (res.content as Array<{ type: string; text: string }>)[0].text; + expect(() => JSON.parse(text)).not.toThrow(); + }); + + it('describe_device preserves structured error metadata on ApiError (code 401 auth-failed)', async () => { + // Mock the GET (fetchDeviceList inside describeDevice) to throw auth error + apiMock.__instance.get.mockRejectedValueOnce( + new ApiError('Authentication failed', 401, { transient: false, retryable: false }) + ); + const { client } = await pair(); + + const res = await client.callTool({ + name: 'describe_device', + arguments: { deviceId: 'ANY1' }, + }); + + expect(res.isError).toBe(true); + const sc = (res as { structuredContent?: unknown }).structuredContent as + | { error?: { subKind?: string; errorClass?: string } } + | undefined; + expect(sc?.error?.subKind).toBe('auth-failed'); + expect(sc?.error?.errorClass).toBe('api'); + }); + + it('run_scene preserves structured error metadata on ApiError (code 190 device-internal-error)', async () => { + // Mock the POST (executeScene) to throw device-internal-error ApiError + apiMock.__instance.post.mockRejectedValueOnce( + new ApiError('Device internal error', 190, { transient: false }) + ); + const { client } = await pair(); + + const res = await client.callTool({ + name: 'run_scene', + arguments: { sceneId: 'SCENE1' }, + }); + + expect(res.isError).toBe(true); + const sc = (res as { structuredContent?: unknown }).structuredContent as + | { error?: { subKind?: string } } + | undefined; + expect(sc?.error?.subKind).toBe('device-internal-error'); + }); }); diff --git a/tests/commands/plan.test.ts b/tests/commands/plan.test.ts index 7b73c05c..08d5563f 100644 --- a/tests/commands/plan.test.ts +++ b/tests/commands/plan.test.ts @@ -160,6 +160,12 @@ describe('plan command', () => { expect(out.valid).toBe(true); expect(out.steps).toBe(1); }); + + it('--help output contains "structural only" (bug #32)', async () => { + const res = await runCli(registerPlanCommand, ['plan', 'validate', '--help']); + const all = [...res.stdout, ...res.stderr].join('\n'); + expect(all).toMatch(/structural only/); + }); }); describe('plan run', () => { diff --git a/tests/commands/scenes.test.ts b/tests/commands/scenes.test.ts index 42b02f29..193d6394 100644 --- a/tests/commands/scenes.test.ts +++ b/tests/commands/scenes.test.ts @@ -110,6 +110,9 @@ describe('scenes command', () => { describe('execute', () => { it('POSTs to the scene execute endpoint and prints success', async () => { + apiMock.__instance.get.mockResolvedValue({ + data: { body: [{ sceneId: 'SCENE-1', sceneName: 'Morning' }] }, + }); apiMock.__instance.post.mockResolvedValue({ data: {} }); const res = await runCli(registerScenesCommand, ['scenes', 'execute', 'SCENE-1']); expect(apiMock.__instance.post).toHaveBeenCalledWith('/v1.1/scenes/SCENE-1/execute'); @@ -117,6 +120,9 @@ describe('scenes command', () => { }); it('exits 1 when execution fails', async () => { + apiMock.__instance.get.mockResolvedValue({ + data: { body: [{ sceneId: 'missing', sceneName: 'X' }] }, + }); apiMock.__instance.post.mockRejectedValue(new Error('not found')); const res = await runCli(registerScenesCommand, ['scenes', 'execute', 'missing']); expect(res.exitCode).toBe(1); @@ -128,6 +134,20 @@ describe('scenes command', () => { expect(apiMock.__instance.post).not.toHaveBeenCalled(); expect(res.stderr.join('\n').toLowerCase()).toContain('missing required'); }); + + it('exits 2 with scene_not_found and never calls executeScene for bogus sceneId (bug #31)', async () => { + apiMock.__instance.get.mockResolvedValue({ + data: { body: [{ sceneId: 'S1', sceneName: 'Good Morning' }] }, + }); + const res = await runCli(registerScenesCommand, ['scenes', 'execute', 'BOGUS-ID', '--json']); + expect(res.exitCode).toBe(2); + expect(apiMock.__instance.post).not.toHaveBeenCalled(); + // Bug #SYS-1: --json errors now emit on stdout so piped consumers see them. + const out = res.stdout.join('\n'); + const parsed = JSON.parse(out); + expect(parsed.error?.context?.error).toBe('scene_not_found'); + expect(parsed.error?.context?.sceneId).toBe('BOGUS-ID'); + }); }); describe('describe', () => { @@ -160,7 +180,8 @@ describe('scenes command', () => { }); const res = await runCli(registerScenesCommand, ['scenes', 'describe', 'MISSING', '--json']); expect(res.exitCode).toBe(2); - const out = res.stderr.join('\n'); + // Bug #SYS-1: --json errors now emit on stdout so piped consumers see them. + const out = res.stdout.join('\n'); const parsed = JSON.parse(out); expect(parsed.error?.context?.error).toBe('scene_not_found'); expect(parsed.error?.context?.sceneId).toBe('MISSING'); diff --git a/tests/commands/watch.test.ts b/tests/commands/watch.test.ts index 094272c2..907fddb9 100644 --- a/tests/commands/watch.test.ts +++ b/tests/commands/watch.test.ts @@ -106,6 +106,19 @@ describe('devices watch', () => { expect(res.stderr.join('\n')).toMatch(/--max/); }); + it('--for stops the loop after elapsed time', async () => { + cacheMock.map.set('BOT1', { type: 'Bot', name: 'Kitchen', category: 'physical' }); + apiMock.__instance.get.mockResolvedValue({ + data: { statusCode: 100, body: { power: 'on', battery: 90 } }, + }); + const res = await runCli(registerDevicesCommand, [ + '--json', 'devices', 'watch', 'BOT1', '--interval', '1s', '--for', '200ms', + ]); + // --for triggers AbortController.abort() after 200ms; the loop exits + // cleanly with exit code null (no unhandled throw). + expect(res.exitCode).toBeNull(); + }, 3000); + it('emits one JSONL event per device on first tick with from:null (--max=1)', async () => { cacheMock.map.set('BOT1', { type: 'Bot', name: 'Kitchen', category: 'physical' }); apiMock.__instance.get.mockResolvedValueOnce({ diff --git a/tests/devices/cache-scoping.test.ts b/tests/devices/cache-scoping.test.ts new file mode 100644 index 00000000..190e8202 --- /dev/null +++ b/tests/devices/cache-scoping.test.ts @@ -0,0 +1,215 @@ +/** + * Tests for per-profile cache scoping (Bug #37). + * + * Each test: + * - Redirects os.homedir() to a fresh tmpdir so no real ~/.switchbot is touched. + * - Sets/clears the active profile via process.argv or withRequestContext. + * - Verifies that the file created on disk is at the expected scoped path. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; +import { createHash } from 'node:crypto'; + +import { + loadCache, + updateCacheFromDeviceList, + setCachedStatus, + resetListCache, + resetStatusCache, +} from '../../src/devices/cache.js'; +import { withRequestContext } from '../../src/lib/request-context.js'; + +let tmpDir: string; + +const sampleBody = { + deviceList: [{ deviceId: 'DEV-1', deviceName: 'Bot', deviceType: 'Bot' }], + infraredRemoteList: [], +}; + +function sha8(profile: string): string { + return createHash('sha256').update(profile).digest('hex').slice(0, 8); +} + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sbcli-scoping-')); + vi.spyOn(os, 'homedir').mockReturnValue(tmpDir); + // Start each test with no profile flag and clean argv + process.argv = ['node', 'switchbot']; + resetListCache(); + resetStatusCache(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + resetListCache(); + resetStatusCache(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +// โ”€โ”€ a. No profile (default) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe('cache scoping โ€” no profile (legacy path)', () => { + it('writes devices.json to the legacy ~/.switchbot/devices.json path', () => { + // No profile set โ€” process.argv has no --profile flag + updateCacheFromDeviceList(sampleBody); + + const legacy = path.join(tmpDir, '.switchbot', 'devices.json'); + expect(fs.existsSync(legacy)).toBe(true); + + // The scoped sub-directory must NOT exist + const scopedDir = path.join(tmpDir, '.switchbot', 'cache'); + expect(fs.existsSync(scopedDir)).toBe(false); + }); +}); + +// โ”€โ”€ b. Named profile โ†’ scoped path โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe('cache scoping โ€” named profile "alpha"', () => { + it('writes devices.json under ~/.switchbot/cache//devices.json', () => { + const expected = path.join(tmpDir, '.switchbot', 'cache', sha8('alpha'), 'devices.json'); + + withRequestContext({ profile: 'alpha' }, () => { + updateCacheFromDeviceList(sampleBody); + }); + + expect(fs.existsSync(expected)).toBe(true); + + // Legacy path must NOT have been created + const legacy = path.join(tmpDir, '.switchbot', 'devices.json'); + expect(fs.existsSync(legacy)).toBe(false); + }); +}); + +// โ”€โ”€ c. Different profiles โ†’ different directories, no cross-contamination โ”€โ”€โ”€ + +describe('cache scoping โ€” profile isolation', () => { + it('alpha and beta get separate directories; switching profile is a cache miss', () => { + // Write as "alpha" + withRequestContext({ profile: 'alpha' }, () => { + updateCacheFromDeviceList(sampleBody); + }); + + resetListCache(); + + // Read as "beta" โ€” should be a cache miss (null) + const result = withRequestContext({ profile: 'beta' }, () => { + // loadCache is imported inside the module; we test the side-effect: + // after writing for alpha, writing for beta creates a separate file + updateCacheFromDeviceList({ + deviceList: [{ deviceId: 'DEV-2', deviceName: 'Plug', deviceType: 'Plug' }], + infraredRemoteList: [], + }); + return fs.existsSync(path.join(tmpDir, '.switchbot', 'cache', sha8('beta'), 'devices.json')); + }); + + expect(result).toBe(true); + + // Alpha's file must still exist independently + const alphaFile = path.join(tmpDir, '.switchbot', 'cache', sha8('alpha'), 'devices.json'); + expect(fs.existsSync(alphaFile)).toBe(true); + + // Alpha's content must be the original write, not beta's + const alphaCache = JSON.parse(fs.readFileSync(alphaFile, 'utf-8')); + expect(alphaCache.devices['DEV-1']).toBeDefined(); + expect(alphaCache.devices['DEV-2']).toBeUndefined(); + }); +}); + +// โ”€โ”€ d. Status cache parity โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe('cache scoping โ€” status cache follows the same rule', () => { + it('no profile โ†’ status.json at legacy ~/.switchbot/status.json', () => { + setCachedStatus('DEV-1', { power: 'on' }); + + const legacy = path.join(tmpDir, '.switchbot', 'status.json'); + expect(fs.existsSync(legacy)).toBe(true); + + const scopedDir = path.join(tmpDir, '.switchbot', 'cache'); + expect(fs.existsSync(scopedDir)).toBe(false); + }); + + it('profile "alpha" โ†’ status.json at ~/.switchbot/cache//status.json', () => { + const expected = path.join(tmpDir, '.switchbot', 'cache', sha8('alpha'), 'status.json'); + + withRequestContext({ profile: 'alpha' }, () => { + setCachedStatus('DEV-1', { power: 'on' }); + }); + + expect(fs.existsSync(expected)).toBe(true); + + const legacy = path.join(tmpDir, '.switchbot', 'status.json'); + expect(fs.existsSync(legacy)).toBe(false); + }); +}); + +// โ”€โ”€ e. In-memory cache does not leak across profile switches โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe('cache scoping โ€” in-memory hot cache isolation across profiles', () => { + it('in-memory cache does not leak across profile switches within a single process', () => { + const alphaBody = { + deviceList: [{ deviceId: 'ALPHA-1', deviceName: 'Alpha Bot', deviceType: 'Bot' }], + infraredRemoteList: [], + }; + const betaBody = { + deviceList: [{ deviceId: 'BETA-1', deviceName: 'Beta Plug', deviceType: 'Plug' }], + infraredRemoteList: [], + }; + + // Write alpha cache on disk and populate in-memory hot cache for alpha. + withRequestContext({ profile: 'alpha' }, () => { + updateCacheFromDeviceList(alphaBody); + }); + + // Verify alpha is in-memory. + const alphaResult = withRequestContext({ profile: 'alpha' }, () => loadCache()); + expect(alphaResult?.devices['ALPHA-1']).toBeDefined(); + + // Write beta's inventory directly to disk (bypassing the hot-cache write path), + // simulating the scenario where beta's data was written in a prior process and + // only the hot cache is "stale" (points to alpha). + const betaDir = path.join(tmpDir, '.switchbot', 'cache', sha8('beta')); + fs.mkdirSync(betaDir, { recursive: true }); + const betaCache = { + lastUpdated: new Date().toISOString(), + devices: { 'BETA-1': { type: 'Plug', name: 'Beta Plug', category: 'physical' } }, + }; + fs.writeFileSync(path.join(betaDir, 'devices.json'), JSON.stringify(betaCache)); + + // Read under profile "beta" WITHOUT calling resetListCache() first. + // With the bug (single global _listCache), this would return alpha's data. + // With the fix (Map keyed by profile), this must read from disk and return beta's data. + const betaResult = withRequestContext({ profile: 'beta' }, () => loadCache()); + + expect(betaResult?.devices['BETA-1']).toBeDefined(); + expect(betaResult?.devices['ALPHA-1']).toBeUndefined(); + }); +}); + +// โ”€โ”€ f. --config-path override takes precedence (profile is ignored) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe('cache scoping โ€” --config-path override', () => { + it('uses config override dirname regardless of active profile', () => { + const custom = path.join(tmpDir, 'alt', 'cfg.json'); + fs.mkdirSync(path.dirname(custom), { recursive: true }); + process.argv = ['node', 'switchbot', '--config', custom]; + resetListCache(); + resetStatusCache(); + + withRequestContext({ profile: 'alpha' }, () => { + updateCacheFromDeviceList(sampleBody); + }); + + // Must land in the override dir, not the scoped dir + const overrideFile = path.join(tmpDir, 'alt', 'devices.json'); + expect(fs.existsSync(overrideFile)).toBe(true); + + // Neither legacy nor scoped paths should exist + const legacy = path.join(tmpDir, '.switchbot', 'devices.json'); + const scoped = path.join(tmpDir, '.switchbot', 'cache', sha8('alpha'), 'devices.json'); + expect(fs.existsSync(legacy)).toBe(false); + expect(fs.existsSync(scoped)).toBe(false); + }); +}); diff --git a/tests/mcp/server-version.test.ts b/tests/mcp/server-version.test.ts index a5da5adb..a3c2678d 100644 --- a/tests/mcp/server-version.test.ts +++ b/tests/mcp/server-version.test.ts @@ -13,7 +13,6 @@ describe('mcp server version', () => { // Verify the VERSION constant matches expect(VERSION).toBe(expectedVersion); - expect(VERSION).toBe('2.5.0'); }); }); diff --git a/tests/utils/arg-parsers.test.ts b/tests/utils/arg-parsers.test.ts index 15daeb65..713bc5bb 100644 --- a/tests/utils/arg-parsers.test.ts +++ b/tests/utils/arg-parsers.test.ts @@ -54,6 +54,8 @@ describe('durationArg', () => { expect(parse('500ms')).toBe('500ms'); expect(parse('1m')).toBe('1m'); expect(parse('1h')).toBe('1h'); + expect(parse('1d')).toBe('1d'); + expect(parse('2w')).toBe('2w'); expect(parse('1000')).toBe('1000'); // bare ms }); @@ -66,6 +68,12 @@ describe('durationArg', () => { expect(() => parse('abc')).toThrow(/must look like/); expect(() => parse('devices')).toThrow(/must look like/); }); + + it('rejects unsupported units (y, month, week) with helpful hint', () => { + expect(() => parse('1y')).toThrow(/ms, s, m, h, d, w/); + expect(() => parse('1year')).toThrow(/ms, s, m, h, d, w/); + expect(() => parse('1month')).toThrow(/must look like/); + }); }); describe('stringArg', () => { diff --git a/tests/utils/filter.test.ts b/tests/utils/filter.test.ts index 335d7fd9..8e379110 100644 --- a/tests/utils/filter.test.ts +++ b/tests/utils/filter.test.ts @@ -1,10 +1,18 @@ import { describe, it, expect } from 'vitest'; -import { parseFilter, applyFilter, FilterSyntaxError } from '../../src/utils/filter.js'; +import { + parseFilter, + parseFilterExpr, + matchClause, + applyFilter, + FilterSyntaxError, + type FilterClause, +} from '../../src/utils/filter.js'; import type { Device, InfraredDevice } from '../../src/lib/devices.js'; const devices: Device[] = [ { deviceId: 'BOT1', deviceName: 'Kitchen Bot', deviceType: 'Bot', familyName: 'Home', roomName: 'Kitchen', enableCloudService: true, hubDeviceId: 'HUB1' }, { deviceId: 'BOT2', deviceName: 'Office Bot', deviceType: 'Bot', familyName: 'Home', roomName: 'Office', enableCloudService: true, hubDeviceId: 'HUB1' }, + { deviceId: 'BOT3', deviceName: 'Garage Bot Plus', deviceType: 'Bot Plus', familyName: 'Home', roomName: 'Garage', enableCloudService: true, hubDeviceId: 'HUB1' }, { deviceId: 'LAMP', deviceName: 'Desk', deviceType: 'Color Bulb', familyName: 'Home', roomName: 'Office', enableCloudService: true, hubDeviceId: 'HUB1' }, { deviceId: 'METER', deviceName: 'Outside', deviceType: 'Meter', familyName: 'Cabin', roomName: 'Porch', enableCloudService: true, hubDeviceId: 'HUB2' }, ]; @@ -19,19 +27,41 @@ const hubLoc = new Map([ ['HUB2', { family: 'Cabin', room: 'Bedroom' }], ]); -describe('parseFilter', () => { +describe('parseFilter (batch-key default)', () => { it('returns [] for undefined / empty string', () => { expect(parseFilter(undefined)).toEqual([]); expect(parseFilter('')).toEqual([]); expect(parseFilter(' ')).toEqual([]); }); - it('parses a single exact clause', () => { - expect(parseFilter('type=Bot')).toEqual([{ key: 'type', op: '=', value: 'Bot' }]); + it('parses "key=value" as an eq clause (raw preserved)', () => { + expect(parseFilter('type=Bot')).toEqual([ + { key: 'type', op: 'eq', raw: 'Bot', regex: undefined }, + ]); }); - it('parses a substring (~=) clause', () => { - expect(parseFilter('type~=Light')).toEqual([{ key: 'type', op: '~=', value: 'Light' }]); + it('parses "key~value" as a sub clause', () => { + expect(parseFilter('type~Light')).toEqual([ + { key: 'type', op: 'sub', raw: 'Light', regex: undefined }, + ]); + }); + + it('parses "key=/pattern/" as a regex clause with case-insensitive RegExp', () => { + const [c] = parseFilter('type=/Bot.*/'); + expect(c.key).toBe('type'); + expect(c.op).toBe('regex'); + expect(c.raw).toBe('Bot.*'); + expect(c.regex?.source).toBe('Bot.*'); + expect(c.regex?.flags).toContain('i'); + }); + + it('rejects the legacy "~=" spelling with a helpful hint', () => { + expect(() => parseFilter('type~=Light')).toThrow(FilterSyntaxError); + expect(() => parseFilter('type~=Light')).toThrow(/~=.*no longer supported/); + }); + + it('rejects invalid regex with FilterSyntaxError', () => { + expect(() => parseFilter('type=/[/')).toThrow(FilterSyntaxError); }); it('parses multi-clause AND expressions', () => { @@ -43,7 +73,7 @@ describe('parseFilter', () => { it('trims whitespace around keys and values', () => { const [c] = parseFilter(' type = Bot Plus '); - expect(c).toEqual({ key: 'type', op: '=', value: 'Bot Plus' }); + expect(c).toEqual({ key: 'type', op: 'eq', raw: 'Bot Plus', regex: undefined }); }); it('rejects unknown keys', () => { @@ -59,26 +89,103 @@ describe('parseFilter', () => { }); }); +describe('parseFilterExpr with custom allowedKeys', () => { + it('accepts events-tail keys (deviceId, type)', () => { + const c = parseFilterExpr('deviceId=ABC,type~Bot', ['deviceId', 'type']); + expect(c).toHaveLength(2); + expect(c[0]).toEqual({ key: 'deviceId', op: 'eq', raw: 'ABC', regex: undefined }); + expect(c[1]).toEqual({ key: 'type', op: 'sub', raw: 'Bot', regex: undefined }); + }); + + it('rejects keys outside the allowed set', () => { + expect(() => parseFilterExpr('family=Home', ['deviceId', 'type'])).toThrow( + FilterSyntaxError, + ); + }); + + it('accepts list keys including "name"', () => { + const [c] = parseFilterExpr('name~office', ['type', 'name', 'category', 'room']); + expect(c).toEqual({ key: 'name', op: 'sub', raw: 'office', regex: undefined }); + }); +}); + +describe('matchClause', () => { + const sub = (key: string, raw: string): FilterClause => ({ key, op: 'sub', raw }); + const eq = (key: string, raw: string): FilterClause => ({ key, op: 'eq', raw }); + const rx = (key: string, src: string): FilterClause => ({ + key, + op: 'regex', + raw: src, + regex: new RegExp(src, 'i'), + }); + + it('sub is a case-insensitive substring match', () => { + expect(matchClause('Color Bulb', sub('type', 'color'))).toBe(true); + expect(matchClause('Color Bulb', sub('type', 'BULB'))).toBe(true); + expect(matchClause('Color Bulb', sub('type', 'neon'))).toBe(false); + }); + + it('eq is substring for non-exact keys', () => { + expect(matchClause('Bot Plus', eq('type', 'Bot'))).toBe(true); + expect(matchClause('Color Bulb', eq('type', 'neon'))).toBe(false); + }); + + it('eq is exact (case-insensitive) for "category" by default', () => { + expect(matchClause('physical', eq('category', 'physical'))).toBe(true); + expect(matchClause('physical', eq('category', 'phys'))).toBe(false); + expect(matchClause('IR', eq('category', 'ir'))).toBe(true); + }); + + it('regex.test against the raw candidate (not lowercased)', () => { + expect(matchClause('Bot Plus', rx('type', 'Bot.*'))).toBe(true); + expect(matchClause('Air Conditioner', rx('type', '^Air'))).toBe(true); + expect(matchClause('Air Conditioner', rx('type', 'conditioner'))).toBe(true); + expect(matchClause('TV', rx('type', 'conditioner'))).toBe(false); + }); + + it('undefined candidate never matches', () => { + expect(matchClause(undefined, sub('family', 'Home'))).toBe(false); + expect(matchClause(undefined, eq('category', 'physical'))).toBe(false); + }); + + it('custom exactKeys can make any key exact', () => { + expect( + matchClause('Bot Plus', eq('type', 'Bot'), { exactKeys: ['type'] }), + ).toBe(false); + expect( + matchClause('Bot', eq('type', 'Bot'), { exactKeys: ['type'] }), + ).toBe(true); + }); +}); + describe('applyFilter', () => { it('returns every candidate when the clause list is empty', () => { const all = applyFilter([], devices, irRemotes, hubLoc); expect(all.map((d) => d.deviceId).sort()).toEqual( - ['AC1', 'BOT1', 'BOT2', 'LAMP', 'METER', 'TV1'] + ['AC1', 'BOT1', 'BOT2', 'BOT3', 'LAMP', 'METER', 'TV1'], ); }); - it('filters by exact type on physical devices', () => { + it('type=Bot is now a substring match (was exact in <=2.5.0) โ€” also hits Bot Plus', () => { const matched = applyFilter(parseFilter('type=Bot'), devices, irRemotes, hubLoc); - expect(matched.map((d) => d.deviceId).sort()).toEqual(['BOT1', 'BOT2']); + expect(matched.map((d) => d.deviceId).sort()).toEqual(['BOT1', 'BOT2', 'BOT3']); }); - it('substring match with ~= is case-insensitive', () => { - const matched = applyFilter(parseFilter('type~=light'), devices, irRemotes, hubLoc); - // "Color Bulb" doesn't contain "light", so only the IR remotes that do โ€” none here. - // Let's check against a real substring. - const meter = applyFilter(parseFilter('type~=met'), devices, irRemotes, hubLoc); + it('substring match with ~ is case-insensitive', () => { + const meter = applyFilter(parseFilter('type~met'), devices, irRemotes, hubLoc); expect(meter.map((d) => d.deviceId)).toEqual(['METER']); - expect(matched).toEqual([]); // Color Bulb / Meter / Bot / TV / AC: none contain 'light' + const bulb = applyFilter(parseFilter('type~bulb'), devices, irRemotes, hubLoc); + expect(bulb.map((d) => d.deviceId)).toEqual(['LAMP']); + }); + + it('regex filter supports alternation', () => { + const matched = applyFilter( + parseFilter('type=/Bulb|Meter/'), + devices, + irRemotes, + hubLoc, + ); + expect(matched.map((d) => d.deviceId).sort()).toEqual(['LAMP', 'METER']); }); it('AND-joins multiple clauses', () => { @@ -86,7 +193,7 @@ describe('applyFilter', () => { parseFilter('type=Bot,room=Office'), devices, irRemotes, - hubLoc + hubLoc, ); expect(matched.map((d) => d.deviceId)).toEqual(['BOT2']); }); @@ -96,14 +203,19 @@ describe('applyFilter', () => { expect(matched.map((d) => d.deviceId).sort()).toEqual(['AC1', 'METER']); }); - it('filters by category=ir', () => { + it('filters by category=ir (exact, never substring)', () => { const matched = applyFilter(parseFilter('category=ir'), devices, irRemotes, hubLoc); expect(matched.map((d) => d.deviceId).sort()).toEqual(['AC1', 'TV1']); }); - it('filters by category=physical', () => { + it('filters by category=physical (exact)', () => { const matched = applyFilter(parseFilter('category=physical'), devices, irRemotes, hubLoc); - expect(matched.map((d) => d.deviceId).sort()).toEqual(['BOT1', 'BOT2', 'LAMP', 'METER']); + expect(matched.map((d) => d.deviceId).sort()).toEqual(['BOT1', 'BOT2', 'BOT3', 'LAMP', 'METER']); + }); + + it('category=phys (substring prefix) returns empty because category is exact', () => { + const matched = applyFilter(parseFilter('category=phys'), devices, irRemotes, hubLoc); + expect(matched).toEqual([]); }); it('returns empty when a clause has no matches', () => { diff --git a/tests/utils/flags.test.ts b/tests/utils/flags.test.ts index f437d51e..5cc7795e 100644 --- a/tests/utils/flags.test.ts +++ b/tests/utils/flags.test.ts @@ -4,6 +4,7 @@ import { isDryRun, getTimeout, getConfigPath, + parseDurationToMs, } from '../../src/utils/flags.js'; describe('utils/flags', () => { @@ -81,4 +82,32 @@ describe('utils/flags', () => { expect(getConfigPath()).toBeUndefined(); }); }); + + describe('parseDurationToMs', () => { + it('accepts ms / s / m / h units', () => { + expect(parseDurationToMs('500ms')).toBe(500); + expect(parseDurationToMs('30s')).toBe(30_000); + expect(parseDurationToMs('5m')).toBe(5 * 60_000); + expect(parseDurationToMs('2h')).toBe(2 * 60 * 60_000); + }); + + it('accepts d (days) and w (weeks) units', () => { + expect(parseDurationToMs('1d')).toBe(24 * 60 * 60_000); + expect(parseDurationToMs('7d')).toBe(7 * 24 * 60 * 60_000); + expect(parseDurationToMs('1w')).toBe(7 * 24 * 60 * 60_000); + expect(parseDurationToMs('2w')).toBe(14 * 24 * 60 * 60_000); + }); + + it('treats bare numbers as milliseconds', () => { + expect(parseDurationToMs('1000')).toBe(1000); + }); + + it('rejects unsupported units and malformed values', () => { + expect(parseDurationToMs('1y')).toBeNull(); + expect(parseDurationToMs('1year')).toBeNull(); + expect(parseDurationToMs('1month')).toBeNull(); + expect(parseDurationToMs('abc')).toBeNull(); + expect(parseDurationToMs('')).toBeNull(); + }); + }); }); diff --git a/tests/utils/name-resolver.test.ts b/tests/utils/name-resolver.test.ts index 2e1d062b..f010acc8 100644 --- a/tests/utils/name-resolver.test.ts +++ b/tests/utils/name-resolver.test.ts @@ -5,7 +5,7 @@ import { resolveDeviceId } from '../../src/utils/name-resolver.js'; import { updateCacheFromDeviceList, resetListCache } from '../../src/devices/cache.js'; import { saveDeviceMeta } from '../../src/devices/device-meta.js'; -vi.mock('../../src/utils/flags.js', () => ({ getConfigPath: () => undefined })); +vi.mock('../../src/utils/flags.js', () => ({ getConfigPath: () => undefined, getProfile: () => undefined })); const sampleBody = { deviceList: [ diff --git a/tests/utils/output.test.ts b/tests/utils/output.test.ts index 58a65a59..05b6a739 100644 --- a/tests/utils/output.test.ts +++ b/tests/utils/output.test.ts @@ -199,7 +199,7 @@ describe('handleError', () => { expect(() => handleError(new ApiError('x', 190))).toThrow('__exit'); const joined = errSpy.mock.calls.map((c) => String(c[0])).join('\n'); expect(joined).toContain('Hint:'); - expect(joined).toMatch(/devices list|devices describe/); + expect(joined).toMatch(/generic internal error/); }); it('does not print a hint for unknown/unmapped codes', async () => { @@ -231,77 +231,83 @@ describe('handleError', () => { describe('--json mode', () => { let originalArgv: string[]; + let originalIsTTY: boolean | undefined; beforeEach(() => { originalArgv = process.argv; process.argv = ['node', 'cli', '--json', 'devices', 'status', 'X']; + originalIsTTY = process.stderr.isTTY; + // Force non-TTY so the human-readable stderr mirror is suppressed and + // the structured JSON envelope is the only output we assert on. + (process.stderr as { isTTY?: boolean }).isTTY = false; }); afterEach(() => { process.argv = originalArgv; + (process.stderr as { isTTY?: boolean }).isTTY = originalIsTTY; }); - it('outputs structured JSON error to stderr for ApiError', async () => { + it('outputs structured JSON error to stdout for ApiError (bug #SYS-1)', async () => { const { ApiError } = await import('../../src/api/client.js'); - const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('__exit'); }); expect(() => handleError(new ApiError('bad device', 190))).toThrow('__exit'); - const raw = errSpy.mock.calls[0][0]; + const raw = logSpy.mock.calls[0][0]; const parsed = JSON.parse(raw); expect(parsed.schemaVersion).toBe('1.1'); expect(parsed.error.code).toBe(190); expect(parsed.error.message).toBe('bad device'); - expect(parsed.error.hint).toMatch(/devices/); + expect(parsed.error.hint).toMatch(/generic internal error/); }); it('marks 429 errors as retryable when ApiError.retryable is true', async () => { const { ApiError } = await import('../../src/api/client.js'); - const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('__exit'); }); // Simulate what client.ts creates: retryable: true set explicitly. expect(() => handleError(new ApiError('rate limited', 429, { retryable: true, hint: 'check quota' }))).toThrow('__exit'); - const parsed = JSON.parse(errSpy.mock.calls[0][0]); + const parsed = JSON.parse(logSpy.mock.calls[0][0]); expect(parsed.error.retryable).toBe(true); expect(parsed.error.hint).toBe('check quota'); }); it('prefers ApiError.hint over errorHint fallback when both exist', async () => { const { ApiError } = await import('../../src/api/client.js'); - const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('__exit'); }); // code 429 has an errorHint, but the explicit hint should win. expect(() => handleError(new ApiError('over limit', 429, { retryable: true, hint: 'custom hint from client' }))).toThrow('__exit'); - const parsed = JSON.parse(errSpy.mock.calls[0][0]); + const parsed = JSON.parse(logSpy.mock.calls[0][0]); expect(parsed.error.hint).toBe('custom hint from client'); }); it('does NOT set retryable when ApiError.retryable is false', async () => { const { ApiError } = await import('../../src/api/client.js'); - const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('__exit'); }); expect(() => handleError(new ApiError('auth failed', 401, { retryable: false }))).toThrow('__exit'); - const parsed = JSON.parse(errSpy.mock.calls[0][0]); + const parsed = JSON.parse(logSpy.mock.calls[0][0]); expect(parsed.error.retryable).toBeUndefined(); }); it('outputs structured JSON error for generic Error', () => { - const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('__exit'); }); expect(() => handleError(new Error('kaboom'))).toThrow('__exit'); - const parsed = JSON.parse(errSpy.mock.calls[0][0]); + const parsed = JSON.parse(logSpy.mock.calls[0][0]); expect(parsed.error.code).toBe(1); expect(parsed.error.message).toBe('kaboom'); }); @@ -349,4 +355,73 @@ describe('buildErrorPayload', () => { expect(p.hint).toContain('deviceId'); expect(p.transient).toBe(false); }); + + it('ApiError code 190 โ†’ subKind device-internal-error (bug #27)', async () => { + const { ApiError } = await import('../../src/api/client.js'); + const p = buildErrorPayload(new ApiError('internal error', 190)); + expect(p.subKind).toBe('device-internal-error'); + expect(p.hint).toMatch(/generic internal error/); + }); + + it('ApiError code 3005 โ†’ subKind command-not-supported with --type customize hint (bug #29)', async () => { + const { ApiError } = await import('../../src/api/client.js'); + const p = buildErrorPayload(new ApiError('invalid value', 3005, { transient: false })); + expect(p.subKind).toBe('command-not-supported'); + expect(p.hint).toMatch(/--type customize/); + }); +}); + +describe('handleError under --json (bug #SYS-1)', () => { + const originalArgv = process.argv; + const originalIsTTY = process.stderr.isTTY; + beforeEach(() => { + process.argv = ['node', 'test', '--json']; + }); + afterEach(() => { + process.argv = originalArgv; + (process.stderr as { isTTY?: boolean }).isTTY = originalIsTTY; + vi.restoreAllMocks(); + }); + + it('routes the structured error envelope to stdout, not stderr', () => { + (process.stderr as { isTTY?: boolean }).isTTY = false; + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('__exit'); + }) as never); + + expect(() => handleError(new Error('boom'))).toThrow('__exit'); + expect(logSpy).toHaveBeenCalledTimes(1); + const payload = JSON.parse(String(logSpy.mock.calls[0][0])); + expect(payload.schemaVersion).toBe(SCHEMA_VERSION); + expect(payload.error.message).toBe('boom'); + expect(payload.error.kind).toBe('runtime'); + // In non-TTY mode, stderr stays clean so `cli --json | jq` is unpolluted. + expect(errSpy).not.toHaveBeenCalled(); + }); + + it('emits a human-readable one-liner on stderr when stderr is a TTY', () => { + (process.stderr as { isTTY?: boolean }).isTTY = true; + vi.spyOn(console, 'log').mockImplementation(() => {}); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('__exit'); + }) as never); + + expect(() => handleError(new Error('ttys eek'))).toThrow('__exit'); + expect(errSpy).toHaveBeenCalled(); + expect(String(errSpy.mock.calls[0][0])).toContain('ttys eek'); + }); + + it('preserves exit code 2 for usage errors under --json', () => { + (process.stderr as { isTTY?: boolean }).isTTY = false; + vi.spyOn(console, 'log').mockImplementation(() => {}); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('__exit'); + }) as never); + + expect(() => handleError(new UsageError('bad input'))).toThrow('__exit'); + expect(exitSpy).toHaveBeenCalledWith(2); + }); });