Skip to content

feat(devices): cache deviceId→type locally and pre-validate commands …#1

Merged
chenliuyun merged 1 commit intomainfrom
feat/device-cache-command-validation
Apr 17, 2026
Merged

feat(devices): cache deviceId→type locally and pre-validate commands …#1
chenliuyun merged 1 commit intomainfrom
feat/device-cache-command-validation

Conversation

@chenliuyun
Copy link
Copy Markdown
Collaborator

…against catalog

  • Add src/devices/cache.ts: persists deviceId → {type, name, category} at ~/.switchbot/devices.json (0o600, atomic, sibling to --config override)
  • devices list / describe now refresh the cache
  • devices command pre-validates against the catalog before hitting the API:
    • unsupported command → exit 2 with supported-command list
    • no-param command given a parameter → exit 2 with corrected invocation
    • --type customize bypasses validation (arbitrary IR button names)
    • unknown deviceId / stale cache gracefully fall through
  • Validation is placed outside the action's try/catch so its exit(2) is not re-handled as exit(1) by handleError
  • Add 19 tests (11 cache unit tests + 8 command validation tests)

♻️ Current situation

Describe the current situation. Explain current problems, if there are any. Be as descriptive as possible (e.g., including examples or code snippets).

💡 Proposed solution

Describe the proposed solution and changes. How does it affect the project? How does it affect the internal structure (e.g., refactorings)?

⚙️ Release Notes

Provide a summary of the changes or features from a user's point of view. If there are breaking changes, provide migration guides using code examples of the affected features.

➕ Additional Information

If applicable, provide additional context in this section.

Testing

Which tests were added? Which existing tests were adapted/changed? Which situations are covered, and what edge cases are missing?

Reviewer Nudging

Where should the reviewer start? what is a good entry point?

…against catalog

- Add src/devices/cache.ts: persists deviceId → {type, name, category} at
  ~/.switchbot/devices.json (0o600, atomic, sibling to --config override)
- devices list / describe now refresh the cache
- devices command pre-validates against the catalog before hitting the API:
  * unsupported command → exit 2 with supported-command list
  * no-param command given a parameter → exit 2 with corrected invocation
  * --type customize bypasses validation (arbitrary IR button names)
  * unknown deviceId / stale cache gracefully fall through
- Validation is placed outside the action's try/catch so its exit(2) is
  not re-handled as exit(1) by handleError
- Add 19 tests (11 cache unit tests + 8 command validation tests)
@chenliuyun chenliuyun merged commit 515bd20 into main Apr 17, 2026
3 checks passed
@chenliuyun chenliuyun deleted the feat/device-cache-command-validation branch April 18, 2026 18:29
chenliuyun pushed a commit that referenced this pull request Apr 19, 2026
Covers all seven findings from the post-implementation review plus
the /iot/credential integration fixes found during smoke testing:

1. events stream filter mismatch — `--filter deviceId=` and
   `--filter type=` silently matched nothing because the matcher read
   ctx.deviceMac/ctx.deviceType from the shadow payload, but those
   fields live on webhook bodies. Added matchShadowEventFilter that
   reads top-level deviceId/deviceType from the parsed shadow event.

2. ErrorSubKind extensions — introduced MqttError with subKind values
   mqtt-tls-failed, mqtt-connect-timeout, mqtt-disconnected (wired into
   ErrorSubKind union, buildErrorPayload, handleError). Credential
   fetch now maps 401/429 to ApiError(auth-failed/quota-exceeded) and
   network timeouts to MqttError(mqtt-connect-timeout). Classify
   initial connect failures via classifyMqttConnectError so cert
   errors surface as mqtt-tls-failed.

3. Runtime reconnect loop — previously MqttTlsClient only retried the
   initial connect; mid-session disconnects silently stopped streaming
   with reconnectAttempts and checkConnectionStability sitting as dead
   code. Now attaches a close handler post-connect that drives
   connectWithRetry on drop, exhausting 5 attempts before emitting an
   mqtt-disconnected MqttError to the registered runtime-error
   handler. Sleep between attempts is now abortable, so end() cancels
   a pending backoff immediately instead of waiting out the delay.

4. Credential cache path — replaced the literal '~' fallback with
   os.homedir(). Also clean up the .tmp file if the atomic
   rename fails so we do not leave orphan writes behind.

5. events stream smoke tests — added tests/commands/events-stream.test.ts
   covering extractShadowEvent parsing and the end-to-end filter path
   that broke before finding #1. Added tests/mqtt/errors.test.ts for
   MqttError classification and buildErrorPayload integration.
   Extended credential tests for 401/429/null-body handling.

6. Quota SIGINT/SIGTERM — quota.ts previously registered global
   signal handlers that called process.exit(130/143), short-circuiting
   command-layer cleanup (watch / events stream finally blocks). The
   handlers now only flush the counter; they fall back to the
   conventional exit code only when quota is the sole listener, so
   short one-shot commands keep their old behavior while long-running
   commands retain control of their own exit path.

7. Status cache cross-process staleness — status.json was read into
   a process-local hot cache with no invalidation, so a long-running
   MCP server could not see writes from a concurrent one-shot CLI.
   loadStatusCache now stats mtime before every read and reloads when
   the file has changed on disk; saveStatusCache/clearStatusCache/
   resetStatusCache update the tracked mtime accordingly. Same-process
   reads remain zero-IO when mtime is unchanged.

8. /iot/credential integration — the endpoint rejected POST {} with
   statusCode 190 "param is invalid". The signing convention differs
   from the public OpenAPI: nonce is the literal string "OpenClaw"
   (not a UUID), the HMAC signature is NOT uppercased, the `t` header
   is numeric, and the body requires a 12-char random `instanceId`.
   Response shape is also nested under body.channels.mqtt with
   topics: {status: string} (wrap to single-element array for our
   subscribe path). Error messages surface at the outer `message`
   field, not body.message. Split buildCredentialHeaders from the
   OpenAPI buildAuthHeaders to keep both conventions clean.

9. TLS material encoding — caBase64/certBase64/keyBase64 are a
   misnomer: the /iot/credential response carries literal PEM text
   in those fields. Decoding them as base64 corrupted the material
   ("PEM routines::no start line"). Pass the strings through to mqtt
   as-is. Also align connect options with OpenClaw's reference
   implementation (keepalive: 60, reschedulePings: true) and dispose
   the prior client before reconnecting so stale listeners from a
   dead TCP socket do not leak.

Tests: 697/697 passing (+24 new).
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
…tch (bug #1)

Under require-unique, the exact-name short-circuit at line 73 returned
immediately on the first exact hit, bypassing the ambiguity check entirely.
This meant a query like '空调' on an account with both '空调' (exact) and
'卧室空调' (substring) silently resolved to the exact device instead of
raising ambiguous_name_match (exit 2).

Fix: gate the short-circuit by strategy. For require-unique, the exact hit is
pushed as a score-0 candidate and the full candidate list is assembled before
the ambiguity check runs. Fuzzy/first/exact/prefix/substring behaviour is
unchanged.

Tests added (4 new):
- exact + substring under require-unique → ambiguous (reporter scenario)
- two exact-same-name devices under require-unique → ambiguous
- single exact match, no other matches under require-unique → resolves OK
- fuzzy still short-circuits on exact match (regression pin)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
Document every fix landed in this branch beyond the history-aggregate
feature: bugs #1, #4, #5, #6, #8, #9, #10, #11, #12, #13, #14, #15,
#16, #17, #18 from the OpenClaw v2.4.0 smoke-test report. Call out
the deferred items (#2, #7) explicitly so readers don't assume they
were overlooked.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
…tch (bug #1)

Under require-unique, the exact-name short-circuit at line 73 returned
immediately on the first exact hit, bypassing the ambiguity check entirely.
This meant a query like '空调' on an account with both '空调' (exact) and
'卧室空调' (substring) silently resolved to the exact device instead of
raising ambiguous_name_match (exit 2).

Fix: gate the short-circuit by strategy. For require-unique, the exact hit is
pushed as a score-0 candidate and the full candidate list is assembled before
the ambiguity check runs. Fuzzy/first/exact/prefix/substring behaviour is
unchanged.

Tests added (4 new):
- exact + substring under require-unique → ambiguous (reporter scenario)
- two exact-same-name devices under require-unique → ambiguous
- single exact match, no other matches under require-unique → resolves OK
- fuzzy still short-circuits on exact match (regression pin)
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
Document every fix landed in this branch beyond the history-aggregate
feature: bugs #1, #4, #5, #6, #8, #9, #10, #11, #12, #13, #14, #15,
#16, #17, #18 from the OpenClaw v2.4.0 smoke-test report. Call out
the deferred items (#2, #7) explicitly so readers don't assume they
were overlooked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant