Skip to content

Tuya - Add support for pilot thermostat devices - PR8#2478

Open
Terdious wants to merge 94 commits intoGladysAssistant:masterfrom
Terdious:tuya-pilot-thermostat-support
Open

Tuya - Add support for pilot thermostat devices - PR8#2478
Terdious wants to merge 94 commits intoGladysAssistant:masterfrom
Terdious:tuya-pilot-thermostat-support

Conversation

@Terdious
Copy link
Copy Markdown
Contributor

@Terdious Terdious commented Mar 3, 2026

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

  • If your changes affect the code, did you write the tests?
  • Are tests passing? (npm test on both front/server)
  • Is the linter passing? (npm run eslint on both front/server)
  • Did you run prettier? (npm run prettier on both front/server)
  • If you are adding a new feature/service, did you run the integration comparator? (npm run compare-translations on front)
  • Did you test this pull request in real life? With real devices? If this development is a big feature or a new service, we recommend that you provide a Docker image to the community (forum) for testing before merging.
  • If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
  • If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.
  • Did you add fake requests data for the demo mode (front/src/config/demo.js) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Summary

  • Add support for the Tuya pilot thermostat reported in issue #2475.
  • Support both local 3.5 control and cloud control for this thermostat family, including pilot wire modes, child lock, current temperature, target temperature, power, daily energy index, and window state.
  • Extend the shared Tuya cloud strategy model so one persisted CLOUD_STRATEGY param drives both cloud polling and cloud writes.
  • Add a migration to rename existing Tuya CLOUD_READ_STRATEGY params to CLOUD_STRATEGY.

Details

  • Front (prod):
    • add read-only rendering for pilot wire mode values so running_mode is displayed as a translated label instead of a numeric value
    • extend the pilot wire mode selector with Programming and Thermostat
  • Translations:
    • add the new pilot wire mode labels in English, French, and German
  • Server (prod):
    • add a dedicated pilot-thermostat Tuya device type with explicit cloud and local mappings
    • support thermostat-specific features:
      • mode
      • running_mode
      • temp_set
      • temp_current
      • child_lock
      • average_power
      • electricity_statistics
      • window_state
    • extend Gladys pilot wire constants with Programming and Thermostat
    • add temperature unit normalization and conversion logic for poll/write flows while preserving the Gladys feature unit
    • support cloud writes through Tuya Thing Model shadow properties for devices that use shadow
    • replace separate cloud read/write strategy params with a single persisted CLOUD_STRATEGY
    • add a migration to rename old Tuya CLOUD_READ_STRATEGY params to CLOUD_STRATEGY
    • keep the Tuya feedback poll after command and delay it slightly to better match real device application latency
  • Server tests:
    • add a complete fixture set for pilote-thermostat-c03zek9b5daz7omr
    • cover conversion, local mapping, cloud poll, local poll, and local setValue
    • add regression coverage for:
      • pilot wire mode read/write conversions
      • read-only running_mode
      • temperature conversions
      • Thing Model shadow cloud writes
      • unified cloud strategy persistence and usage
    • align existing Smart Meter and Smart Socket fixtures/tests with the new single CLOUD_STRATEGY param

Why this matters

  • This thermostat exposes a richer Tuya model than the previously supported Smart Socket and Smart Meter devices, including pilot wire modes and mixed local/cloud capabilities.
  • Supporting it through explicit mappings keeps the onboarding model predictable and consistent with the rest of the Tuya stack.
  • Unifying cloud routing behind one persisted CLOUD_STRATEGY keeps the implementation simpler than separate read/write params while still handling Thing Model-only devices correctly.
  • The migration ensures already discovered Tuya devices continue to work after the param rename.

Scope

Compared to tuya-lsc-power-plug-fr-power-meter:

  • Front (prod)

    • +23 lines / -1 lines
  • Translations

    • +18 lines / -6 lines
  • Server (prod)

    • +763 lines / -85 lines
  • Server Tests

    • +1255 lines / -46 lines
  • No package changes.

  • Total

    • 39 files changed
    • +2059 lines / -138 lines

Summary by CodeRabbit

  • New Features

    • Local device scanning/polling and new Tuya status/disconnect endpoints
    • GitHub issue creation flow to report device support payloads
    • Pilot wire mode: added "Programming" and "Thermostat" options
    • Support for smart-meter, smart-socket and pilot-thermostat device types
  • Improvements

    • Richer Tuya setup UI (cloud/local guidance, renamed credential labels, disconnect/status)
    • Show more device details and IP mode/protocol controls
    • Auto-reconnect, manual-disconnect handling, device ranking/sorting and expanded translations (EN/DE/FR)

Terdious and others added 30 commits February 11, 2026 15:58
…er models

- Added support for air conditioning devices with new mappings and DPS configurations.
- Introduced local polling for Tuya devices to improve responsiveness.
- Enhanced device conversion logic to include additional parameters such as cloud IP and local override.
- Updated feature conversion to utilize advanced DPS mappings for air conditioning.
- Implemented new models for air conditioning and power meter, including specific feature mappings.
- Improved error handling and logging for local polling and device value setting.
- Added unit tests for new feature mappings and conversion logic.
… champ d'erreur dans le payload de l'événement WebSocket
…s et ajouter des tests pour la gestion des appareils locaux
…eurs de port et mise à jour des traductions
…re des liens vers la documentation et les options de connexion
…réation de rapports GitHub pour les appareils Tuya
…l disconnect features

- Added new translations for connection status messages in German, English, and French.
- Implemented API endpoints to get Tuya connection status and to manually disconnect from the Tuya cloud.
- Updated the Tuya service to handle automatic reconnection logic and manual disconnect state.
- Enhanced the SetupTab component to reflect connection status and provide a disconnect button.
- Added tests for the new functionality, including status retrieval and manual disconnect.
- Implemented device ranking and sorting in DiscoverTab for better user experience.
- Added loading indicators and improved UI feedback during device scanning.
- Refactored local polling logic to update discovered devices with local information.
- Introduced utility functions for managing device parameters, including upserting and normalizing values.
- Enhanced local scan response handling to merge existing device parameters.
- Updated tests to cover new functionality and ensure reliability of device management.
…age, ajouter des tests pour la reconnexion automatique et la découverte des appareils
… des paramètres dans le code de configuration Tuya
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

Adds extensive Tuya integration: local UDP scanning and local polling (multi-protocol), cloud/local hybrid polling and write paths, new device-type mappings (pilot-thermostat, smart-meter, smart-socket), UI/i18n extensions, GitHub-issue helpers, reconnect/manual-disconnect logic, many utilities, DB migration, and large test/fixture additions.

Changes

Cohort / File(s) Summary
Frontend — Pilot-wire & Sensor UI
front/src/components/boxs/device-in-room/device-features/PilotWireModeDeviceFeature.jsx, front/src/components/boxs/device-in-room/device-features/sensor-value/PilotWireModeDeviceValue.jsx, front/src/components/boxs/device-in-room/device-features/SensorDeviceFeature.jsx
Add new Pilot Wire mode options and a new display component; register PilotWireMode renderer.
Frontend — Tuya UI / i18n
front/src/config/i18n/en.json, front/src/config/i18n/de.json, front/src/config/i18n/fr.json, front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx, front/src/routes/integration/all/tuya/discover-page/DiscoverTab.jsx, front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx, front/src/routes/integration/all/tuya/discover-page/style.css
Add many Tuya i18n keys; UI controls for local/cloud modes, protocol/IP controls, UDP scan, GitHub issue flows, discovery UI and setup/status display.
Frontend — commons helpers
front/src/routes/.../commons/deviceHelpers.js, front/src/routes/.../commons/githubIssue.js
Add device param resolution and GitHub issue payload/URL builders used by Tuya UI flows.
Server API & controller
server/services/tuya/api/tuya.controller.js
Add endpoints: POST /api/v1/service/tuya/local-poll, POST /api/v1/service/tuya/local-scan, GET /api/v1/service/tuya/status, POST /api/v1/service/tuya/disconnect.
Core Tuya service & lifecycle
server/services/tuya/index.js, server/services/tuya/lib/tuya.init.js, server/services/tuya/lib/tuya.connect.js, server/services/tuya/lib/tuya.disconnect.js, server/services/tuya/lib/tuya.getStatus.js, server/services/tuya/lib/tuya.getConfiguration.js, server/services/tuya/lib/tuya.saveConfiguration.js, server/services/tuya/lib/tuya.manualDisconnect.js
Add auto-reconnect/quick-reconnect, config-hash tracking, app-account validation, manual-disconnect handling, status API, and new stored variables (APP_USERNAME, MANUAL_DISCONNECT, LAST_CONNECTED_CONFIG_HASH).
Local ops & networking
server/services/tuya/lib/tuya.localScan.js, server/services/tuya/lib/tuya.localPoll.js, server/services/tuya/lib/device/tuya.localMapping.js
Implement UDP multi-port local scan, local polling via TuyAPI/newgen with retries/timeouts, DPS parsing and local DP resolution, and helpers to add fallback binary features.
Device conversion & mappings
server/services/tuya/lib/device/tuya.convertDevice.js, server/services/tuya/lib/device/tuya.convertFeature.js, server/services/tuya/lib/device/tuya.deviceMapping.js, server/services/tuya/lib/mappings/...
Enrich device conversion: product fields, params, ignored-codes, temperature unit/scale handling; add new cloud/local mapping files (pilot-thermostat, smart-meter, smart-socket) and read/write handlers (including pilot-wire mappings).
Polling & setValue
server/services/tuya/lib/tuya.poll.js, server/services/tuya/lib/tuya.setValue.js
Rewrite polling and setValue for hybrid local/cloud flows, cloud strategy (LEGACY/SHADOW), DPS mapping, unit/scale conversions, feedback polling, and deduplicated event emission.
Mapping framework & utils
server/services/tuya/lib/mappings/index.js, server/services/tuya/lib/mappings/cloud/*.js, server/services/tuya/lib/mappings/local/*.js, server/services/tuya/lib/utils/tuya.cloudStrategy.js, server/services/tuya/lib/utils/tuya.config.js, server/services/tuya/lib/utils/tuya.constants.js, server/services/tuya/lib/utils/tuya.normalize.js, server/services/tuya/lib/utils/tuya.deviceParams.js, server/services/tuya/lib/utils/tuya.report.js
Add device-type resolution, cloud/local mapping accessors, ignored lists, device-param helpers, config-hash utility, cloud strategy resolver, normalization helpers, and structured Tuya reporting utilities.
DB migration
server/migrations/20260303080000-rename-tuya-cloud-strategy-param.js
Migration copying CLOUD_READ_STRATEGYCLOUD_STRATEGY (if present) and removing the old param.
Dependencies & constants
server/services/tuya/package.json, server/utils/constants.js
Add dependencies tuyapi and @demirdeniz/tuyapi-newgen; extend constants: PILOT_WIRE_MODE with PROGRAMMING/THERMOSTAT and WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR; add DEVICE_PARAM_NAME constants.
Tests & fixtures
server/test/services/tuya/fixtures/**, server/test/services/tuya/lib/**, server/test/services/tuya/index.test.js, server/test/services/tuya/tuya.mock.test.js, server/test/services/tuya/fixtures/fixtureHelper.js
Large suite of unit and fixture tests added/updated covering discovery, local/cloud poll, local-scan, setValue, connect/init/disconnect, mapping resolution, tuya report, and many edge cases across smart-socket, smart-meter, and pilot-thermostat.
Misc — small frontend tweaks
front/src/components/...
Minor UI adjustments and a new CSS rule .scanLoader.

Sequence Diagram(s)

sequenceDiagram
    actor Frontend
    participant DeviceBox as TuyaDeviceBox (UI)
    participant Backend as Tuya Service (Server)
    participant LocalPoll as Local Poll Worker
    participant TuyaDevice as Tuya Device (Local)
    participant Cloud as Tuya Cloud API

    Frontend->>DeviceBox: Trigger local poll
    DeviceBox->>Backend: POST /api/v1/service/tuya/local-poll (device params)
    activate Backend
    Backend->>LocalPoll: localPoll(payload)
    activate LocalPoll
    LocalPoll->>TuyaDevice: connect via TuyAPI / TuyAPI-NewGen
    TuyaDevice-->>LocalPoll: return DPS map / error / timeout
    alt DPS received
        LocalPoll-->>Backend: DPS map
        Backend->>Backend: update discovered device, merge params
        Backend-->>DeviceBox: respond with local poll result
        DeviceBox->>Frontend: display local-derived states
    else failure/incomplete
        LocalPoll-->>Backend: error
        Backend->>Cloud: fetch cloud properties (fallback)
        Cloud-->>Backend: cloud property payload
        Backend-->>DeviceBox: respond with cloud states
        DeviceBox->>Frontend: display cloud-derived states
    end
    deactivate LocalPoll
    deactivate Backend
Loading
sequenceDiagram
    actor User
    participant SetupTab as Setup UI
    participant Backend as Tuya Service
    participant TuyaAPI as Tuya Cloud API
    participant Gladys as Gladys Core

    User->>SetupTab: Submit credentials (endpoint, clientId, secret, appAccountId)
    SetupTab->>Backend: save configuration
    activate Backend
    Backend->>TuyaAPI: validate appAccountId (devices request)
    alt valid
        TuyaAPI-->>Backend: success
        Backend->>Gladys: set variables (ENDPOINT, ACCESS_KEY, SECRET_KEY, APP_ACCOUNT_UID, APP_USERNAME, MANUAL_DISCONNECT=false, LAST_CONNECTED_CONFIG_HASH)
        Backend->>Backend: connect() + schedule auto-reconnect
        Backend-->>SetupTab: connected status (error:null)
    else invalid
        TuyaAPI-->>Backend: error
        Backend-->>SetupTab: error + message
    end
    deactivate Backend
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Suggested reviewers

  • Pierre-Gilles

Poem

🐰 I hopped through UDP and cloud-made trails,
I found the keys, the IPs, the tails,
Reconnects set, reports compiled bright,
Thermostats, meters — hum into night,
🥕 Hooray for Tuya's newfound light!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.87%. Comparing base (8db6b0d) to head (034de26).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2478      +/-   ##
==========================================
+ Coverage   98.79%   98.87%   +0.07%     
==========================================
  Files        1006     1025      +19     
  Lines       17478    18670    +1192     
==========================================
+ Hits        17268    18460    +1192     
  Misses        210      210              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@relativeci
Copy link
Copy Markdown

relativeci Bot commented Mar 3, 2026

#3979 Bundle Size — 11.52MiB (+1.11%).

034de26(current) vs 8db6b0d master#3902(baseline)

Warning

Bundle contains 2 duplicate packages – View duplicate packages

Bundle metrics  Change 5 changes Regression 2 regressions
                 Current
#3979
     Baseline
#3902
Regression  Initial JS 6.51MiB(+1.96%) 6.38MiB
Regression  Initial CSS 310.24KiB(+0.02%) 310.18KiB
Change  Cache Invalidation 67.47% 0%
No change  Chunks 51 51
No change  Assets 179 179
Change  Modules 1646(+0.18%) 1643
No change  Duplicate Modules 21 21
Change  Duplicate Code 0.93%(-1.06%) 0.94%
No change  Packages 136 136
No change  Duplicate Packages 2 2
Bundle size by type  Change 2 changes Regression 2 regressions
                 Current
#3979
     Baseline
#3902
Regression  JS 8.41MiB (+1.52%) 8.28MiB
No change  IMG 2.66MiB 2.66MiB
Regression  CSS 328.09KiB (+0.02%) 328.03KiB
No change  Fonts 93.55KiB 93.55KiB
No change  Other 18.82KiB 18.82KiB
No change  HTML 13.58KiB 13.58KiB

Bundle analysis reportBranch Terdious:tuya-pilot-thermostat-s...Project dashboard


Generated by RelativeCIDocumentationReport issue

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (16)
server/test/services/tuya/lib/tuya.getStatus.test.js (1)

17-25: Use a fresh TuyaHandler per test to avoid state bleed.

This keeps tests isolated and prevents future order-dependent failures.

♻️ Proposed refactor
 describe('TuyaHandler.getStatus', () => {
-  const tuyaHandler = new TuyaHandler(gladys, serviceId);
+  let tuyaHandler;
 
   beforeEach(() => {
     sinon.reset();
+    tuyaHandler = new TuyaHandler(gladys, serviceId);
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.getStatus.test.js` around lines 17 - 25,
Tests currently reuse a single TuyaHandler instance (tuyaHandler) across cases
which can cause state bleed; update the setup to instantiate a fresh TuyaHandler
inside beforeEach (and remove any shared top-level instantiation) so each test
gets a new instance, referencing the TuyaHandler constructor used in the file
and ensuring teardown/reset is still run in afterEach (keep sinon.reset()
there).
server/services/tuya/lib/tuya.getStatus.js (1)

11-15: Parallelize Line 11-Line 15 variable reads to reduce endpoint latency.

The reads are independent; Promise.all keeps behavior identical while reducing total wait time.

♻️ Proposed refactor
 async function getStatus() {
-  const endpoint = await this.gladys.variable.getValue(GLADYS_VARIABLES.ENDPOINT, this.serviceId);
-  const accessKey = await this.gladys.variable.getValue(GLADYS_VARIABLES.ACCESS_KEY, this.serviceId);
-  const secretKey = await this.gladys.variable.getValue(GLADYS_VARIABLES.SECRET_KEY, this.serviceId);
-  const appAccountId = await this.gladys.variable.getValue(GLADYS_VARIABLES.APP_ACCOUNT_UID, this.serviceId);
-  const manualDisconnect = await this.gladys.variable.getValue(GLADYS_VARIABLES.MANUAL_DISCONNECT, this.serviceId);
+  const [endpoint, accessKey, secretKey, appAccountId, manualDisconnect] = await Promise.all([
+    this.gladys.variable.getValue(GLADYS_VARIABLES.ENDPOINT, this.serviceId),
+    this.gladys.variable.getValue(GLADYS_VARIABLES.ACCESS_KEY, this.serviceId),
+    this.gladys.variable.getValue(GLADYS_VARIABLES.SECRET_KEY, this.serviceId),
+    this.gladys.variable.getValue(GLADYS_VARIABLES.APP_ACCOUNT_UID, this.serviceId),
+    this.gladys.variable.getValue(GLADYS_VARIABLES.MANUAL_DISCONNECT, this.serviceId),
+  ]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.getStatus.js` around lines 11 - 15, The four
independent variable reads calling this.gladys.variable.getValue with
GLADYS_VARIABLES.ENDPOINT, GLADYS_VARIABLES.ACCESS_KEY,
GLADYS_VARIABLES.SECRET_KEY, and GLADYS_VARIABLES.APP_ACCOUNT_UID (and
MANUAL_DISCONNECT) should be parallelized with Promise.all to reduce latency:
invoke Promise.all on the array of getValue promises (each passing
this.serviceId) and destructure the results into endpoint, accessKey, secretKey,
appAccountId, manualDisconnect so behavior remains identical but the awaits run
concurrently.
server/services/tuya/lib/mappings/local/smart-meter.js (1)

23-37: Consider adding aliases for common forward/reverse spelling variants.

Given the mixed code spellings already present, aliases would make local mapping more resilient across firmware variants without changing current behavior.

♻️ Proposed refactor
-  codeAliases: {},
+  codeAliases: {
+    energy_forward_a: ['energy_forword_a'],
+    energy_forward_b: ['energy_forword_b'],
+    energy_reverse_b: ['energy_reserse_b'],
+  },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/local/smart-meter.js` around lines 23 - 37,
The dps map contains several misspellings (e.g., energy_forword_a,
energy_reserse_b) and you already have a codeAliases object — populate
codeAliases to map the canonical/expected keys to the existing misspelled keys
so behavior stays the same across firmware variants; e.g., map energy_forward_a
-> energy_forword_a, energy_forward_b -> energy_forword_b, energy_reverse_a ->
energy_reverse_a (if needed) and energy_reverse_b -> energy_reserse_b, and also
include total variants like forward_energy_total -> forward_energy_total and
reverse_energy_total -> reverse_energy_total as aliases where spellings differ;
update the codeAliases object in this file so lookups can resolve either
spelling to the existing dps entries.
server/test/services/tuya/lib/tuya.init.test.js (1)

136-154: Restore stubs in finally to prevent cross-test leakage.

If an assertion fails before manual restore, the stubbed methods can bleed into later cases.

Safer stub lifecycle pattern
   it('should connect with null configuration when no config is stored', async () => {
     const connectStub = sinon.stub(tuyaHandler, 'connect').resolves();
     const getConfigurationStub = sinon.stub(tuyaHandler, 'getConfiguration').resolves(null);
@@
-    await tuyaHandler.init();
-
-    expect(tuyaHandler.autoReconnectAllowed).to.equal(false);
-    assert.calledOnce(getConfigurationStub);
-    assert.calledOnce(connectStub);
-    assert.calledWith(connectStub, null);
-
-    connectStub.restore();
-    getConfigurationStub.restore();
+    try {
+      await tuyaHandler.init();
+      expect(tuyaHandler.autoReconnectAllowed).to.equal(false);
+      assert.calledOnce(getConfigurationStub);
+      assert.calledOnce(connectStub);
+      assert.calledWith(connectStub, null);
+    } finally {
+      connectStub.restore();
+      getConfigurationStub.restore();
+    }
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.init.test.js` around lines 136 - 154, The
test currently stubs tuyaHandler.connect and tuyaHandler.getConfiguration but
restores them only at the end of the happy path, which can leak into other tests
if an assertion fails; update the test for the case in the tuya.init.test.js to
ensure connect and getConfiguration stubs are restored in a finally block (or
use a sinon sandbox/afterEach cleanup) around the await tuyaHandler.init() call
so that tuyaHandler.connect and tuyaHandler.getConfiguration are always restored
even on failure.
server/test/services/tuya/lib/tuya.setValue.test.js (1)

31-38: Use a write-response shaped default stub in this suite.

The current default payload (result.list/has_more) is unrelated to command writes and can mask setValue response-handling regressions.

Cleaner default for write-path tests
   beforeEach(() => {
     sinon.reset();
     tuyaHandler.connector = {
-      request: sinon
-        .stub()
-        .onFirstCall()
-        .resolves({ result: { list: [{ id: 1 }], total: 2, has_more: true, last_row_key: 'next' } })
-        .onSecondCall()
-        .resolves({ result: { list: [{ id: 2 }], total: 2, has_more: false } }),
+      request: sinon.stub().resolves({ success: true }),
     };
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.setValue.test.js` around lines 31 - 38,
The test's default stub for tuyaHandler.connector.request is returning a
paginated read-style payload (result.list/has_more) which is irrelevant for
setValue/write-path tests; update the stub used in this suite so its default
resolved value matches a write-response shape used by setValue (e.g., result
indicating success/status rather than list/has_more). Locate the stub on
tuyaHandler.connector.request in the test and change the default .resolves(...)
to a write-style response (and keep any specialized onFirstCall/onSecondCall
variants as needed) so the suite exercises setValue response handling correctly.
server/services/tuya/lib/tuya.setValue.js (1)

23-46: Consider extracting shared helper logic with tuya.poll.js.

getTemperatureUnitFromProperties and getFeatureWithFallbackScale are now duplicated in both runtime modules; centralizing them would reduce drift risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.setValue.js` around lines 23 - 46, Both
getTemperatureUnitFromProperties and getFeatureWithFallbackScale are duplicated;
extract them into a shared helper module and import it from both runtime
modules. Create a new module (e.g., tuyaHelpers) that exports
getTemperatureUnitFromProperties and getFeatureWithFallbackScale (and any
dependent helpers like normalizeTemperatureUnit, getDeviceType,
getFeatureMapping if not already shared), update tuya.setValue.js to import
those functions and remove the local copies, and update tuya.poll.js to import
the same helpers so both files use the centralized implementations.
server/test/services/tuya/lib/controllers/tuya.controller.test.js (1)

88-105: Seed LOCAL_KEY in the fixture before asserting removal.

The current setup only contains IP_ADDRESS, so expect(localKeyParam).to.equal(undefined) will pass even if removal logic regresses.

Proposed test hardening
     tuyaManager.discoveredDevices = [
       {
         external_id: 'tuya:device1',
         product_id: 'pid',
         product_key: 'pkey',
-        params: [{ name: 'IP_ADDRESS', value: '1.1.1.1' }],
+        params: [
+          { name: 'IP_ADDRESS', value: '1.1.1.1' },
+          { name: 'LOCAL_KEY', value: 'old-local-key' },
+        ],
       },
     ];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/controllers/tuya.controller.test.js` around
lines 88 - 105, The test currently seeds tuyaManager.discoveredDevices with only
an IP_ADDRESS so the assertion expect(localKeyParam).to.equal(undefined) is
vacuously true; update the fixture to include a LOCAL_KEY param (e.g., add {
name: 'LOCAL_KEY', value: 'someKey' }) in the discoveredDevices entry before
invoking controller['post /api/v1/service/tuya/local-poll'].controller(req,
res), then keep the assertion that LOCAL_KEY is removed (localKeyParam is
undefined) after the controller runs to ensure removal logic is actually tested.
server/services/tuya/lib/tuya.loadDeviceDetails.js (1)

12-14: Minor: Redundant null check for listDeviceEntry.

At line 14, tuyaDevice ? { ...tuyaDevice } : null is unnecessary since line 13 already destructures tuyaDevice.id, which would throw if tuyaDevice were null/undefined.

♻️ Simplified assignment
 async function loadDeviceDetails(tuyaDevice) {
   const { id: deviceId } = tuyaDevice;
-  const listDeviceEntry = tuyaDevice ? { ...tuyaDevice } : null;
+  const listDeviceEntry = { ...tuyaDevice };
   logger.debug(`Loading ${deviceId} Tuya device specifications`);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.loadDeviceDetails.js` around lines 12 - 14, The
code in loadDeviceDetails performs a redundant null check when assigning
listDeviceEntry using tuyaDevice ? { ...tuyaDevice } : null even though
tuyaDevice is already destructured earlier (const { id: deviceId } =
tuyaDevice), so remove the ternary and simply assign listDeviceEntry = {
...tuyaDevice } (or remove the assignment entirely if unused) to avoid
dead/contradictory null-checks; update any downstream uses accordingly.
server/services/tuya/lib/utils/tuya.report.js (1)

30-68: Consider reducing repetition in createBaseReport.

The repeated currentReport.cloud && currentReport.cloud.assembled checks are verbose. Optional chaining could simplify this, though the current approach is safe and functional.

♻️ Optional simplification using optional chaining
 const createBaseReport = (currentReportInput = {}) => {
   const currentReport = currentReportInput || {};
+  const assembled = currentReport.cloud?.assembled;
+  const raw = currentReport.cloud?.raw;
   return {
     schema_version: currentReport.schema_version || REPORT_SCHEMA_VERSION,
     cloud: {
       assembled: {
-        specifications:
-          currentReport.cloud && currentReport.cloud.assembled
-            ? currentReport.cloud.assembled.specifications || null
-            : null,
-        properties:
-          currentReport.cloud && currentReport.cloud.assembled
-            ? currentReport.cloud.assembled.properties || null
-            : null,
-        thing_model:
-          currentReport.cloud && currentReport.cloud.assembled
-            ? currentReport.cloud.assembled.thing_model || null
-            : null,
+        specifications: assembled?.specifications || null,
+        properties: assembled?.properties || null,
+        thing_model: assembled?.thing_model || null,
       },
       raw: {
-        device_list_entry:
-          currentReport.cloud && currentReport.cloud.raw ? currentReport.cloud.raw.device_list_entry || null : null,
+        device_list_entry: raw?.device_list_entry || null,
         // ... similar for other raw fields
       },
     },
     // ...
   };
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/utils/tuya.report.js` around lines 30 - 68, The
createBaseReport function repeats verbose null-checks like currentReport.cloud
&& currentReport.cloud.assembled; refactor it to use optional chaining and
nullish coalescing (e.g., currentReport?.cloud?.assembled?.specifications ??
null) to simplify each field assignment while preserving the same null fallback
behavior; update references inside createBaseReport (currentReport,
cloud.assembled, cloud.raw, local.scan) and keep REPORT_SCHEMA_VERSION
unchanged.
server/services/tuya/lib/tuya.connect.js (1)

68-77: Minor: Redundant null check on response.

At line 73, the response && guard is unnecessary since you already throw at line 71 when !response. This is a minor clarity improvement.

♻️ Suggested simplification
   if (!response) {
     const error = new Error('TUYA_APP_ACCOUNT_UID_INVALID');
     error.code = 'TUYA_APP_ACCOUNT_UID_INVALID';
     throw error;
   }
-  if (response && response.success === false) {
+  if (response.success === false) {
     const error = new Error(response.msg || response.message || 'TUYA_APP_ACCOUNT_UID_INVALID');
     error.code = response.code || 'TUYA_APP_ACCOUNT_UID_INVALID';
     throw error;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.connect.js` around lines 68 - 77, The second
conditional redundantly checks `response &&` after the prior `if (!response)`
already throws; remove the `response &&` guard and simplify the condition to `if
(response.success === false)` (keeping the existing error construction that uses
`response.msg || response.message || 'TUYA_APP_ACCOUNT_UID_INVALID'` and
`response.code || 'TUYA_APP_ACCOUNT_UID_INVALID'`) so only a falsy
`response.success` triggers the error throw while preserving existing behavior.
server/migrations/20260303080000-rename-tuya-cloud-strategy-param.js (1)

59-59: Add a reversible down migration.

Line 59 is a no-op, so rollback cannot restore previous param naming during incident rollback.

♻️ Suggested reversible down migration
-  down: async () => {},
+  down: async () => {
+    const service = await db.Service.findOne({
+      where: {
+        name: 'tuya',
+      },
+    });
+    if (service === null) {
+      return;
+    }
+
+    const tuyaDevices = await db.Device.findAll({
+      where: {
+        service_id: service.id,
+      },
+    });
+
+    await Promise.each(tuyaDevices, async (device) => {
+      const newParam = await db.DeviceParam.findOne({
+        where: {
+          device_id: device.id,
+          name: NEW_PARAM_NAME,
+        },
+      });
+      if (newParam === null) {
+        return;
+      }
+
+      const existingOldParam = await db.DeviceParam.findOne({
+        where: {
+          device_id: device.id,
+          name: OLD_PARAM_NAME,
+        },
+      });
+
+      if (existingOldParam === null) {
+        await db.DeviceParam.create({
+          device_id: device.id,
+          name: OLD_PARAM_NAME,
+          value: newParam.value,
+        });
+      }
+
+      await newParam.destroy();
+    });
+  },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/migrations/20260303080000-rename-tuya-cloud-strategy-param.js` at line
59, The migration's down handler is a no-op so rollbacks won't restore the
previous parameter names; implement a reversible down migration in the same file
(20260303080000-rename-tuya-cloud-strategy-param.js) by replacing the current
empty async down: () => {} with an async function that reverses the changes made
in the up migration (i.e., rename the param keys back to their original names
using the same table/model operations used in up), ensuring the down uses the
same identifiers and SQL/ORM methods so a rollback restores prior param naming.
server/test/services/tuya/lib/tuya.setValue.fixtures.test.js (1)

46-48: Consider handling missing feature gracefully.

If featureExternalId doesn't match any feature in the device, currentFeature will be undefined, causing the test to fail with a potentially confusing error downstream in setValue. Adding an assertion or guard would make test failures more diagnosable.

💡 Suggested improvement
       const currentDevice = fixtureCase.load(device);
       const currentFeature = currentDevice.features.find((feature) => feature.external_id === featureExternalId);
+      expect(currentFeature, `Feature ${featureExternalId} not found in device fixture`).to.not.be.undefined;
       const ctx = {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.setValue.fixtures.test.js` around lines 46
- 48, The test should guard for a missing feature before calling setValue: after
loading the device with fixtureCase.load and locating currentFeature via
currentDevice.features.find using featureExternalId, add an assertion or
explicit guard that currentFeature is defined (e.g., assert.ok/currentFeature or
throw with a clear message) so the test fails fast and explains that
featureExternalId was not found rather than letting setValue fail later with a
confusing error.
server/test/services/tuya/lib/device/tuya.convertDevice.test.js (1)

63-64: Prefer constants over hardcoded cloud-strategy literals in tests.

Line 63 and Line 129 assert raw strings ('legacy', 'shadow'). Using the exported strategy constants makes these tests less brittle.

♻️ Suggested test hardening
 const { DEVICE_PARAM_NAME } = require('../../../../../services/tuya/lib/utils/tuya.constants');
+const { CLOUD_STRATEGY } = require('../../../../../services/tuya/lib/utils/tuya.cloudStrategy');
@@
-    expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal('legacy');
+    expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal(CLOUD_STRATEGY.LEGACY);
@@
-    expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal('shadow');
+    expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal(CLOUD_STRATEGY.SHADOW);

Also applies to: 129-130

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/device/tuya.convertDevice.test.js` around lines
63 - 64, Replace the hardcoded cloud-strategy string literals used in assertions
(e.g., expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal('legacy') and
the similar 'shadow' assertion) with the exported strategy constants (e.g.,
CLOUD_STRATEGY.LEGACY and CLOUD_STRATEGY.SHADOW); import CLOUD_STRATEGY at the
top of the test alongside DEVICE_PARAM_NAME and update the
expect(...).to.equal(...) calls to reference these constants so the tests use
canonical symbols instead of brittle strings.
server/test/services/tuya/lib/tuya.localScan.test.js (1)

55-68: Always restore fake timers in finally to avoid cross-test leakage.

Several tests restore the clock only on the happy path. If an assertion/promise fails before restore, subsequent tests may run with leaked fake timers.

♻️ Suggested pattern
-const clock = sinon.useFakeTimers();
-const promise = localScan(1);
-// ...
-await clock.tickAsync(1100);
-const result = await promise;
-clock.restore();
+const clock = sinon.useFakeTimers();
+try {
+  const promise = localScan(1);
+  // ...
+  await clock.tickAsync(1100);
+  const result = await promise;
+} finally {
+  clock.restore();
+}

Also applies to: 121-138, 192-205, 248-260, 311-316, 358-363, 408-420, 473-489, 548-563

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 55 - 68,
The test uses sinon.useFakeTimers() to create clock and calls clock.restore()
only on the success path; update the test around the localScan(1) invocation to
ensure clock.restore() always runs by wrapping the await clock.tickAsync(1100) /
await promise sequence in a try/finally (or use an afterEach) so clock.restore()
is called in the finally block; apply the same pattern for other occurrences
that use sinon.useFakeTimers() (references: clock, sinon.useFakeTimers,
clock.tickAsync, clock.restore, promise, localScan) to avoid leaking fake timers
across tests.
server/test/services/tuya/lib/tuya.poll.test.js (1)

368-370: Align unit-persistence test intent with what is actually asserted.

These tests declare setParam stubs but never inject them into the poll context, so the setParam.called === false assertions do not validate persistence behavior.

✅ Tighten test intent (minimal cleanup)
-  it('should convert thermostat temperatures from local dps unit and persist reported unit', async () => {
+  it('should convert thermostat temperatures from local dps unit', async () => {
@@
-    const setParam = sinon.stub().resolves();
@@
-    expect(setParam.called).to.equal(false);
@@
   it('should convert cloud thermostat temperatures from reported unit', async () => {
@@
-    const setParam = sinon.stub().resolves();
@@
-    expect(setParam.called).to.equal(false);

Also applies to: 375-376, 418-419, 430-431, 439-440, 469-470

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.poll.test.js` around lines 368 - 370, The
test stubs a setParam but never supplies it to the polled module, so assertions
like setParam.called === false don't actually validate persistence; update the
test that constructs poll via proxyquire (the test around poll/localPoll stub)
to inject a stubbed setParam into the module's dependencies (the object passed
as the second arg to proxyquire) or alternatively remove/adjust the setParam
assertions to match reality; specifically locate the proxyquire call that
imports '../../../../services/tuya/lib/tuya.poll' and add the setParam stub into
that override (referenced symbols: poll, localPoll, setParam) so the poll
implementation receives the stub and the persistence assertions become
meaningful.
server/services/tuya/lib/mappings/index.js (1)

103-109: Tighten keyword fallback matching to reduce false-positive type detection.

At Line 103, a single required-code hit plus a generic keyword (e.g., thermostat) is enough to classify. That can over-route non-pilot thermostats to pilot-thermostat mappings.

🔧 Suggested heuristic refinement
-  const hasRequiredCode = requiredCodes.size === 0 || Array.from(requiredCodes).some((code) => codes.has(code));
-  if (!hasRequiredCode || !modelName || keywords.length === 0) {
+  const matchedRequiredCodes = Array.from(requiredCodes).filter((code) => codes.has(code)).length;
+  const hasEnoughRequiredCodes =
+    requiredCodes.size === 0 || matchedRequiredCodes >= Math.min(2, requiredCodes.size);
+  if (!hasEnoughRequiredCodes || !modelName || keywords.length === 0) {
     return false;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/index.js` around lines 103 - 109, The
current fallback treats any single required-code hit plus a generic keyword
(e.g., "thermostat") as a match; tighten the heuristic in the block that
computes hasRequiredCode and the subsequent return so that a generic keyword
alone does not trigger mapping: require either (a) at least one non-generic
keyword match or (b) multiple required-code matches before accepting a generic
keyword. Implement this by adding a small set of generic tokens (e.g.,
"thermostat", "light", "switch") and changing the keyword check to prefer
non-generic keyword matches (matching modelName using whole-word,
case-insensitive checks rather than plain includes) and only allow generic-token
matches when requiredCodes has more than one hit; update the logic referencing
hasRequiredCode, requiredCodes, codes, modelName, and keywords accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@front/src/components/boxs/device-in-room/device-features/sensor-value/PilotWireModeDeviceValue.jsx`:
- Around line 5-10: The component PilotWireModeDeviceValue.jsx treats only null
as "no value" and still builds translation keys when deviceFeature.last_value is
undefined; change the conditional logic to treat both null and undefined as "no
value" (e.g., use a nullish check like deviceFeature.last_value == null or
explicit undefined check) so the first branch renders the fallback Text id
"dashboard.boxes.devicesInRoom.noValue" when last_value is nullish, and only
build the translation key in the second branch when last_value is a defined
value.

In `@front/src/routes/integration/all/tuya/commons/deviceHelpers.js`:
- Around line 6-7: normalizeBoolean currently only accepts strict forms (true,
1, '1', 'true', 'TRUE') and misses common truthy variants like 'on' and
untrimmed/case variants; update the normalizeBoolean function to robustly handle
booleans by: treating literal true and numeric 1 as truthy, and for string
inputs first trim() and then compare the lowercased value against 'true', '1',
and 'on' (i.e., value = String(value).trim().toLowerCase(); return value ===
'true' || value === '1' || value === 'on'), ensuring normalizeBoolean handles
null/undefined safely by converting to string only after checking for
boolean/number types.

In `@front/src/routes/integration/all/tuya/commons/githubIssue.js`:
- Around line 181-190: buildSuggestedCloudStatus currently only reads
get(issuePayload, 'cloud.assembled.properties.properties') which loses data when
cloud.assembled.properties is already an array; change the function
(buildSuggestedCloudStatus) to first read const raw = get(issuePayload,
'cloud.assembled.properties') and then set properties = Array.isArray(raw) ? raw
: (raw && raw.properties) || []; keep the existing filtering/mapping logic on
properties and return the same result object so both shapes (array or {
properties: [...] }) are supported.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 514-521: Replace the non-semantic, non-keyboard-accessible <span>
used to toggle the client secret with a <button> element in SetupTab.jsx so
keyboard users can activate it; keep the existing
onClick={this.toggleClientSecret}, ensure the button has type="button" to avoid
form submission, add an appropriate accessible name such as
aria-pressed={state.showClientSecret} and an aria-label that reflects the action
(toggle show/hide client secret), and preserve the icon element with its cx(...)
class logic and visual styling (including the cursor style if needed) so
toggleClientSecret and state.showClientSecret continue to work unchanged.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx`:
- Around line 900-911: The input currently renders the sensitive localKey value
in clear text (see localKey and the input in TuyaDeviceBox.jsx); change the UI
to avoid exposing it by default — render the field masked (use password-style
masking or replace value with bullets) and add an explicit "show/hide" toggle
control that reveals the real localKey only when user intentionally clicks it;
ensure the input's disabled prop is set correctly (boolean) and keep
accessibility (aria-controls/aria-pressed) for the toggle so users can reveal
the secret intentionally.
- Around line 486-491: The popup blocker issue is caused by calling
window.open(issueUrl, '_blank') after awaiting checkGithubIssueExists(), losing
the user gesture; change the flow to always open a blank popup synchronously on
the user gesture (reuse the existing popup variable created before the await)
and then set popup.location = issueUrl for both the "exists" and fallback paths
instead of calling window.open after the await; update the code paths around
checkGithubIssueExists, the popup variable, and any window.open(issueUrl,
'_blank') calls so they navigate the already-opened popup (or remove the
post-await fallback window.open entirely).

In `@server/services/tuya/index.js`:
- Around line 87-97: The quickReconnectTimeouts array is never cleared when max
retries are reached, allowing IDs to accumulate; inside the runAttempt logic
where attempts >= QUICK_RECONNECT_ATTEMPTS and before setting
quickReconnectInProgress = false (and also after a successful reconnect),
iterate quickReconnectTimeouts calling clearTimeout on each id (and unref if
needed), then set quickReconnectTimeouts.length = 0 to reset it; reference the
symbols quickReconnectTimeouts, quickReconnectInProgress, runAttempt,
QUICK_RECONNECT_ATTEMPTS, and QUICK_RECONNECT_DELAY_MS to locate and update the
timeout-cleanup logic.

In `@server/services/tuya/lib/tuya.loadDevices.js`:
- Around line 52-57: The loadDevices pagination can recurse indefinitely when
has_more stays true; modify the method tuya.loadDevices (function loadDevices)
to enforce a hard recursion/page limit (e.g., maxPages or maxDepth) and stop
with a clear error when exceeded: add an optional parameter like maxPages
(default e.g. 100) or currentDepth, increment on each recursive call, check
before recursing and throw an Error when the limit is hit, or refactor the
recursion into an iterative loop that tracks pages and aborts after the same
maxPages threshold; ensure the thrown error message names loadDevices and
includes current pageNo and the configured limit for observability.

In `@server/services/tuya/lib/tuya.localScan.js`:
- Around line 18-19: Update the JSDoc for the localScan function to match the
actual returned shape: document that it returns a Promise resolving to an object
with two properties — "devices" (a map/object of deviceId -> { ip, version,
productKey }) and "portErrors" (an array of port error objects or messages);
change the `@returns` annotation and any examples to reflect Promise<{ devices:
Record<string, { ip: string, version: string, productKey: string }>, portErrors:
any[] }> and mention the keys so callers of localScan know to read
result.devices and result.portErrors.

In
`@server/test/services/tuya/fixtures/devices/smart-socket-basic/local-dps.json`:
- Around line 1-4: Update the DP ID in the smart-socket-basic local DPS fixture
so the power value uses the correct DP key: change the object key "6" to "19" in
local-dps.json (keep the value 2245) so it aligns with
expected-local-mapping.json's mapping for cur_power.

---

Nitpick comments:
In `@server/migrations/20260303080000-rename-tuya-cloud-strategy-param.js`:
- Line 59: The migration's down handler is a no-op so rollbacks won't restore
the previous parameter names; implement a reversible down migration in the same
file (20260303080000-rename-tuya-cloud-strategy-param.js) by replacing the
current empty async down: () => {} with an async function that reverses the
changes made in the up migration (i.e., rename the param keys back to their
original names using the same table/model operations used in up), ensuring the
down uses the same identifiers and SQL/ORM methods so a rollback restores prior
param naming.

In `@server/services/tuya/lib/mappings/index.js`:
- Around line 103-109: The current fallback treats any single required-code hit
plus a generic keyword (e.g., "thermostat") as a match; tighten the heuristic in
the block that computes hasRequiredCode and the subsequent return so that a
generic keyword alone does not trigger mapping: require either (a) at least one
non-generic keyword match or (b) multiple required-code matches before accepting
a generic keyword. Implement this by adding a small set of generic tokens (e.g.,
"thermostat", "light", "switch") and changing the keyword check to prefer
non-generic keyword matches (matching modelName using whole-word,
case-insensitive checks rather than plain includes) and only allow generic-token
matches when requiredCodes has more than one hit; update the logic referencing
hasRequiredCode, requiredCodes, codes, modelName, and keywords accordingly.

In `@server/services/tuya/lib/mappings/local/smart-meter.js`:
- Around line 23-37: The dps map contains several misspellings (e.g.,
energy_forword_a, energy_reserse_b) and you already have a codeAliases object —
populate codeAliases to map the canonical/expected keys to the existing
misspelled keys so behavior stays the same across firmware variants; e.g., map
energy_forward_a -> energy_forword_a, energy_forward_b -> energy_forword_b,
energy_reverse_a -> energy_reverse_a (if needed) and energy_reverse_b ->
energy_reserse_b, and also include total variants like forward_energy_total ->
forward_energy_total and reverse_energy_total -> reverse_energy_total as aliases
where spellings differ; update the codeAliases object in this file so lookups
can resolve either spelling to the existing dps entries.

In `@server/services/tuya/lib/tuya.connect.js`:
- Around line 68-77: The second conditional redundantly checks `response &&`
after the prior `if (!response)` already throws; remove the `response &&` guard
and simplify the condition to `if (response.success === false)` (keeping the
existing error construction that uses `response.msg || response.message ||
'TUYA_APP_ACCOUNT_UID_INVALID'` and `response.code ||
'TUYA_APP_ACCOUNT_UID_INVALID'`) so only a falsy `response.success` triggers the
error throw while preserving existing behavior.

In `@server/services/tuya/lib/tuya.getStatus.js`:
- Around line 11-15: The four independent variable reads calling
this.gladys.variable.getValue with GLADYS_VARIABLES.ENDPOINT,
GLADYS_VARIABLES.ACCESS_KEY, GLADYS_VARIABLES.SECRET_KEY, and
GLADYS_VARIABLES.APP_ACCOUNT_UID (and MANUAL_DISCONNECT) should be parallelized
with Promise.all to reduce latency: invoke Promise.all on the array of getValue
promises (each passing this.serviceId) and destructure the results into
endpoint, accessKey, secretKey, appAccountId, manualDisconnect so behavior
remains identical but the awaits run concurrently.

In `@server/services/tuya/lib/tuya.loadDeviceDetails.js`:
- Around line 12-14: The code in loadDeviceDetails performs a redundant null
check when assigning listDeviceEntry using tuyaDevice ? { ...tuyaDevice } : null
even though tuyaDevice is already destructured earlier (const { id: deviceId } =
tuyaDevice), so remove the ternary and simply assign listDeviceEntry = {
...tuyaDevice } (or remove the assignment entirely if unused) to avoid
dead/contradictory null-checks; update any downstream uses accordingly.

In `@server/services/tuya/lib/tuya.setValue.js`:
- Around line 23-46: Both getTemperatureUnitFromProperties and
getFeatureWithFallbackScale are duplicated; extract them into a shared helper
module and import it from both runtime modules. Create a new module (e.g.,
tuyaHelpers) that exports getTemperatureUnitFromProperties and
getFeatureWithFallbackScale (and any dependent helpers like
normalizeTemperatureUnit, getDeviceType, getFeatureMapping if not already
shared), update tuya.setValue.js to import those functions and remove the local
copies, and update tuya.poll.js to import the same helpers so both files use the
centralized implementations.

In `@server/services/tuya/lib/utils/tuya.report.js`:
- Around line 30-68: The createBaseReport function repeats verbose null-checks
like currentReport.cloud && currentReport.cloud.assembled; refactor it to use
optional chaining and nullish coalescing (e.g.,
currentReport?.cloud?.assembled?.specifications ?? null) to simplify each field
assignment while preserving the same null fallback behavior; update references
inside createBaseReport (currentReport, cloud.assembled, cloud.raw, local.scan)
and keep REPORT_SCHEMA_VERSION unchanged.

In `@server/test/services/tuya/lib/controllers/tuya.controller.test.js`:
- Around line 88-105: The test currently seeds tuyaManager.discoveredDevices
with only an IP_ADDRESS so the assertion
expect(localKeyParam).to.equal(undefined) is vacuously true; update the fixture
to include a LOCAL_KEY param (e.g., add { name: 'LOCAL_KEY', value: 'someKey' })
in the discoveredDevices entry before invoking controller['post
/api/v1/service/tuya/local-poll'].controller(req, res), then keep the assertion
that LOCAL_KEY is removed (localKeyParam is undefined) after the controller runs
to ensure removal logic is actually tested.

In `@server/test/services/tuya/lib/device/tuya.convertDevice.test.js`:
- Around line 63-64: Replace the hardcoded cloud-strategy string literals used
in assertions (e.g.,
expect(params[DEVICE_PARAM_NAME.CLOUD_STRATEGY]).to.equal('legacy') and the
similar 'shadow' assertion) with the exported strategy constants (e.g.,
CLOUD_STRATEGY.LEGACY and CLOUD_STRATEGY.SHADOW); import CLOUD_STRATEGY at the
top of the test alongside DEVICE_PARAM_NAME and update the
expect(...).to.equal(...) calls to reference these constants so the tests use
canonical symbols instead of brittle strings.

In `@server/test/services/tuya/lib/tuya.getStatus.test.js`:
- Around line 17-25: Tests currently reuse a single TuyaHandler instance
(tuyaHandler) across cases which can cause state bleed; update the setup to
instantiate a fresh TuyaHandler inside beforeEach (and remove any shared
top-level instantiation) so each test gets a new instance, referencing the
TuyaHandler constructor used in the file and ensuring teardown/reset is still
run in afterEach (keep sinon.reset() there).

In `@server/test/services/tuya/lib/tuya.init.test.js`:
- Around line 136-154: The test currently stubs tuyaHandler.connect and
tuyaHandler.getConfiguration but restores them only at the end of the happy
path, which can leak into other tests if an assertion fails; update the test for
the case in the tuya.init.test.js to ensure connect and getConfiguration stubs
are restored in a finally block (or use a sinon sandbox/afterEach cleanup)
around the await tuyaHandler.init() call so that tuyaHandler.connect and
tuyaHandler.getConfiguration are always restored even on failure.

In `@server/test/services/tuya/lib/tuya.localScan.test.js`:
- Around line 55-68: The test uses sinon.useFakeTimers() to create clock and
calls clock.restore() only on the success path; update the test around the
localScan(1) invocation to ensure clock.restore() always runs by wrapping the
await clock.tickAsync(1100) / await promise sequence in a try/finally (or use an
afterEach) so clock.restore() is called in the finally block; apply the same
pattern for other occurrences that use sinon.useFakeTimers() (references: clock,
sinon.useFakeTimers, clock.tickAsync, clock.restore, promise, localScan) to
avoid leaking fake timers across tests.

In `@server/test/services/tuya/lib/tuya.poll.test.js`:
- Around line 368-370: The test stubs a setParam but never supplies it to the
polled module, so assertions like setParam.called === false don't actually
validate persistence; update the test that constructs poll via proxyquire (the
test around poll/localPoll stub) to inject a stubbed setParam into the module's
dependencies (the object passed as the second arg to proxyquire) or
alternatively remove/adjust the setParam assertions to match reality;
specifically locate the proxyquire call that imports
'../../../../services/tuya/lib/tuya.poll' and add the setParam stub into that
override (referenced symbols: poll, localPoll, setParam) so the poll
implementation receives the stub and the persistence assertions become
meaningful.

In `@server/test/services/tuya/lib/tuya.setValue.fixtures.test.js`:
- Around line 46-48: The test should guard for a missing feature before calling
setValue: after loading the device with fixtureCase.load and locating
currentFeature via currentDevice.features.find using featureExternalId, add an
assertion or explicit guard that currentFeature is defined (e.g.,
assert.ok/currentFeature or throw with a clear message) so the test fails fast
and explains that featureExternalId was not found rather than letting setValue
fail later with a confusing error.

In `@server/test/services/tuya/lib/tuya.setValue.test.js`:
- Around line 31-38: The test's default stub for tuyaHandler.connector.request
is returning a paginated read-style payload (result.list/has_more) which is
irrelevant for setValue/write-path tests; update the stub used in this suite so
its default resolved value matches a write-response shape used by setValue
(e.g., result indicating success/status rather than list/has_more). Locate the
stub on tuyaHandler.connector.request in the test and change the default
.resolves(...) to a write-style response (and keep any specialized
onFirstCall/onSecondCall variants as needed) so the suite exercises setValue
response handling correctly.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8db6b0d and 2b359f1.

⛔ Files ignored due to path filters (1)
  • server/services/tuya/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (113)
  • front/src/components/boxs/device-in-room/device-features/PilotWireModeDeviceFeature.jsx
  • front/src/components/boxs/device-in-room/device-features/sensor-value/PilotWireModeDeviceValue.jsx
  • front/src/components/boxs/device-in-room/device-features/sensor-value/SensorDeviceFeature.jsx
  • front/src/config/i18n/de.json
  • front/src/config/i18n/en.json
  • front/src/config/i18n/fr.json
  • front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx
  • front/src/routes/integration/all/tuya/commons/deviceHelpers.js
  • front/src/routes/integration/all/tuya/commons/githubIssue.js
  • front/src/routes/integration/all/tuya/discover-page/DiscoverTab.jsx
  • front/src/routes/integration/all/tuya/discover-page/style.css
  • front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx
  • server/migrations/20260303080000-rename-tuya-cloud-strategy-param.js
  • server/services/tuya/api/tuya.controller.js
  • server/services/tuya/index.js
  • server/services/tuya/lib/device/tuya.convertDevice.js
  • server/services/tuya/lib/device/tuya.convertFeature.js
  • server/services/tuya/lib/device/tuya.deviceMapping.js
  • server/services/tuya/lib/device/tuya.localMapping.js
  • server/services/tuya/lib/index.js
  • server/services/tuya/lib/mappings/cloud/global.js
  • server/services/tuya/lib/mappings/cloud/pilot-thermostat.js
  • server/services/tuya/lib/mappings/cloud/smart-meter.js
  • server/services/tuya/lib/mappings/cloud/smart-socket.js
  • server/services/tuya/lib/mappings/index.js
  • server/services/tuya/lib/mappings/local/global.js
  • server/services/tuya/lib/mappings/local/pilot-thermostat.js
  • server/services/tuya/lib/mappings/local/smart-meter.js
  • server/services/tuya/lib/mappings/local/smart-socket.js
  • server/services/tuya/lib/tuya.connect.js
  • server/services/tuya/lib/tuya.disconnect.js
  • server/services/tuya/lib/tuya.discoverDevices.js
  • server/services/tuya/lib/tuya.getConfiguration.js
  • server/services/tuya/lib/tuya.getStatus.js
  • server/services/tuya/lib/tuya.init.js
  • server/services/tuya/lib/tuya.loadDeviceDetails.js
  • server/services/tuya/lib/tuya.loadDevices.js
  • server/services/tuya/lib/tuya.localPoll.js
  • server/services/tuya/lib/tuya.localScan.js
  • server/services/tuya/lib/tuya.manualDisconnect.js
  • server/services/tuya/lib/tuya.poll.js
  • server/services/tuya/lib/tuya.saveConfiguration.js
  • server/services/tuya/lib/tuya.setValue.js
  • server/services/tuya/lib/utils/tuya.cloudStrategy.js
  • server/services/tuya/lib/utils/tuya.config.js
  • server/services/tuya/lib/utils/tuya.constants.js
  • server/services/tuya/lib/utils/tuya.deviceParams.js
  • server/services/tuya/lib/utils/tuya.normalize.js
  • server/services/tuya/lib/utils/tuya.report.js
  • server/services/tuya/package.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/cloud-status.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/expected-device.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/expected-events.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/expected-local-mapping.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/input-device.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/local-dps.json
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/manifest.js
  • server/test/services/tuya/fixtures/devices/pilote-thermostat-c03zek9b5daz7omr/poll-device.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/cloud-status.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-device.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-events.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-local-mapping.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/input-device.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/local-dps.json
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/manifest.js
  • server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/poll-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/cloud-status.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-cloud-events.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-local-events.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-local-mapping.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/input-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/local-dps.json
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/manifest.js
  • server/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/poll-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/cloud-status.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/expected-cloud-events.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/expected-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/expected-local-events.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/expected-local-mapping.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/input-device.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/local-dps.json
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/manifest.js
  • server/test/services/tuya/fixtures/devices/smart-socket-basic/poll-device.json
  • server/test/services/tuya/fixtures/fixtureHelper.js
  • server/test/services/tuya/index.test.js
  • server/test/services/tuya/lib/controllers/tuya.controller.test.js
  • server/test/services/tuya/lib/device/feature/tuya.convertFeature.test.js
  • server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js
  • server/test/services/tuya/lib/device/tuya.convertDevice.fixtures.test.js
  • server/test/services/tuya/lib/device/tuya.convertDevice.test.js
  • server/test/services/tuya/lib/device/tuya.localMapping.fixtures.test.js
  • server/test/services/tuya/lib/device/tuya.localMapping.test.js
  • server/test/services/tuya/lib/mappings/index.test.js
  • server/test/services/tuya/lib/tuya.connect.test.js
  • server/test/services/tuya/lib/tuya.disconnect.test.js
  • server/test/services/tuya/lib/tuya.discoverDevices.test.js
  • server/test/services/tuya/lib/tuya.getConfiguration.test.js
  • server/test/services/tuya/lib/tuya.getStatus.test.js
  • server/test/services/tuya/lib/tuya.init.test.js
  • server/test/services/tuya/lib/tuya.loadDeviceDetails.test.js
  • server/test/services/tuya/lib/tuya.loadDevices.test.js
  • server/test/services/tuya/lib/tuya.localPoll.test.js
  • server/test/services/tuya/lib/tuya.localScan.test.js
  • server/test/services/tuya/lib/tuya.manualDisconnect.test.js
  • server/test/services/tuya/lib/tuya.poll.fixtures.test.js
  • server/test/services/tuya/lib/tuya.poll.test.js
  • server/test/services/tuya/lib/tuya.saveConfiguration.test.js
  • server/test/services/tuya/lib/tuya.setValue.fixtures.test.js
  • server/test/services/tuya/lib/tuya.setValue.test.js
  • server/test/services/tuya/lib/utils/tuya.deviceParams.test.js
  • server/test/services/tuya/tuya.mock.test.js
  • server/utils/constants.js

Comment on lines +5 to +10
{deviceFeature.last_value === null && <Text id="dashboard.boxes.devicesInRoom.noValue" />}
{deviceFeature.last_value !== null && (
<Text
id={`deviceFeatureValue.category.${deviceFeature.category}.${deviceFeature.type}.${deviceFeature.last_value}`}
/>
)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle undefined last_value to avoid invalid translation keys.

The component only treats null as “no value”. undefined currently builds a key with .undefined instead of showing fallback text.

Proposed fix
-    {deviceFeature.last_value === null && <Text id="dashboard.boxes.devicesInRoom.noValue" />}
-    {deviceFeature.last_value !== null && (
+    {(deviceFeature.last_value === null || deviceFeature.last_value === undefined) && (
+      <Text id="dashboard.boxes.devicesInRoom.noValue" />
+    )}
+    {deviceFeature.last_value !== null && deviceFeature.last_value !== undefined && (
       <Text
         id={`deviceFeatureValue.category.${deviceFeature.category}.${deviceFeature.type}.${deviceFeature.last_value}`}
       />
     )}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{deviceFeature.last_value === null && <Text id="dashboard.boxes.devicesInRoom.noValue" />}
{deviceFeature.last_value !== null && (
<Text
id={`deviceFeatureValue.category.${deviceFeature.category}.${deviceFeature.type}.${deviceFeature.last_value}`}
/>
)}
{(deviceFeature.last_value === null || deviceFeature.last_value === undefined) && (
<Text id="dashboard.boxes.devicesInRoom.noValue" />
)}
{deviceFeature.last_value !== null && deviceFeature.last_value !== undefined && (
<Text
id={`deviceFeatureValue.category.${deviceFeature.category}.${deviceFeature.type}.${deviceFeature.last_value}`}
/>
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@front/src/components/boxs/device-in-room/device-features/sensor-value/PilotWireModeDeviceValue.jsx`
around lines 5 - 10, The component PilotWireModeDeviceValue.jsx treats only null
as "no value" and still builds translation keys when deviceFeature.last_value is
undefined; change the conditional logic to treat both null and undefined as "no
value" (e.g., use a nullish check like deviceFeature.last_value == null or
explicit undefined check) so the first branch renders the fallback Text id
"dashboard.boxes.devicesInRoom.noValue" when last_value is nullish, and only
build the translation key in the second branch when last_value is a defined
value.

Comment on lines +6 to +7
export const normalizeBoolean = value =>
value === true || value === 1 || value === '1' || value === 'true' || value === 'TRUE';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align frontend boolean normalization with backend Tuya semantics.

Line [6] currently misses accepted truthy forms like 'on' and trimmed string variants, which can drift local/cloud mode interpretation between frontend and server.

🔧 Proposed fix
-export const normalizeBoolean = value =>
-  value === true || value === 1 || value === '1' || value === 'true' || value === 'TRUE';
+export const normalizeBoolean = value => {
+  if (value === true || value === 1 || value === '1') {
+    return true;
+  }
+  return typeof value === 'string' && ['true', 'on'].includes(value.trim().toLowerCase());
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/commons/deviceHelpers.js` around lines
6 - 7, normalizeBoolean currently only accepts strict forms (true, 1, '1',
'true', 'TRUE') and misses common truthy variants like 'on' and untrimmed/case
variants; update the normalizeBoolean function to robustly handle booleans by:
treating literal true and numeric 1 as truthy, and for string inputs first
trim() and then compare the lowercased value against 'true', '1', and 'on'
(i.e., value = String(value).trim().toLowerCase(); return value === 'true' ||
value === '1' || value === 'on'), ensuring normalizeBoolean handles
null/undefined safely by converting to string only after checking for
boolean/number types.

Comment on lines +181 to +190
const buildSuggestedCloudStatus = issuePayload => {
const properties = get(issuePayload, 'cloud.assembled.properties.properties') || [];
return {
result: properties
.filter(item => item && item.code !== undefined && item.code !== null && item.value !== undefined)
.map(item => ({
code: item.code,
value: item.value
}))
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Support both properties shapes when generating suggested cloud-status.json.

Line [182] currently only reads cloud.assembled.properties.properties. If cloud.assembled.properties is already an array, the generated status fixture becomes empty.

🔧 Proposed fix
 const buildSuggestedCloudStatus = issuePayload => {
-  const properties = get(issuePayload, 'cloud.assembled.properties.properties') || [];
+  const assembledProperties = get(issuePayload, 'cloud.assembled.properties');
+  const properties = Array.isArray(assembledProperties)
+    ? assembledProperties
+    : Array.isArray(assembledProperties && assembledProperties.properties)
+    ? assembledProperties.properties
+    : [];
   return {
     result: properties
       .filter(item => item && item.code !== undefined && item.code !== null && item.value !== undefined)
       .map(item => ({
         code: item.code,
         value: item.value
       }))
   };
 };

Based on learnings, Tuya fixtures intentionally preserve real-world payload inconsistencies (including varying payload structures across sections), so this conversion path should support both forms.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/commons/githubIssue.js` around lines
181 - 190, buildSuggestedCloudStatus currently only reads get(issuePayload,
'cloud.assembled.properties.properties') which loses data when
cloud.assembled.properties is already an array; change the function
(buildSuggestedCloudStatus) to first read const raw = get(issuePayload,
'cloud.assembled.properties') and then set properties = Array.isArray(raw) ? raw
: (raw && raw.properties) || []; keep the existing filtering/mapping logic on
properties and return the same result object so both shapes (array or {
properties: [...] }) are supported.

Comment on lines +514 to +521
<span class="input-icon-addon cursor-pointer" onClick={this.toggleClientSecret}>
<i
class={cx('fe', {
'fe-eye': !state.showClientSecret,
'fe-eye-off': state.showClientSecret
})}
/>
</span>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check if the file exists and examine the lines around 514-521
if [ -f "front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx" ]; then
  echo "=== Lines 510-530 of SetupTab.jsx ==="
  sed -n '510,530p' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx | cat -n
  
  echo -e "\n=== Full code block for the toggle (lines 514-521) ==="
  sed -n '514,521p' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx
else
  echo "File not found"
fi

Repository: GladysAssistant/Gladys

Length of output: 1534


🏁 Script executed:

#!/bin/bash
# Search for other clickable span elements in the same file
echo "=== Searching for clickable spans in SetupTab.jsx ==="
rg -n '<span[^>]*onClick=' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx -C3 || echo "No matches found or file not accessible"

# Also search for keyboard handlers on the toggleClientSecret function
echo -e "\n=== Searching for toggleClientSecret method ==="
rg -n 'toggleClientSecret' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx -C5 || echo "Not found"

Repository: GladysAssistant/Gladys

Length of output: 1360


🏁 Script executed:

#!/bin/bash
# Check if there are any onKeyDown or onKeyUp handlers in the same area
echo "=== Searching for keyboard handlers in SetupTab.jsx ==="
rg -n 'onKey' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx | head -20 || echo "No keyboard handlers found"

# Count total occurrences of clickable spans
echo -e "\n=== Count of clickable non-button elements in file ==="
rg '<span[^>]*onClick=' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx | wc -l || echo "Search failed"

Repository: GladysAssistant/Gladys

Length of output: 179


Replace the clickable <span> with a semantic button for the secret toggle.

This control is mouse-only today; keyboard users cannot reliably activate it. The span element lacks keyboard event handlers, and the toggleClientSecret method has no keyboard-specific logic. Converting to a <button> element makes this control keyboard accessible and semantically correct.

♿ Suggested fix
-                    <span class="input-icon-addon cursor-pointer" onClick={this.toggleClientSecret}>
+                    <button
+                      type="button"
+                      class="input-icon-addon cursor-pointer btn btn-link p-0"
+                      onClick={this.toggleClientSecret}
+                      aria-label={state.showClientSecret ? 'Hide client secret' : 'Show client secret'}
+                    >
                       <i
                         class={cx('fe', {
                           'fe-eye': !state.showClientSecret,
                           'fe-eye-off': state.showClientSecret
                         })}
                       />
-                    </span>
+                    </button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
514 - 521, Replace the non-semantic, non-keyboard-accessible <span> used to
toggle the client secret with a <button> element in SetupTab.jsx so keyboard
users can activate it; keep the existing onClick={this.toggleClientSecret},
ensure the button has type="button" to avoid form submission, add an appropriate
accessible name such as aria-pressed={state.showClientSecret} and an aria-label
that reflects the action (toggle show/hide client secret), and preserve the icon
element with its cx(...) class logic and visual styling (including the cursor
style if needed) so toggleClientSecret and state.showClientSecret continue to
work unchanged.

Comment on lines +486 to +491
if (popup) {
popup.location = issueUrl;
return;
}
window.open(issueUrl, '_blank');
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the file and check size
wc -l front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx

Repository: GladysAssistant/Gladys

Length of output: 126


🏁 Script executed:

#!/bin/bash
# Read context around lines 486-491
sed -n '480,495p' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx
echo "---"
# Read context around lines 557-561
sed -n '550,565p' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx

Repository: GladysAssistant/Gladys

Length of output: 850


🏁 Script executed:

#!/bin/bash
# Read full context of the async function containing the second window.open call
sed -n '530,565p' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx

Repository: GladysAssistant/Gladys

Length of output: 1136


🏁 Script executed:

#!/bin/bash
# Find all window.open calls in the file
rg -n 'window\.open\(' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx -A1 -B1

Repository: GladysAssistant/Gladys

Length of output: 499


🏁 Script executed:

#!/bin/bash
# Check context of lines 486-491 - what function is it in?
sed -n '470,495p' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx

Repository: GladysAssistant/Gladys

Length of output: 659


🏁 Script executed:

#!/bin/bash
# Find the function containing lines 486-491
sed -n '420,495p' front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx | head -70

Repository: GladysAssistant/Gladys

Length of output: 1787


Fix critical popup blocker violation: window.open() calls after await cannot be blocked-bypassed.

Lines 490 and 560 call window.open() after the await checkGithubIssueExists() at line 441, which loses the user gesture context required by popup blockers. These fallback window.open() calls will be blocked by browsers and fail silently. The synchronous window.open('about:blank', '_blank') at lines 429 and 534 work correctly because they're called within the initial user gesture, but the subsequent fallback paths at lines 490 and 560 do not maintain that gesture after the await.

Refactor to navigate the already-opened popup (similar to the pattern used for the primary flow at line 489 and 558-559) or remove the fallback entirely if popups are not opened.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 486 -
491, The popup blocker issue is caused by calling window.open(issueUrl,
'_blank') after awaiting checkGithubIssueExists(), losing the user gesture;
change the flow to always open a blank popup synchronously on the user gesture
(reuse the existing popup variable created before the await) and then set
popup.location = issueUrl for both the "exists" and fallback paths instead of
calling window.open after the await; update the code paths around
checkGithubIssueExists, the popup variable, and any window.open(issueUrl,
'_blank') calls so they navigate the already-opened popup (or remove the
post-await fallback window.open entirely).

Comment on lines +900 to +911
<div class="form-group">
<label class="form-label" for={`local_key_${deviceIndex}`}>
<Text id="integration.tuya.device.localKeyLabel" />
</label>
<input
id={`local_key_${deviceIndex}`}
type="text"
value={localKey}
class="form-control"
disabled="true"
/>
</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not expose localKey in clear text by default.

localKey is a device credential; rendering it plainly increases accidental leakage risk (screenshare/screenshot/shoulder-surfing).

🔐 Suggested hardening
                 <div class="form-group">
                   <label class="form-label" for={`local_key_${deviceIndex}`}>
                     <Text id="integration.tuya.device.localKeyLabel" />
                   </label>
                   <input
                     id={`local_key_${deviceIndex}`}
-                    type="text"
+                    type="password"
                     value={localKey}
                     class="form-control"
                     disabled="true"
                   />
                 </div>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div class="form-group">
<label class="form-label" for={`local_key_${deviceIndex}`}>
<Text id="integration.tuya.device.localKeyLabel" />
</label>
<input
id={`local_key_${deviceIndex}`}
type="text"
value={localKey}
class="form-control"
disabled="true"
/>
</div>
<div class="form-group">
<label class="form-label" for={`local_key_${deviceIndex}`}>
<Text id="integration.tuya.device.localKeyLabel" />
</label>
<input
id={`local_key_${deviceIndex}`}
type="password"
value={localKey}
class="form-control"
disabled="true"
/>
</div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx` around lines 900 -
911, The input currently renders the sensitive localKey value in clear text (see
localKey and the input in TuyaDeviceBox.jsx); change the UI to avoid exposing it
by default — render the field masked (use password-style masking or replace
value with bullets) and add an explicit "show/hide" toggle control that reveals
the real localKey only when user intentionally clicks it; ensure the input's
disabled prop is set correctly (boolean) and keep accessibility
(aria-controls/aria-pressed) for the toggle so users can reveal the secret
intentionally.

Comment on lines +87 to +97
if (attempts < QUICK_RECONNECT_ATTEMPTS) {
const timeoutId = setTimeout(runAttempt, QUICK_RECONNECT_DELAY_MS);
if (timeoutId && typeof timeoutId.unref === 'function') {
timeoutId.unref();
}
quickReconnectTimeouts.push(timeoutId);
return;
}

quickReconnectInProgress = false;
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Quick-reconnect timeout IDs can accumulate across failed cycles.

When max retries are reached, Line 96 only flips the in-progress flag. The stored timeout IDs are not cleared, so repeated disconnect cycles can grow quickReconnectTimeouts indefinitely.

🛠️ Suggested fix
-      quickReconnectInProgress = false;
+      clearQuickReconnects();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/index.js` around lines 87 - 97, The
quickReconnectTimeouts array is never cleared when max retries are reached,
allowing IDs to accumulate; inside the runAttempt logic where attempts >=
QUICK_RECONNECT_ATTEMPTS and before setting quickReconnectInProgress = false
(and also after a successful reconnect), iterate quickReconnectTimeouts calling
clearTimeout on each id (and unref if needed), then set
quickReconnectTimeouts.length = 0 to reset it; reference the symbols
quickReconnectTimeouts, quickReconnectInProgress, runAttempt,
QUICK_RECONNECT_ATTEMPTS, and QUICK_RECONNECT_DELAY_MS to locate and update the
timeout-cleanup logic.

Comment on lines 52 to +57
if (hasMore) {
const nextResult = await this.loadDevices(nextLastRowKey);
nextResult.forEach((device) => list.push(device));
if (list.length === 0) {
throw new Error('Tuya API pagination did not advance (has_more=true with empty page)');
}
const nextResult = await this.loadDevices(pageNo + 1, pageSize);
list.push(...nextResult);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add a hard stop for pagination recursion.

If the upstream keeps returning has_more=true with non-empty pages, this can recurse indefinitely and eventually exhaust request budget/stack.

Suggested safeguard
+const MAX_PAGE_NO = 1000;
+
 async function loadDevices(pageNo = 1, pageSize = 100) {
@@
   if (hasMore) {
     if (list.length === 0) {
       throw new Error('Tuya API pagination did not advance (has_more=true with empty page)');
     }
+    if (pageNo >= MAX_PAGE_NO) {
+      throw new Error(`Tuya API pagination exceeded safe limit at page ${pageNo}`);
+    }
     const nextResult = await this.loadDevices(pageNo + 1, pageSize);
     list.push(...nextResult);
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.loadDevices.js` around lines 52 - 57, The
loadDevices pagination can recurse indefinitely when has_more stays true; modify
the method tuya.loadDevices (function loadDevices) to enforce a hard
recursion/page limit (e.g., maxPages or maxDepth) and stop with a clear error
when exceeded: add an optional parameter like maxPages (default e.g. 100) or
currentDepth, increment on each recursive call, check before recursing and throw
an Error when the limit is hit, or refactor the recursion into an iterative loop
that tracks pages and aborts after the same maxPages threshold; ensure the
thrown error message names loadDevices and includes current pageNo and the
configured limit for observability.

Comment on lines +18 to +19
* @returns {Promise<object>} Map of deviceId -> { ip, version, productKey }.
* @example
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix localScan JSDoc return contract.

Line 18 says the function returns a device map, but implementation returns an object containing both devices and portErrors. This can mislead callers.

📝 Suggested doc fix
- * `@returns` {Promise<object>} Map of deviceId -> { ip, version, productKey }.
+ * `@returns` {Promise<object>} Object with:
+ *   - devices: Map of deviceId -> { ip, version, productKey }
+ *   - portErrors: Map of port -> error message
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* @returns {Promise<object>} Map of deviceId -> { ip, version, productKey }.
* @example
* `@returns` {Promise<object>} Object with:
* - devices: Map of deviceId -> { ip, version, productKey }
* - portErrors: Map of port -> error message
* `@example`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.localScan.js` around lines 18 - 19, Update the
JSDoc for the localScan function to match the actual returned shape: document
that it returns a Promise resolving to an object with two properties — "devices"
(a map/object of deviceId -> { ip, version, productKey }) and "portErrors" (an
array of port error objects or messages); change the `@returns` annotation and any
examples to reflect Promise<{ devices: Record<string, { ip: string, version:
string, productKey: string }>, portErrors: any[] }> and mention the keys so
callers of localScan know to read result.devices and result.portErrors.

Comment on lines +1 to +4
{
"1": true,
"6": 2245
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for consistency between local-dps and expected-local-mapping in smart-socket-basic fixtures
echo "=== local-dps.json ==="
cat server/test/services/tuya/fixtures/devices/smart-socket-basic/local-dps.json

echo -e "\n=== expected-local-mapping.json ==="
cat server/test/services/tuya/fixtures/devices/smart-socket-basic/expected-local-mapping.json

echo -e "\n=== poll-device.json (if exists, check DP mappings) ==="
cat server/test/services/tuya/fixtures/devices/smart-socket-basic/poll-device.json 2>/dev/null || echo "File not found"

Repository: GladysAssistant/Gladys

Length of output: 977


🏁 Script executed:

#!/bin/bash
# Look for other fixture files in smart-socket-basic that might clarify the DP mapping
echo "=== Files in smart-socket-basic directory ==="
ls -la server/test/services/tuya/fixtures/devices/smart-socket-basic/

echo -e "\n=== cloud-status.json (if exists) ==="
cat server/test/services/tuya/fixtures/devices/smart-socket-basic/cloud-status.json 2>/dev/null || echo "File not found"

echo -e "\n=== Search for test files that use these fixtures ==="
find server/test/services/tuya -name "*.test.js" -o -name "*.spec.js" | head -5

echo -e "\n=== Check if there are other device fixtures with similar structure ==="
ls -d server/test/services/tuya/fixtures/devices/*/  2>/dev/null | head -10

Repository: GladysAssistant/Gladys

Length of output: 1732


🏁 Script executed:

#!/bin/bash
# Check another device fixture to see if DP ID mismatches are intentional
echo "=== smart-meter-bbcg1hrkrj5rifsd files ==="
ls -la server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/

echo -e "\n=== smart-meter local-dps.json ==="
cat server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/local-dps.json 2>/dev/null || echo "File not found"

echo -e "\n=== smart-meter expected-local-mapping.json ==="
cat server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-local-mapping.json 2>/dev/null || echo "File not found"

echo -e "\n=== smart-meter cloud-status.json ==="
cat server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/cloud-status.json 2>/dev/null || echo "File not found"

Repository: GladysAssistant/Gladys

Length of output: 3305


🏁 Script executed:

#!/bin/bash
# Find test files that use the smart-socket-basic fixture
echo "=== Search for test files referencing local-dps or expected-local-mapping ==="
grep -r "local-dps\|expected-local-mapping" server/test/services/tuya --include="*.js" -l

echo -e "\n=== Check the main tuya test file ==="
head -100 server/test/services/tuya/index.test.js | grep -A 10 -B 10 "local-dps\|expected-local-mapping" || echo "Checking further..."

echo -e "\n=== Look for fixture loading pattern ==="
grep -r "smart-socket-basic\|fixtures/devices" server/test/services/tuya --include="*.js" -A 2 -B 2 | head -50

Repository: GladysAssistant/Gladys

Length of output: 577


🏁 Script executed:

#!/bin/bash
# Check the manifest files to understand fixture structure
echo "=== smart-socket-basic manifest.js ==="
cat server/test/services/tuya/fixtures/devices/smart-socket-basic/manifest.js

echo -e "\n=== smart-meter manifest.js ==="
cat server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/manifest.js

echo -e "\n=== Check test file that loads fixtures ==="
grep -r "manifest\|fixture" server/test/services/tuya/index.test.js -A 3 -B 3 | head -80

Repository: GladysAssistant/Gladys

Length of output: 1520


Fix DP ID mismatch in local-dps.json fixture.

The local-dps.json uses DP ID "6" for the power value (2245), but expected-local-mapping.json maps cur_power to DP ID 19. Comparing with the smart-meter fixture (which correctly aligns DP IDs between local-dps.json and expected-local-mapping.json), the smart-socket-basic fixture has a misalignment. Update local-dps.json to use DP "19" instead of "6" for the cur_power value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/fixtures/devices/smart-socket-basic/local-dps.json`
around lines 1 - 4, Update the DP ID in the smart-socket-basic local DPS fixture
so the power value uses the correct DP key: change the object key "6" to "19" in
local-dps.json (keep the value 2245) so it aligns with
expected-local-mapping.json's mapping for cur_power.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (10)
server/test/services/tuya/lib/utils/tuya.report.test.js (1)

46-48: Consider asserting all null raw fields in this degraded-mode case.

You already assert three fields; adding the remaining raw fields would make this test guard the full null-input contract more tightly.

Suggested test hardening
     expect(report.cloud.raw.device_list_entry).to.equal(null);
     expect(report.cloud.raw.device_specification).to.equal(null);
+    expect(report.cloud.raw.device_details).to.equal(null);
+    expect(report.cloud.raw.thing_shadow_properties).to.equal(null);
     expect(report.cloud.raw.thing_model).to.equal(null);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/utils/tuya.report.test.js` around lines 46 -
48, The test currently asserts three specific null fields on report.cloud.raw
(device_list_entry, device_specification, thing_model) but should assert the
full degraded-mode contract; update the test to iterate over all properties of
report.cloud.raw (e.g., Object.keys(report.cloud.raw)) and assert each value is
null (or add explicit expect statements for any remaining known raw fields) so
every raw field on the report object is verified as null in this degraded case.
server/test/services/tuya/lib/utils/tuya.cloudStrategy.test.js (1)

10-40: Add a couple of edge-case assertions to harden strategy resolution tests.

Current cases are valid, but they don’t guard against regressions for (1) unsupported non-null codes and (2) default strategy when device is missing. Adding these keeps the suite aligned with your new unified strategy behavior.

✅ Suggested test additions
 describe('Tuya cloud strategy utils', () => {
   it('should return null when cloud codes are invalid', () => {
@@
     expect(strategy).to.equal(null);
   });

+  it('should return null when cloud codes are unsupported strings', () => {
+    const strategy = resolveCloudStrategy(
+      {
+        specifications: {
+          functions: [{ code: 'unsupported_code' }],
+          status: [],
+        },
+      },
+      'smart-socket',
+    );
+
+    expect(strategy).to.equal(null);
+  });
+
   it('should resolve legacy strategy from specifications status', () => {
@@
     expect(strategy).to.equal(CLOUD_STRATEGY.LEGACY);
   });

   it('should default configured strategy to legacy', () => {
     expect(getConfiguredCloudStrategy({ params: [] })).to.equal(CLOUD_STRATEGY.LEGACY);
+    expect(getConfiguredCloudStrategy()).to.equal(CLOUD_STRATEGY.LEGACY);
   });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/utils/tuya.cloudStrategy.test.js` around lines
10 - 40, Add two edge-case assertions: (1) call resolveCloudStrategy with a
specifications.functions entry containing a non-null unsupported code (e.g., {
functions: [{ code: 'unsupported_code' }], status: [] }, deviceType
'smart-socket') and assert it returns null; (2) call getConfiguredCloudStrategy
with a missing/undefined device payload (e.g., invoke
getConfiguredCloudStrategy() or pass undefined) and assert it returns
CLOUD_STRATEGY.LEGACY. Reference resolveCloudStrategy,
getConfiguredCloudStrategy, and CLOUD_STRATEGY when adding these tests.
server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js (1)

191-234: Consider table-driven tests to reduce duplication in energy-sensor scale cases.

The current tests are correct, but this block is repetitive and can be made easier to extend/maintain with parameterized cases.

♻️ Optional refactor sketch
+      [
+        { type: DEVICE_FEATURE_TYPES.ENERGY_SENSOR.POWER, raw: '706', scale: 1, expected: 70.6 },
+        { type: DEVICE_FEATURE_TYPES.ENERGY_SENSOR.ENERGY, raw: '149241', scale: 2, expected: 1492.41 },
+        { type: DEVICE_FEATURE_TYPES.ENERGY_SENSOR.INDEX_TODAY, raw: '149241', scale: 2, expected: 1492.41 },
+        { type: DEVICE_FEATURE_TYPES.ENERGY_SENSOR.VOLTAGE, raw: '2301', scale: 1, expected: 230.1 },
+        { type: DEVICE_FEATURE_TYPES.ENERGY_SENSOR.CURRENT, raw: '123', scale: 0, expected: 123 },
+      ].forEach(({ type, raw, scale, expected }) => {
+        it(`${type} with scale ${scale}`, () => {
+          const result = readValues[DEVICE_FEATURE_CATEGORIES.ENERGY_SENSOR][type](raw, { scale });
+          expect(result).to.eq(expected);
+        });
+      });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js`
around lines 191 - 234, The energy-sensor tests under the 'energy sensor'
describe block are repetitive; replace the repeated it(...) cases with a
table-driven (parameterized) test: create an array of case objects containing
the category key (e.g., DEVICE_FEATURE_CATEGORIES.ENERGY_SENSOR or
ENERGY_PRODUCTION_SENSOR), the feature type constant (e.g.,
DEVICE_FEATURE_TYPES.ENERGY_SENSOR.POWER, .ENERGY, .INDEX_TODAY, .VOLTAGE,
.CURRENT or DEVICE_FEATURE_TYPES.ENERGY_PRODUCTION_SENSOR.INDEX), the raw value
string, the options object (like { scale: 1 } or { scale: 2 }), and the expected
numeric result, then iterate over the array and for each case call the same
readValues[...] invocation and assert equality; keep the existing test names by
deriving them from case properties so behavior and coverage stay identical.
server/test/services/tuya/lib/utils/tuya.normalize.test.js (1)

11-17: Consider a table-driven temperature-unit test matrix.

You can consolidate and expand alias coverage (especially Fahrenheit + trim/case normalization) with one compact parameterized test.

♻️ Proposed refactor
-  it('should return null for unknown temperature units', () => {
-    expect(normalizeTemperatureUnit('kelvin')).to.equal(null);
-  });
-
-  it('should normalize celsius aliases', () => {
-    expect(normalizeTemperatureUnit('celcius')).to.equal(DEVICE_FEATURE_UNITS.CELSIUS);
-  });
+  it('should normalize temperature units across known aliases', () => {
+    const cases = [
+      ['kelvin', null],
+      ['celcius', DEVICE_FEATURE_UNITS.CELSIUS],
+      ['fahrenheit', DEVICE_FEATURE_UNITS.FAHRENHEIT],
+      [' F ', DEVICE_FEATURE_UNITS.FAHRENHEIT],
+      ['℃', DEVICE_FEATURE_UNITS.CELSIUS],
+      ['℉', DEVICE_FEATURE_UNITS.FAHRENHEIT],
+    ];
+
+    cases.forEach(([input, expected]) => {
+      expect(normalizeTemperatureUnit(input)).to.equal(expected);
+    });
+  });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/utils/tuya.normalize.test.js` around lines 11 -
17, Replace the two ad-hoc tests with a single table-driven (parameterized) test
that iterates over a list of {input, expected} rows to cover unknown units,
celsius aliases (e.g., "celcius", "Celsius", with surrounding whitespace),
fahrenheit aliases (e.g., "F", "fahrenheit", mixed case and trimmed) and invalid
units like "kelvin"; call normalizeTemperatureUnit for each row and assert
equality against expected (use DEVICE_FEATURE_UNITS.CELSIUS,
DEVICE_FEATURE_UNITS.FAHRENHEIT or null), so the file-level tests become concise
and cover case/trim normalization and alias variants in one loop.
server/test/services/tuya/lib/tuya.setValue.test.js (2)

45-62: Consider adding explicit failure when no error is thrown.

The try/catch pattern doesn't fail the test if setValue unexpectedly resolves without throwing. Later tests (e.g., lines 572-578, 601-606) correctly use expect.fail('Expected setValue to throw'). Consider aligning this test for consistency.

Suggested improvement
     try {
       await tuyaHandler.setValue(
         {},
         {
           external_id: 'test:uuid:switch_0',
           category: DEVICE_FEATURE_CATEGORIES.SWITCH,
           type: DEVICE_FEATURE_TYPES.SWITCH.BINARY,
         },
         1,
       );
+      expect.fail('Expected setValue to throw');
     } catch (error) {
       expect(error).to.be.an.instanceof(BadParameters);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.setValue.test.js` around lines 45 - 62,
The test for tuyaHandler.setValue currently catches the error but doesn't fail
if no error is thrown; update the 'should throw an error (should starts with
"tuya:")' test to explicitly fail when setValue resolves by adding an assertion
like expect.fail('Expected setValue to throw') immediately after the await call
(or throw inside the try) so the test fails if no exception is raised, keeping
the existing error checks for BadParameters and the message intact.

740-779: Test name mentions logging but doesn't verify log output.

The test correctly verifies that setValue completes successfully despite a disconnect error, but the test name "should log disconnect failures and still return on local success" implies logging verification which isn't performed. Consider either:

  1. Adding logger mock via proxyquire (similar to lines 677-712) to verify the warning log
  2. Updating the test name to "should succeed on local write even if disconnect fails"

This is a minor naming inconsistency that doesn't affect test validity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.setValue.test.js` around lines 740 - 779,
The test name says it should log disconnect failures but the test does not
assert any logging; update the test by either (A) injecting a logger mock via
proxyquire (as done earlier in the file) and asserting the warning was called
when TuyAPI.disconnect rejects, or (B) change the it(...) description to "should
succeed on local write even if disconnect fails" to reflect the current
assertions; target the test around the setValue import and the
TuyAPIStub/connect/set/disconnect stubs so the logging mock or name change
applies to this specific case.
server/test/services/tuya/lib/tuya.poll.test.js (2)

179-179: Use unique test titles for the two shadow-strategy cases.

Both it(...) blocks have the same name, which makes failing output ambiguous. Please rename one with clearer context.

Also applies to: 663-663

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.poll.test.js` at line 179, Two tests share
the identical title "should read cloud values from thing shadow when strategy is
shadow", making failures ambiguous; update the test titles for the two `it(...)`
blocks (the one at the current diff and the other around the second location) to
unique, descriptive names (for example add context like "when shadow contains
reported state" and "when shadow contains desired state" or "with missing
reported values") by editing the `it(...)` descriptions in the offending test
cases so each title clearly differentiates the scenario under test.

388-396: “Persist reported unit” is not actually being tested.

These tests mention persistence, but setParam is never wired into the execution path and is asserted as not called. This currently validates conversion only, not unit persistence.

Suggested cleanup if conversion-only is intended
-  it('should convert thermostat temperatures from local dps unit and persist reported unit', async () => {
+  it('should convert thermostat temperatures from local dps unit', async () => {
@@
-    const setParam = sinon.stub().resolves();
@@
-    expect(setParam.called).to.equal(false);
-  it('should convert cloud thermostat temperatures from reported unit', async () => {
+  it('should convert cloud thermostat temperatures from reported unit', async () => {
@@
-    const setParam = sinon.stub().resolves();
@@
-    expect(setParam.called).to.equal(false);

Also applies to: 438-438, 573-583, 612-612

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.poll.test.js` around lines 388 - 396, The
test claims to verify "persist reported unit" but never wires the setParam stub
into the module under test so poll never calls it; update the proxyquire setup
in tuya.poll.test.js to inject your setParam stub into the module that poll uses
(the service/params or device state helper used by poll) so that the poll
function will call setParam (e.g., proxyquire
'../../services/tuya/lib/tuya.poll' with the dependency that exposes setParam
replaced by your stub), then assert setParam was called with the expected unit;
alternatively, if you only want to test conversion, remove the persistence claim
and related assertions for setParam in the tests at the referenced locations
(including the other cases at ~438 and 573–583, 612).
server/services/tuya/lib/tuya.poll.js (1)

77-80: Consider importing hasDpsKey from tuya.localMapping.js to avoid duplication.

This helper duplicates the implementation in server/services/tuya/lib/device/tuya.localMapping.js (lines 43-49). The local version omits the null/type guard, which is safe here since dps is validated at line 383 before use, but importing the existing utility would reduce maintenance overhead.

♻️ Suggested refactor
-const { getLocalDpsFromCode } = require('./device/tuya.localMapping');
+const { getLocalDpsFromCode, hasDpsKey } = require('./device/tuya.localMapping');

Then remove lines 77-80.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 77 - 80, The helper
hasDpsKey in tuya.poll.js duplicates logic already defined in
server/services/tuya/lib/device/tuya.localMapping.js; replace the duplicate by
importing the existing hasDpsKey from that module (update the module import at
the top of tuya.poll.js to include hasDpsKey) and remove the local definition
(lines defining const hasDpsKey = ...) so callers in tuya.poll.js use the shared
function to avoid maintenance drift.
server/services/tuya/lib/tuya.localPoll.js (1)

113-121: Prefer iterative retry over recursion for readability.

The current recursion is bounded, but a small loop is easier to maintain in this hot path.

♻️ Suggested refactor
-    const tryAttempt = async (index) => {
-      try {
-        return await runGet(attempts[index]);
-      } catch (e) {
-        if (index >= attempts.length - 1) {
-          throw e;
-        }
-        return tryAttempt(index + 1);
-      }
-    };
-    const data = await tryAttempt(0);
+    let data;
+    let lastAttemptError;
+    for (let i = 0; i < attempts.length; i += 1) {
+      try {
+        data = await runGet(attempts[i]);
+        break;
+      } catch (e) {
+        lastAttemptError = e;
+        if (i === attempts.length - 1) {
+          throw e;
+        }
+      }
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.localPoll.js` around lines 113 - 121, Replace
the recursive retry helper tryAttempt with an iterative loop: instead of calling
tryAttempt(index + 1) on failure, iterate over attempts using a for (let i = 0;
i < attempts.length; i++) loop, call await runGet(attempts[i]) inside the loop,
return the result on success, and on failure continue to the next iteration; if
the final attempt fails rethrow that error so behavior matches the current
logic; update any references to tryAttempt to call the new iterative retry logic
in the same scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/test/services/tuya/lib/tuya.localPoll.test.js`:
- Around line 7-8: The test is requiring the module
(updateDiscoveredDeviceAfterLocalPoll) before proxyquire is configured, so stubs
don't replace it; update the proxyquire setup to use noPreserveCache by changing
the proxyquire instantiation to use .noCallThru().noPreserveCache(), and remove
or move the direct require of '../../../../services/tuya/lib/tuya.localPoll'
(the updateDiscoveredDeviceAfterLocalPoll import) so that proxyquire can load
the module with your stubs applied.

---

Nitpick comments:
In `@server/services/tuya/lib/tuya.localPoll.js`:
- Around line 113-121: Replace the recursive retry helper tryAttempt with an
iterative loop: instead of calling tryAttempt(index + 1) on failure, iterate
over attempts using a for (let i = 0; i < attempts.length; i++) loop, call await
runGet(attempts[i]) inside the loop, return the result on success, and on
failure continue to the next iteration; if the final attempt fails rethrow that
error so behavior matches the current logic; update any references to tryAttempt
to call the new iterative retry logic in the same scope.

In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 77-80: The helper hasDpsKey in tuya.poll.js duplicates logic
already defined in server/services/tuya/lib/device/tuya.localMapping.js; replace
the duplicate by importing the existing hasDpsKey from that module (update the
module import at the top of tuya.poll.js to include hasDpsKey) and remove the
local definition (lines defining const hasDpsKey = ...) so callers in
tuya.poll.js use the shared function to avoid maintenance drift.

In `@server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js`:
- Around line 191-234: The energy-sensor tests under the 'energy sensor'
describe block are repetitive; replace the repeated it(...) cases with a
table-driven (parameterized) test: create an array of case objects containing
the category key (e.g., DEVICE_FEATURE_CATEGORIES.ENERGY_SENSOR or
ENERGY_PRODUCTION_SENSOR), the feature type constant (e.g.,
DEVICE_FEATURE_TYPES.ENERGY_SENSOR.POWER, .ENERGY, .INDEX_TODAY, .VOLTAGE,
.CURRENT or DEVICE_FEATURE_TYPES.ENERGY_PRODUCTION_SENSOR.INDEX), the raw value
string, the options object (like { scale: 1 } or { scale: 2 }), and the expected
numeric result, then iterate over the array and for each case call the same
readValues[...] invocation and assert equality; keep the existing test names by
deriving them from case properties so behavior and coverage stay identical.

In `@server/test/services/tuya/lib/tuya.poll.test.js`:
- Line 179: Two tests share the identical title "should read cloud values from
thing shadow when strategy is shadow", making failures ambiguous; update the
test titles for the two `it(...)` blocks (the one at the current diff and the
other around the second location) to unique, descriptive names (for example add
context like "when shadow contains reported state" and "when shadow contains
desired state" or "with missing reported values") by editing the `it(...)`
descriptions in the offending test cases so each title clearly differentiates
the scenario under test.
- Around line 388-396: The test claims to verify "persist reported unit" but
never wires the setParam stub into the module under test so poll never calls it;
update the proxyquire setup in tuya.poll.test.js to inject your setParam stub
into the module that poll uses (the service/params or device state helper used
by poll) so that the poll function will call setParam (e.g., proxyquire
'../../services/tuya/lib/tuya.poll' with the dependency that exposes setParam
replaced by your stub), then assert setParam was called with the expected unit;
alternatively, if you only want to test conversion, remove the persistence claim
and related assertions for setParam in the tests at the referenced locations
(including the other cases at ~438 and 573–583, 612).

In `@server/test/services/tuya/lib/tuya.setValue.test.js`:
- Around line 45-62: The test for tuyaHandler.setValue currently catches the
error but doesn't fail if no error is thrown; update the 'should throw an error
(should starts with "tuya:")' test to explicitly fail when setValue resolves by
adding an assertion like expect.fail('Expected setValue to throw') immediately
after the await call (or throw inside the try) so the test fails if no exception
is raised, keeping the existing error checks for BadParameters and the message
intact.
- Around line 740-779: The test name says it should log disconnect failures but
the test does not assert any logging; update the test by either (A) injecting a
logger mock via proxyquire (as done earlier in the file) and asserting the
warning was called when TuyAPI.disconnect rejects, or (B) change the it(...)
description to "should succeed on local write even if disconnect fails" to
reflect the current assertions; target the test around the setValue import and
the TuyAPIStub/connect/set/disconnect stubs so the logging mock or name change
applies to this specific case.

In `@server/test/services/tuya/lib/utils/tuya.cloudStrategy.test.js`:
- Around line 10-40: Add two edge-case assertions: (1) call resolveCloudStrategy
with a specifications.functions entry containing a non-null unsupported code
(e.g., { functions: [{ code: 'unsupported_code' }], status: [] }, deviceType
'smart-socket') and assert it returns null; (2) call getConfiguredCloudStrategy
with a missing/undefined device payload (e.g., invoke
getConfiguredCloudStrategy() or pass undefined) and assert it returns
CLOUD_STRATEGY.LEGACY. Reference resolveCloudStrategy,
getConfiguredCloudStrategy, and CLOUD_STRATEGY when adding these tests.

In `@server/test/services/tuya/lib/utils/tuya.normalize.test.js`:
- Around line 11-17: Replace the two ad-hoc tests with a single table-driven
(parameterized) test that iterates over a list of {input, expected} rows to
cover unknown units, celsius aliases (e.g., "celcius", "Celsius", with
surrounding whitespace), fahrenheit aliases (e.g., "F", "fahrenheit", mixed case
and trimmed) and invalid units like "kelvin"; call normalizeTemperatureUnit for
each row and assert equality against expected (use DEVICE_FEATURE_UNITS.CELSIUS,
DEVICE_FEATURE_UNITS.FAHRENHEIT or null), so the file-level tests become concise
and cover case/trim normalization and alias variants in one loop.

In `@server/test/services/tuya/lib/utils/tuya.report.test.js`:
- Around line 46-48: The test currently asserts three specific null fields on
report.cloud.raw (device_list_entry, device_specification, thing_model) but
should assert the full degraded-mode contract; update the test to iterate over
all properties of report.cloud.raw (e.g., Object.keys(report.cloud.raw)) and
assert each value is null (or add explicit expect statements for any remaining
known raw fields) so every raw field on the report object is verified as null in
this degraded case.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b359f1 and df476d3.

📒 Files selected for processing (9)
  • server/services/tuya/lib/tuya.localPoll.js
  • server/services/tuya/lib/tuya.poll.js
  • server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js
  • server/test/services/tuya/lib/tuya.localPoll.test.js
  • server/test/services/tuya/lib/tuya.poll.test.js
  • server/test/services/tuya/lib/tuya.setValue.test.js
  • server/test/services/tuya/lib/utils/tuya.cloudStrategy.test.js
  • server/test/services/tuya/lib/utils/tuya.normalize.test.js
  • server/test/services/tuya/lib/utils/tuya.report.test.js

Comment on lines +7 to +8
const { updateDiscoveredDeviceAfterLocalPoll } = require('../../../../services/tuya/lib/tuya.localPoll');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "tuya.localPoll.test.js" -type f

Repository: GladysAssistant/Gladys

Length of output: 120


🏁 Script executed:

# Read the test file to see the actual import pattern
cat -n server/test/services/tuya/lib/tuya.localPoll.test.js | head -60

Repository: GladysAssistant/Gladys

Length of output: 2730


🌐 Web query:

proxyquire cache behavior noPreserveCache default behavior

💡 Result:

  • Default behavior (no noPreserveCache): proxyquire(...) follows Node’s normal require caching semantics—once a module is loaded, subsequent loads return the cached instance. [1]
  • noPreserveCache() behavior: require('proxyquire').noPreserveCache() returns a proxyquire instance that does not preserve the require cache, so repeated proxyquire('./foo', ...) calls return fresh instances (foo1 !== foo2) and even a later plain require('./foo') will not match a previously proxyquired instance (foo1 !== foo3). [1]
  • Reverting: Call proxyquire.preserveCache() to restore normal Node-like caching (then foo1 === foo2 === foo3). [1]
  • So the default: noPreserveCache is off/disabled by default; you must opt in. [1] [2]

Add .noPreserveCache() to enable proxyquire stubs to override the cached module.

Line 7 directly requires the module before proxyquire, which loads it into Node's require cache. Since line 4 uses only .noCallThru() (not .noPreserveCache()), the subsequent proxyquire() calls at lines 17–20 and 41–44 will return the cached version from line 7 rather than the stubbed version. Change line 4 to const proxyquire = require('proxyquire').noCallThru().noPreserveCache(); to ensure stubs are applied correctly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localPoll.test.js` around lines 7 - 8, The
test is requiring the module (updateDiscoveredDeviceAfterLocalPoll) before
proxyquire is configured, so stubs don't replace it; update the proxyquire setup
to use noPreserveCache by changing the proxyquire instantiation to use
.noCallThru().noPreserveCache(), and remove or move the direct require of
'../../../../services/tuya/lib/tuya.localPoll' (the
updateDiscoveredDeviceAfterLocalPoll import) so that proxyquire can load the
module with your stubs applied.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
server/services/tuya/lib/tuya.poll.js (1)

164-173: Consider adding a comment to clarify the range-based fallback heuristic.

This logic handles cases where Tuya devices may report values in a different unit than their properties claim. If the converted value falls outside the feature's range but the raw value fits, it assumes the device is already reporting in the expected unit. While this is a reasonable heuristic given Tuya's inconsistencies, a brief comment would help future maintainers understand this defensive behavior.

📝 Suggested documentation
   const roundedValue = roundTemperatureValue(convertedValue, code);
   if (code === 'temp_current' && deviceFeature) {
     const min = Number(deviceFeature.min);
     const max = Number(deviceFeature.max);
     if (Number.isFinite(min) && Number.isFinite(max)) {
       const isConvertedInRange = roundedValue >= min && roundedValue <= max;
       const isRawInRange = numericValue >= min && numericValue <= max;
+      // Tuya devices sometimes report values in the expected unit despite properties
+      // claiming otherwise. If conversion pushes value out of range but raw fits,
+      // assume device is already reporting in the expected unit.
       if (!isConvertedInRange && isRawInRange) {
         return roundTemperatureValue(numericValue, code);
       }
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 164 - 173, Add a brief
inline comment above the range-check in the temp_current handling (the block
using variables deviceFeature, min, max, roundedValue, numericValue and
returning roundTemperatureValue(numericValue, code)) explaining the fallback
heuristic: when the converted/rounded value falls outside the declared feature
range but the raw numericValue is inside, we assume the device reported in the
expected unit already and therefore use the raw value; mention this is a
defensive workaround for Tuya unit/reporting inconsistencies to help future
maintainers understand the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 164-173: Add a brief inline comment above the range-check in the
temp_current handling (the block using variables deviceFeature, min, max,
roundedValue, numericValue and returning roundTemperatureValue(numericValue,
code)) explaining the fallback heuristic: when the converted/rounded value falls
outside the declared feature range but the raw numericValue is inside, we assume
the device reported in the expected unit already and therefore use the raw
value; mention this is a defensive workaround for Tuya unit/reporting
inconsistencies to help future maintainers understand the intent.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df476d3 and 33241a1.

📒 Files selected for processing (1)
  • server/services/tuya/lib/tuya.poll.js

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
server/test/services/tuya/lib/tuya.localPoll.test.js (1)

4-7: ⚠️ Potential issue | 🟠 Major

Proxyquire stubs can be bypassed by module cache priming.

tuya.localPoll is required at Line 7 before proxyquire stubbing, and proxyquire is missing .noPreserveCache() at Line 4. This can cause cached module reuse and make localPoll tests run against non-stubbed dependencies.

🔧 Proposed fix
-const proxyquire = require('proxyquire').noCallThru();
+const proxyquire = require('proxyquire').noCallThru().noPreserveCache();
 const { BadParameters } = require('../../../../utils/coreErrors');
 const { DEVICE_PARAM_NAME } = require('../../../../services/tuya/lib/utils/tuya.constants');
 const { updateDiscoveredDeviceAfterLocalPoll } = require('../../../../services/tuya/lib/tuya.localPoll');
#!/bin/bash
# Verify cache-related test setup in tuya.localPoll.test.js
fd tuya.localPoll.test.js --exec sh -c '
  echo "== $1 ==";
  nl -ba "$1" | sed -n "1,25p"
' sh {}

# Inspect proxyquire cache configuration + early direct require ordering
rg -n --type js "proxyquire.*noCallThru|noPreserveCache|updateDiscoveredDeviceAfterLocalPoll.*tuya\\.localPoll"

Expected verification outcome:

  • noPreserveCache should be present in proxyquire initialization.
  • If direct require remains before proxyquire calls, stubbing relies on noPreserveCache to stay deterministic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localPoll.test.js` around lines 4 - 7, The
test requires tuya.localPoll (updateDiscoveredDeviceAfterLocalPoll) before
proxyquire stubs are applied and proxyquire is initialized with only
.noCallThru(), risking cached module usage; fix by initializing proxyquire with
.noCallThru().noPreserveCache() and/or moving the direct require of
updateDiscoveredDeviceAfterLocalPoll out of the top-level so the module is
loaded via proxyquire after stubs are configured (i.e., replace the early
require of updateDiscoveredDeviceAfterLocalPoll in tuya.localPoll.test.js with a
proxyquire-loaded import or ensure proxyquire.noPreserveCache() is used before
any requires).
🧹 Nitpick comments (3)
server/services/tuya/lib/device/tuya.convertFeature.js (1)

66-73: Guard thermostat min/max scaling when scale is not numeric.

10 ** feature.scale can produce NaN on malformed payloads, which then propagates into feature.min/feature.max. A finite-number guard keeps feature metadata stable.

Suggested hardening
   if (
     feature.scale !== undefined &&
     feature.category === DEVICE_FEATURE_CATEGORIES.THERMOSTAT &&
     feature.type === DEVICE_FEATURE_TYPES.THERMOSTAT.TARGET_TEMPERATURE
   ) {
-    const divider = 10 ** feature.scale;
-    feature.min /= divider;
-    feature.max /= divider;
+    const scale = Number(feature.scale);
+    if (Number.isFinite(scale)) {
+      const divider = 10 ** scale;
+      feature.min /= divider;
+      feature.max /= divider;
+    } else {
+      logger.warn(`Tuya function with "${code}" has invalid scale "${feature.scale}"`);
+    }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/device/tuya.convertFeature.js` around lines 66 - 73,
The thermostat min/max scaling code can produce NaN when feature.scale is
malformed; update the block in tuya.convertFeature.js to first coerce and
validate the scale (e.g., const scale = Number(feature.scale)) and only compute
divider and adjust feature.min/feature.max if Number.isFinite(scale) (and
divider !== 0) so that DEVICE_FEATURE_CATEGORIES.THERMOSTAT /
DEVICE_FEATURE_TYPES.THERMOSTAT.TARGET_TEMPERATURE handling is skipped for
non-numeric scales, leaving metadata unchanged.
server/services/tuya/lib/tuya.poll.js (1)

199-210: Use a null-prototype map for cloud value accumulation.

feature.code comes from external payloads; writing into {} can behave oddly for special keys. Prefer a null-prototype object (or Map) for safer lookups.

♻️ Proposed fix
-  const values = {};
+  const values = Object.create(null);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 199 - 210, The
accumulator object in extractValuesFromResultArray should be a null-prototype
map to avoid prototype-key collisions when using externally-supplied
feature.code; replace the plain {} used for values with Object.create(null) (or
a Map if preferred) and keep the existing assignments
(values[String(feature.code)] = feature.value) and the
updatePropertyValue(device, feature.code, feature.value) call; ensure the
function still returns the new null-prototype object (or convert and return a
plain object only if callers require it).
server/test/services/tuya/lib/tuya.poll.test.js (1)

179-179: Disambiguate duplicated test titles.

Both cases use the same it(...) description for shadow strategy; renaming one with context (e.g., “shadow + temperature conversion”) will make CI failures easier to pinpoint.

Also applies to: 628-628

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.poll.test.js` at line 179, Two tests share
the same it(...) title "should read cloud values from thing shadow when strategy
is shadow", which causes ambiguity in CI failures; locate the duplicate it(...)
blocks (the one at the shown diff and the other around the later occurrence) and
rename one of the test titles to something specific (for example "should read
cloud values from thing shadow when strategy is shadow + temperature conversion"
or "should read cloud values from thing shadow when strategy is shadow -
humidity case") so each it(...) description is unique and clearly identifies the
scenario.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/services/tuya/lib/device/tuya.convertFeature.js`:
- Around line 19-20: The override list ignoredCloudCodes is used without
normalization so mixed-case entries won't match codeLower; normalize
ignoredCloudCodes to lower-case before comparing (e.g., map each entry to
.toLowerCase()) when building ignoredCodes and then use
ignoredCodes.includes(codeLower); update the logic around the ignoredCloudCodes
variable and the creation of ignoredCodes (and any use in the same scope) to
ensure case-insensitive matching with getIgnoredCloudCodes and codeLower.

In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 252-261: The cloud polling calls to this.connector.request (used
in the conditional that checks CLOUD_STRATEGY.SHADOW and builds paths with
API.VERSION_2_0 / API.VERSION_1_0) lack an explicit timeout and can hang; update
these request invocations to pass a timeout parameter (e.g., timeoutMs: 3000) as
done in local polling, and likewise add timeout parameters to other cloud-facing
calls such as setValue and device load functions or instead ensure the
TuyaContext/connector is initialized with a sensible default timeout so every
this.connector.request call enforces a timeout.

---

Duplicate comments:
In `@server/test/services/tuya/lib/tuya.localPoll.test.js`:
- Around line 4-7: The test requires tuya.localPoll
(updateDiscoveredDeviceAfterLocalPoll) before proxyquire stubs are applied and
proxyquire is initialized with only .noCallThru(), risking cached module usage;
fix by initializing proxyquire with .noCallThru().noPreserveCache() and/or
moving the direct require of updateDiscoveredDeviceAfterLocalPoll out of the
top-level so the module is loaded via proxyquire after stubs are configured
(i.e., replace the early require of updateDiscoveredDeviceAfterLocalPoll in
tuya.localPoll.test.js with a proxyquire-loaded import or ensure
proxyquire.noPreserveCache() is used before any requires).

---

Nitpick comments:
In `@server/services/tuya/lib/device/tuya.convertFeature.js`:
- Around line 66-73: The thermostat min/max scaling code can produce NaN when
feature.scale is malformed; update the block in tuya.convertFeature.js to first
coerce and validate the scale (e.g., const scale = Number(feature.scale)) and
only compute divider and adjust feature.min/feature.max if
Number.isFinite(scale) (and divider !== 0) so that
DEVICE_FEATURE_CATEGORIES.THERMOSTAT /
DEVICE_FEATURE_TYPES.THERMOSTAT.TARGET_TEMPERATURE handling is skipped for
non-numeric scales, leaving metadata unchanged.

In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 199-210: The accumulator object in extractValuesFromResultArray
should be a null-prototype map to avoid prototype-key collisions when using
externally-supplied feature.code; replace the plain {} used for values with
Object.create(null) (or a Map if preferred) and keep the existing assignments
(values[String(feature.code)] = feature.value) and the
updatePropertyValue(device, feature.code, feature.value) call; ensure the
function still returns the new null-prototype object (or convert and return a
plain object only if callers require it).

In `@server/test/services/tuya/lib/tuya.poll.test.js`:
- Line 179: Two tests share the same it(...) title "should read cloud values
from thing shadow when strategy is shadow", which causes ambiguity in CI
failures; locate the duplicate it(...) blocks (the one at the shown diff and the
other around the later occurrence) and rename one of the test titles to
something specific (for example "should read cloud values from thing shadow when
strategy is shadow + temperature conversion" or "should read cloud values from
thing shadow when strategy is shadow - humidity case") so each it(...)
description is unique and clearly identifies the scenario.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59d9c96d-62de-4993-bdb0-59ee2137d13e

📥 Commits

Reviewing files that changed from the base of the PR and between 33241a1 and 034de26.

📒 Files selected for processing (4)
  • server/services/tuya/lib/device/tuya.convertFeature.js
  • server/services/tuya/lib/tuya.poll.js
  • server/test/services/tuya/lib/tuya.localPoll.test.js
  • server/test/services/tuya/lib/tuya.poll.test.js

Comment on lines +19 to +20
const ignoredCodes = Array.isArray(ignoredCloudCodes) ? ignoredCloudCodes : getIgnoredCloudCodes(deviceType);
if (codeLower && ignoredCodes.includes(codeLower)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Normalize overridden ignoredCloudCodes before matching.

codeLower is normalized, but options.ignoredCloudCodes is used as-is. Mixed-case overrides won’t match and ignored codes can slip through.

Proposed fix
-  const ignoredCodes = Array.isArray(ignoredCloudCodes) ? ignoredCloudCodes : getIgnoredCloudCodes(deviceType);
+  const ignoredCodes = (Array.isArray(ignoredCloudCodes) ? ignoredCloudCodes : getIgnoredCloudCodes(deviceType))
+    .map((value) => normalizeCode(value))
+    .filter((value) => value !== null);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ignoredCodes = Array.isArray(ignoredCloudCodes) ? ignoredCloudCodes : getIgnoredCloudCodes(deviceType);
if (codeLower && ignoredCodes.includes(codeLower)) {
const ignoredCodes = (Array.isArray(ignoredCloudCodes) ? ignoredCloudCodes : getIgnoredCloudCodes(deviceType))
.map((value) => normalizeCode(value))
.filter((value) => value !== null);
if (codeLower && ignoredCodes.includes(codeLower)) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/device/tuya.convertFeature.js` around lines 19 - 20,
The override list ignoredCloudCodes is used without normalization so mixed-case
entries won't match codeLower; normalize ignoredCloudCodes to lower-case before
comparing (e.g., map each entry to .toLowerCase()) when building ignoredCodes
and then use ignoredCodes.includes(codeLower); update the logic around the
ignoredCloudCodes variable and the creation of ignoredCodes (and any use in the
same scope) to ensure case-insensitive matching with getIgnoredCloudCodes and
codeLower.

Comment on lines +252 to +261
const response =
cloudStrategy === CLOUD_STRATEGY.SHADOW
? await this.connector.request({
method: 'GET',
path: `${API.VERSION_2_0}/thing/${topic}/shadow/properties`,
})
: await this.connector.request({
method: 'GET',
path: `${API.VERSION_1_0}/devices/${topic}/status`,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate Tuya connector setup =="
fd 'tuya.connect.js' server/services/tuya/lib -t f -x sh -c 'echo "--- {} ---"; rg -n -C3 "TuyaContext|timeout|request" "{}"'

echo
echo "== Inspect poll cloud request call sites =="
rg -n -C3 'connector\.request\(|path:\s*`\$\{API\.' server/services/tuya/lib/tuya.poll.js

echo
echo "Expected result:"
echo "- Either connector initialization includes a timeout,"
echo "  or request invocations in polling pass timeout options."

Repository: GladysAssistant/Gladys

Length of output: 1396


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Full TuyaContext initialization (tuya.connect.js) =="
sed -n '100,120p' server/services/tuya/lib/tuya.connect.js

echo
echo "== Search for local polling timeout =="
rg -n 'timeoutMs|timeout' server/services/tuya/lib/tuya.poll.js -B2 -A2

echo
echo "== Check if any request calls pass timeout =="
rg -n 'connector\.request' server/services/tuya/lib/tuya.poll.js -A5

echo
echo "== Check TuyaContext package in node_modules or package.json =="
if [ -f server/services/tuya/package.json ]; then
  grep -A2 -B2 '@tuya/tuya-connector' server/services/tuya/package.json || true
fi

echo
echo "== Search for timeout configuration across connector calls =="
rg -n 'timeout' server/services/tuya/lib/tuya.connect.js -B2 -A2

Repository: GladysAssistant/Gladys

Length of output: 1943


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== All connector.request calls in tuya service =="
rg -n 'connector\.request' server/services/tuya/lib -A6 -B1

echo
echo "== Check if request method supports timeout in TuyaContext docs =="
echo "Looking for timeout usage pattern in request calls..."
rg -rn 'timeout' server/services/tuya/lib --type js | head -20

echo
echo "== Inspect the full cloud polling function =="
sed -n '240,280p' server/services/tuya/lib/tuya.poll.js

Repository: GladysAssistant/Gladys

Length of output: 8452


Add explicit timeout to cloud API requests to prevent stalled calls from blocking operations.

Cloud API requests to Tuya (this.connector.request() at lines 254 and 258) lack timeout configuration, while local polling explicitly sets timeoutMs: 3000. Without a timeout, a stalled Tuya API call can hang the polling operation indefinitely. This issue affects not only cloud polling but also cloud-based setValue operations and device loading calls across the service. All connector.request() invocations should include explicit timeout parameters or the TuyaContext connector should be initialized with a default timeout.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 252 - 261, The cloud
polling calls to this.connector.request (used in the conditional that checks
CLOUD_STRATEGY.SHADOW and builds paths with API.VERSION_2_0 / API.VERSION_1_0)
lack an explicit timeout and can hang; update these request invocations to pass
a timeout parameter (e.g., timeoutMs: 3000) as done in local polling, and
likewise add timeout parameters to other cloud-facing calls such as setValue and
device load functions or instead ensure the TuyaContext/connector is initialized
with a sensible default timeout so every this.connector.request call enforces a
timeout.

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