Skip to content

Commit 375d297

Browse files
chenliuyunchenliuyun
andauthored
Fix real-device follow-ups and harden JSON contracts (#38)
* Fix cache-backed device visibility and small command UX regressions * Tighten audit semantics and suggestion safeguards * Make policy validation and migration contract truthful * Polish catalog, status/watch, and operator tooling * Tighten policy validation and status-sync preflight * Finish residual diagnostics and contract coverage * Fix MCP missing-device fallback and lock JSON shapes * Add catalog fidelity fixtures and contract helpers * Harden JSON contracts and release smoke coverage * Expand contract coverage and clean release smoke * Expand low-frequency contracts and catalog fixtures * Align policy rule fixtures with current validation * Correct release-notes registry and pin self-review contracts Self-review of the 3.3.1 follow-up rollup surfaced a false breaking-change record and several contracts that were only held in place by careful reading. This commit corrects the record and pins each contract with a test that fails loudly if it drifts. - src/version-notes.ts: remove the 3.3.0 envelope entry from RELEASE_METADATA. The {schemaVersion,data} envelope actually shipped in commit 33d3825 (v2.0.0) and has been in every 3.x release; flagging 3.3.0 as the breaking boundary would fire upgrade-check warnings on every 3.2.x → 3.3.x upgrade for no reason. - tests/commands/upgrade-check.test.ts + tests/commands/doctor.test.ts: update the asserted contract to match — empty registry returns null; doctor release-notes check reports 'ok' for a version with no notice. - tests/policy/validate.test.ts: add parametric coverage for pathological rule device refs (undefined, empty string, literal `<id>`) against validateLoadedPolicyAgainstInventory. The live validator's silent `continue` on null-resolved refs is only safe because the offline base pass catches the same shapes; the new test couples the two so weakening either side is caught immediately. - tests/utils/audit.test.ts: spy ENOSPC and EACCES fs failures into writeAudit and assert it does not throw. Several callers (dry-run path in src/lib/devices.ts in particular) rely on this being best-effort; the contract is now a test, not a comment. - src/policy/validate.ts: add clarifying comments on the permissive offline device-ID regex (real gate is alias-live-device-not-found) and on the findCatalogEntry ambiguous-array branch (rare, silence beats false positives against drifting upstream types). - package.json: rename `test:release-smoke` → `test:release-smoke:manual` so future maintainers do not assume it runs in prepublishOnly or CI. - package.json + package-lock.json + README.md: bump package version to match the branch-intended rollup and refresh the upgrade-check example. * Fix status-sync --probe preflight and parent-command option inheritance Three follow-up fixes surfaced by deeper review of the 3.3.1 rollup. Each was producing a user-visible regression despite passing tests, because the existing tests pinned the wrong contract. - src/status-sync/manager.ts: the --probe preflight was hitting the OpenClaw base URL with GET and treating any response (including 401 / 404 / 5xx from upstream proxies) as success, so misconfigurations surfaced only after the detached daemon had already been spawned and was silently dropping writes. Probe now POSTs to the same endpoint the sink uses at runtime (`/v1/chat/completions`), with a body the server actually parses, and separates network-error handling from HTTP-error handling. Status-specific hints (401/403 → token, 404 → URL path, 400/422 → model, 5xx → server) point at the misconfigured input so the operator does not have to guess. - src/commands/schema.ts + src/commands/health.ts: the bare fallback forms (`switchbot schema --type Bot`, `switchbot health --prometheus`) were previously declared with options on the root command, on the subcommand, or on both — none of which worked, because commander v12 routes parsing of a parent/child dual-declared option to the PARENT command, leaving the subcommand action with empty opts. Options are now declared only on the root; subcommand actions pull them via `cmd.optsWithGlobals()`. The bare and explicit forms now behave identically, and `health serve --audit-log` still works because optsWithGlobals merges parent options into serve's own port/host. - tests/status-sync/manager.test.ts: default fetch mock flipped from 401-returning to 200-returning so existing success-path tests no longer accidentally exercised the failure hint. Three new tests pin the probe contract: POST to /v1/chat/completions with the model in the body, 401 surfaces a token hint, 404 surfaces a URL-path hint, and trailing slashes on the base URL are trimmed before appending the probe path. * Validate trigger and condition device refs --------- Co-authored-by: chenliuyun <chenliuyun@onero.com>
1 parent 40ec757 commit 375d297

73 files changed

Lines changed: 3792 additions & 666 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ Queries the npm registry for the latest published version and compares it agains
894894

895895
```json
896896
{
897-
"current": "3.3.0",
897+
"current": "3.3.1",
898898
"latest": "4.0.0",
899899
"upToDate": false,
900900
"updateAvailable": true,

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@switchbot/openapi-cli",
3-
"version": "3.3.0",
3+
"version": "3.3.1",
44
"description": "SwitchBot smart home CLI — control devices, run scenes, stream real-time events, and integrate AI agents via MCP. Full API v1.1 coverage.",
55
"keywords": [
66
"switchbot",
@@ -48,6 +48,7 @@
4848
"test": "vitest run",
4949
"test:watch": "vitest",
5050
"test:coverage": "vitest run --coverage",
51+
"test:release-smoke:manual": "npm test -- tests/commands/policy.test.ts tests/commands/devices.test.ts tests/commands/explain.test.ts tests/commands/doctor.test.ts tests/commands/mcp.test.ts tests/commands/health-check.test.ts tests/commands/quota.test.ts tests/commands/status-sync.test.ts tests/status-sync/smoke.test.ts tests/commands/watch.test.ts tests/commands/events.test.ts tests/devices/catalog-fidelity.test.ts tests/commands/schema.test.ts tests/commands/auth.test.ts tests/commands/config.test.ts tests/commands/scenes.test.ts tests/commands/batch.test.ts tests/commands/history.test.ts tests/commands/expand.test.ts tests/commands/webhook.test.ts tests/commands/daemon.test.ts tests/commands/upgrade-check.test.ts tests/commands/install.test.ts tests/commands/uninstall.test.ts tests/commands/rules.test.ts tests/commands/plan.test.ts",
5152
"verify:pre-commit": "npm run build && npm test -- tests/version.test.ts",
5253
"verify:pre-push": "npm run build && npm test -- tests/version.test.ts && npm run smoke:pack-install",
5354
"prepublishOnly": "npm test && npm run build && npm run smoke:pack-install"

src/commands/batch.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,6 @@ Examples:
364364
: undefined,
365365
}));
366366
const planDoc = {
367-
schemaVersion: '1.1',
368367
dryRun: true,
369368
plan: {
370369
command: cmd,
@@ -503,7 +502,6 @@ Examples:
503502
skipped: dryRunned.length + preSkipped.length,
504503
durationMs: Date.now() - startedAt,
505504
unverifiableCount: succeeded.filter((s) => getCachedDevice(s.deviceId)?.category === 'ir').length,
506-
schemaVersion: '1.1',
507505
maxConcurrent: concurrency,
508506
staggerMs,
509507
...(dryRun ? { dryRun: true } : {}),

src/commands/devices.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,30 @@ const EXPAND_HINTS: Record<string, { command: string; flags: string }> = {
4444
'Relay Switch 2PM': { command: 'setMode', flags: '--channel 1 --mode edge' },
4545
};
4646

47+
function annotateStatusPayload(deviceId: string, body: Record<string, unknown>): Record<string, unknown> {
48+
const annotated = { ...body };
49+
if (Object.keys(body).length === 0) {
50+
annotated.supported = false;
51+
annotated.note = 'this device does not expose cloud status';
52+
return annotated;
53+
}
54+
55+
const cached = getCachedDevice(deviceId);
56+
const looksLikeMeter = cached?.type?.toLowerCase().includes('meter') ?? false;
57+
const staleZeroReading =
58+
looksLikeMeter &&
59+
!Object.prototype.hasOwnProperty.call(body, 'onlineStatus') &&
60+
body.battery === 0 &&
61+
body.temperature === 0 &&
62+
body.humidity === 0;
63+
64+
if (staleZeroReading) {
65+
annotated.hint = 'readings look stale; check batteries or hub connectivity';
66+
}
67+
68+
return annotated;
69+
}
70+
4771
export function registerDevicesCommand(program: Command): void {
4872
const COMMAND_TYPES = ['command', 'customize'] as const;
4973
const devices = program
@@ -303,7 +327,7 @@ Examples:
303327
const fetchedAt = new Date().toISOString();
304328
const batch = results.map((r, i) =>
305329
r.status === 'fulfilled'
306-
? { deviceId: ids[i], ok: true, _fetchedAt: fetchedAt, ...(r.value as object) }
330+
? { deviceId: ids[i], ok: true, _fetchedAt: fetchedAt, ...annotateStatusPayload(ids[i], r.value as Record<string, unknown>) }
307331
: { deviceId: ids[i], ok: false, error: (r.reason as Error)?.message ?? String(r.reason) },
308332
);
309333
const batchFmt = resolveFormat();
@@ -342,7 +366,7 @@ Examples:
342366
category: options.nameCategory,
343367
room: options.nameRoom,
344368
});
345-
const body = await fetchDeviceStatus(deviceId);
369+
const body = annotateStatusPayload(deviceId, await fetchDeviceStatus(deviceId));
346370
const fetchedAt = new Date().toISOString();
347371
const fmt = resolveFormat();
348372

@@ -678,7 +702,7 @@ Examples:
678702
if (isJsonMode()) {
679703
printJson({ dryRun: true, wouldSend });
680704
} else {
681-
console.log(`[dry-run] Would POST devices/${_deviceId}/commands with ${JSON.stringify({ command: _cmd, parameter: _parsedParam, commandType })}`);
705+
console.log(`dry-run intercepted for ${_cmd} on ${_deviceId}; see stderr preview for the HTTP request.`);
682706
}
683707
return;
684708
}
@@ -856,6 +880,8 @@ Examples:
856880
capabilities,
857881
source,
858882
suggestedActions: picks,
883+
...(result.catalogNote ? { catalogNote: result.catalogNote } : {}),
884+
...(result.warnings && result.warnings.length > 0 ? { warnings: result.warnings } : {}),
859885
...(expandHint ? { expandHint: { command: expandHint.command, flags: expandHint.flags, example: `switchbot devices expand ${deviceId} ${expandHint.command} ${expandHint.flags}` } } : {}),
860886
});
861887
return;
@@ -893,8 +919,17 @@ Examples:
893919
capabilities && 'liveStatus' in capabilities ? capabilities.liveStatus : undefined;
894920

895921
console.log('');
922+
if (result.warnings && result.warnings.length > 0) {
923+
for (const warning of result.warnings) {
924+
console.log(`Warning: ${warning}`);
925+
}
926+
console.log('');
927+
}
896928
if (!catalog) {
897929
console.log(`(Type "${typeName}" is not in the built-in catalog — no command reference available.)`);
930+
if (result.catalogNote) {
931+
console.log(result.catalogNote);
932+
}
898933
if (isPhysical) {
899934
console.log(`Try 'switchbot devices status ${deviceId}' to see what this device reports.`);
900935
} else {
@@ -995,6 +1030,7 @@ function renderCatalogEntry(entry: DeviceCatalogEntry): void {
9951030
if (entry.statusFields && entry.statusFields.length > 0) {
9961031
console.log('\nStatus fields (from "devices status"):');
9971032
console.log(' ' + entry.statusFields.join(', '));
1033+
console.log(' Note: statusFields are advisory; actual fields can vary by firmware and device variant.');
9981034
}
9991035

10001036
const expandHint = EXPAND_HINTS[entry.type];

src/commands/doctor.ts

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ import { execSync } from 'node:child_process';
66
import { printJson, isJsonMode, exitWithError } from '../utils/output.js';
77
import { getEffectiveCatalog } from '../devices/catalog.js';
88
import { configFilePath, listProfiles, readProfileMeta } from '../config.js';
9-
import { describeCache, resetListCache } from '../devices/cache.js';
9+
import { describeCache, resetListCache, loadCache } from '../devices/cache.js';
1010
import { DAILY_QUOTA, todayUsage } from '../utils/quota.js';
1111
import { AGENT_BOOTSTRAP_SCHEMA_VERSION } from './agent-bootstrap.js';
1212
import { CATALOG_SCHEMA_VERSION } from '../devices/catalog.js';
1313
import { createSwitchBotMcpServer, listRegisteredTools } from './mcp.js';
14+
import { getReleaseMetadata } from '../version-notes.js';
15+
import { VERSION as currentVersion } from '../version.js';
1416
import {
1517
resolvePolicyPath,
1618
loadPolicyFile,
@@ -355,10 +357,56 @@ interface AuditRecord {
355357
kind?: string;
356358
deviceId?: string;
357359
command?: string;
358-
result?: 'ok' | 'error';
360+
result?: 'ok' | 'error' | 'dry-run';
359361
error?: string;
360362
}
361363

364+
function checkInventoryConsistency(): Check {
365+
const cache = loadCache();
366+
if (!cache) {
367+
return {
368+
name: 'inventory',
369+
status: 'ok',
370+
detail: "no local inventory cache — run 'switchbot devices list' to enable hub-reference checks",
371+
};
372+
}
373+
374+
const dangling = Object.entries(cache.devices)
375+
.filter(([, device]) => device.category === 'physical')
376+
.filter(([deviceId, device]) => {
377+
const hubDeviceId = device.hubDeviceId;
378+
return Boolean(
379+
hubDeviceId &&
380+
hubDeviceId !== '000000000000' &&
381+
hubDeviceId !== deviceId &&
382+
!cache.devices[hubDeviceId],
383+
);
384+
})
385+
.map(([deviceId, device]) => ({
386+
deviceId,
387+
deviceName: device.name,
388+
hubDeviceId: device.hubDeviceId!,
389+
deviceType: device.type,
390+
}));
391+
392+
if (dangling.length === 0) {
393+
return {
394+
name: 'inventory',
395+
status: 'ok',
396+
detail: `inventory graph consistent across ${Object.keys(cache.devices).length} cached devices`,
397+
};
398+
}
399+
400+
return {
401+
name: 'inventory',
402+
status: 'warn',
403+
detail: {
404+
message: `${dangling.length} device(s) reference a hubDeviceId that is not present in the current inventory`,
405+
dangling: dangling.slice(0, 10),
406+
},
407+
};
408+
}
409+
362410
function checkAudit(): Check {
363411
// P9: surface recent command failures so agents / ops can spot problems
364412
// before they page. When --audit-log was never enabled, the file won't
@@ -853,6 +901,27 @@ function checkMcp(): Check {
853901
}
854902
}
855903

904+
function checkReleaseNotes(): Check {
905+
const meta = getReleaseMetadata(currentVersion);
906+
if (!meta || !meta.breaking) {
907+
return {
908+
name: 'release-notes',
909+
status: 'ok',
910+
detail: { version: currentVersion, message: 'no known breaking-change notice for the current release' },
911+
};
912+
}
913+
return {
914+
name: 'release-notes',
915+
status: 'warn',
916+
detail: {
917+
version: currentVersion,
918+
breaking: true,
919+
message: meta.summary,
920+
hint: 'If you have scripts pinned to 3.2.x JSON output, update them before rolling this release wider.',
921+
},
922+
};
923+
}
924+
856925
interface CheckDef {
857926
name: string;
858927
description: string;
@@ -871,6 +940,7 @@ const CHECK_REGISTRY: CheckDef[] = [
871940
{ name: 'profiles', description: 'profile definitions valid', run: () => checkProfiles() },
872941
{ name: 'catalog', description: 'catalog loads', run: () => checkCatalog() },
873942
{ name: 'catalog-schema', description: 'catalog vs agent-bootstrap version aligned', run: () => checkCatalogSchema() },
943+
{ name: 'inventory', description: 'cached inventory graph consistency (hubDeviceId references)', run: () => checkInventoryConsistency() },
874944
{ name: 'cache', description: 'device cache state', run: () => checkCache() },
875945
{ name: 'quota', description: 'API quota headroom', run: () => checkQuotaFile() },
876946
{ name: 'clock', description: 'system clock skew', run: () => checkClockSkew() },
@@ -880,6 +950,7 @@ const CHECK_REGISTRY: CheckDef[] = [
880950
run: ({ probe }) => (probe ? checkMqttProbe() : checkMqtt()),
881951
},
882952
{ name: 'mcp', description: 'MCP server instantiable + tool count', run: () => checkMcp() },
953+
{ name: 'release-notes', description: 'current release breaking-change notice', run: () => checkReleaseNotes() },
883954
{ name: 'policy', description: 'policy.yaml present + schema-valid (if configured)', run: () => checkPolicy() },
884955
{ name: 'audit', description: 'recent command errors (last 24h)', run: () => checkAudit() },
885956
{ name: 'daemon', description: 'daemon state file + runtime status', run: () => checkDaemon() },

src/commands/events.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ interface EventRecord {
7474
matched: boolean;
7575
}
7676

77+
function emitJsonStreamRecord<T extends { schemaVersion: string }>(record: T): void {
78+
const { schemaVersion, ...rest } = record as T & Record<string, unknown>;
79+
printJson({
80+
payloadVersion: schemaVersion,
81+
...rest,
82+
});
83+
}
84+
7785
function matchFilterDetail(
7886
body: unknown,
7987
clauses: FilterClause[] | null,
@@ -262,7 +270,7 @@ Examples:
262270
if (!ev.matched) return;
263271
matchedCount++;
264272
if (isJsonMode()) {
265-
printJson(ev);
273+
emitJsonStreamRecord(ev);
266274
} else {
267275
const when = new Date(ev.t).toLocaleTimeString();
268276
console.log(`[${when}] ${ev.remote} ${ev.path} ${JSON.stringify(ev.body)}`);
@@ -457,7 +465,7 @@ Examples:
457465
// connected" even when mqtt-tail exits before the broker connects.
458466
if (isJsonMode()) {
459467
const sessionStartAt = new Date().toISOString();
460-
printJson({
468+
emitJsonStreamRecord({
461469
schemaVersion: EVENTS_SCHEMA_VERSION,
462470
source: 'mqtt',
463471
kind: 'control',
@@ -518,7 +526,7 @@ Examples:
518526
payload: parsed,
519527
};
520528
if (isJsonMode()) {
521-
printJson(record);
529+
emitJsonStreamRecord(record);
522530
} else {
523531
console.log(JSON.stringify(record));
524532
}
@@ -554,7 +562,7 @@ Examples:
554562
// Control events always go to stdout as JSONL so consumers that
555563
// filter real events by presence of `payload` can skip them.
556564
if (isJsonMode()) {
557-
printJson(ctl);
565+
emitJsonStreamRecord(ctl);
558566
} else {
559567
console.log(JSON.stringify(ctl));
560568
}

src/commands/expand.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { Command } from 'commander';
2-
import { intArg, stringArg } from '../utils/arg-parsers.js';
2+
import { intArg, stringArg, enumArg } from '../utils/arg-parsers.js';
33
import { handleError, isJsonMode, printJson, UsageError, exitWithError } from '../utils/output.js';
44
import { getCachedDevice } from '../devices/cache.js';
55
import { executeCommand, isDestructiveCommand, getDestructiveReason } from '../lib/devices.js';
66
import { isDryRun } from '../utils/flags.js';
7-
import { resolveDeviceId } from '../utils/name-resolver.js';
7+
import { resolveDeviceId, ALL_STRATEGIES, type NameResolveStrategy } from '../utils/name-resolver.js';
88
import { DryRunSignal } from '../api/client.js';
99
import {
1010
buildAcSetAll,
@@ -22,6 +22,10 @@ export function registerExpandCommand(devices: Command): void {
2222
.argument('[deviceId]', 'Target device ID from "devices list" (or use --name)')
2323
.argument('[command]', 'Command name: setAll (AC), setPosition (Curtain/Blind Tilt), setMode (Relay Switch 2)')
2424
.option('--name <query>', 'Resolve device by fuzzy name instead of deviceId', stringArg('--name'))
25+
.option('--name-strategy <s>', `Name match strategy: ${ALL_STRATEGIES.join('|')} (default: require-unique)`, stringArg('--name-strategy'))
26+
.option('--name-type <type>', 'Narrow --name by device type (e.g. "Curtain", "Air Conditioner")', stringArg('--name-type'))
27+
.option('--name-category <cat>', 'Narrow --name by category: physical|ir', enumArg('--name-category', ['physical', 'ir'] as const))
28+
.option('--name-room <room>', 'Narrow --name by room name (substring match)', stringArg('--name-room'))
2529
.option('--temp <celsius>', 'AC setAll: temperature in Celsius (16-30)', intArg('--temp', { min: 16, max: 30 }))
2630
.option('--mode <mode>', 'AC: auto|cool|dry|fan|heat Curtain: default|performance|silent Relay: toggle|edge|detached|momentary', stringArg('--mode'))
2731
.option('--fan <speed>', 'AC setAll: fan speed auto|low|mid|high', stringArg('--fan'))
@@ -65,6 +69,10 @@ Examples:
6569
commandArg: string | undefined,
6670
options: {
6771
name?: string;
72+
nameStrategy?: string;
73+
nameType?: string;
74+
nameCategory?: 'physical' | 'ir';
75+
nameRoom?: string;
6876
temp?: string; mode?: string; fan?: string; power?: string;
6977
position?: string; direction?: string; angle?: string;
7078
channel?: string; yes?: boolean;
@@ -82,7 +90,12 @@ Examples:
8290
effectiveDeviceIdArg = undefined;
8391
}
8492

85-
deviceId = resolveDeviceId(effectiveDeviceIdArg, options.name);
93+
deviceId = resolveDeviceId(effectiveDeviceIdArg, options.name, {
94+
strategy: (options.nameStrategy as NameResolveStrategy | undefined) ?? 'require-unique',
95+
type: options.nameType,
96+
category: options.nameCategory,
97+
room: options.nameRoom,
98+
});
8699
if (!effectiveCommand) throw new UsageError('A command argument is required (setAll, setPosition, setMode).');
87100

88101
command = effectiveCommand;

0 commit comments

Comments
 (0)