Tuya - Add support for LSC Power Plug FR smart socket - PR7#2476
Open
Terdious wants to merge 98 commits intoGladysAssistant:masterfrom
Open
Tuya - Add support for LSC Power Plug FR smart socket - PR7#2476Terdious wants to merge 98 commits intoGladysAssistant:masterfrom
Terdious wants to merge 98 commits intoGladysAssistant:masterfrom
Conversation
…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.
…y device conversion logic
…ttre à jour les tests associés
… champ d'erreur dans le payload de l'événement WebSocket
…s et ajouter des tests pour la gestion des appareils locaux
…on Tuya dans les fichiers de langue
…ion Tuya et mettre à jour les traductions
… des appareils Tuya
…tre l'utilisation d'adresses spécifiques
…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.
…ls et des tests associés
… débogage pour la gestion des appareils
…age, ajouter des tests pour la reconnexion automatique et la découverte des appareils
… des paramètres dans le code de configuration Tuya
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces comprehensive local polling and local area network discovery for Tuya devices, along with device-specific mappings for smart sockets and smart meters. It adds backend APIs for local operations, frontend UI components for device management, internationalization support across multiple languages, and extensive test fixtures and coverage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2476 +/- ##
==========================================
+ Coverage 98.79% 98.87% +0.07%
==========================================
Files 1006 1024 +18
Lines 17478 18623 +1145
==========================================
+ Hits 17268 18413 +1145
Misses 210 210 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
#4025 Bundle Size — 11.55MiB (+1.25%).44fe379(current) vs 1aade56 master#4019(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #4025 |
Baseline #4019 |
|
|---|---|---|
 Initial JS |
6.54MiB(+2.22%) |
6.4MiB |
| Initial CSS |
310.55KiB(+0.02%) |
310.49KiB |
 Cache Invalidation |
58.65% |
55.98% |
 Chunks |
51 |
51 |
| Assets |
179 |
179 |
| Modules |
1648(+0.3%) |
1643 |
| Duplicate Modules |
21 |
21 |
| Duplicate Code |
0.93%(-1.06%) |
0.94% |
| Packages |
136 |
136 |
| Duplicate Packages |
2 |
2 |
Bundle size by type 2 changes 2 regressions
| Current #4025 |
Baseline #4019 |
|
|---|---|---|
| JS |
8.44MiB (+1.72%) |
8.3MiB |
| IMG |
2.66MiB |
2.66MiB |
| CSS |
328.4KiB (+0.02%) |
328.34KiB |
| Fonts |
93.55KiB |
93.55KiB |
| Other |
18.82KiB |
18.82KiB |
| HTML |
13.58KiB |
13.58KiB |
Bundle analysis report Branch Terdious:tuya-lsc-power-plug-fr-... Project dashboard
Generated by RelativeCI Documentation Report issue
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (15)
server/test/services/tuya/lib/tuya.localScan.test.js (1)
55-68: Always restore fake timers withtry/finallyto avoid cross-test leakage.These tests restore clocks only on the happy path. If an assertion/promise fails before restore, fake timers can leak and destabilize following tests.
♻️ Suggested pattern
+const withFakeClock = async (fn) => { + const clock = sinon.useFakeTimers(); + try { + return await fn(clock); + } finally { + clock.restore(); + } +}; -const clock = sinon.useFakeTimers(); -const promise = localScan(1); -... -await clock.tickAsync(1100); -const result = await promise; -clock.restore(); +const result = await withFakeClock(async (clock) => { + const promise = localScan(1); + ... + await clock.tickAsync(1100); + return promise; +});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, Wrap each test's fake-timer usage in a try/finally so clocks are always restored: when you call sinon.useFakeTimers() and assign to clock (as seen around localScan(1), clock.tickAsync(1100) and clock.restore()), move the clock.restore() into a finally block that always runs after awaiting the promise/result; apply the same change to the other occurrences (around the blocks at lines referenced in the review) to prevent timer leakage between tests.front/src/routes/integration/all/tuya/commons/deviceHelpers.js (1)
60-68: Unify feature code extraction to avoid parser drift.
getKnownDpsKeysandgetUnknownSpecificationCodesparsefeature.external_iddifferently. Centralizing this in one helper will prevent subtle mismatches in future changes.Also applies to: 99-103
🤖 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 60 - 68, The feature code extraction is duplicated and inconsistent between getKnownDpsKeys and getUnknownSpecificationCodes; create a single helper (e.g., parseFeatureCode or getFeatureCodeFromExternalId) that takes feature.external_id (tolerates null/empty), splits on ':' and returns the code at index 2 or null, then replace the current ad-hoc parsing in getKnownDpsKeys and getUnknownSpecificationCodes (and the similar logic around lines 99-103) to call this helper so both functions use the identical extraction logic.server/test/services/tuya/lib/tuya.saveConfiguration.test.js (1)
27-48: Add a second test for empty/omittedappUsernameto avoid stale config regressions.Given
appUsernameis optional in UI, a dedicated case for clearing/unset input would harden this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/lib/tuya.saveConfiguration.test.js` around lines 27 - 48, Add a second unit test for tuyaHandler.saveConfiguration that covers the case where appUsername is empty or omitted: call saveConfiguration with configuration missing or with appUsername set to '' and assert the returned config and that gladys.variable.setValue is called for GLADYS_VARIABLES.APP_USERNAME with an empty string (''), plus the other expected variable sets (ENDPOINT, ACCESS_KEY, SECRET_KEY, APP_ACCOUNT_UID, MANUAL_DISCONNECT, LAST_CONNECTED_CONFIG_HASH); ensure the test name and assertions mirror the existing "should save configuration" test but expect APP_USERNAME to be cleared to '' to prevent stale config regressions.server/services/tuya/lib/tuya.getStatus.js (1)
11-15: Fetch Gladys variables in parallel to reduce status latency.Lines 11–15 are independent lookups and can run concurrently.
Proposed change
- 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 sequential awaits for fetching Gladys variables (endpoint, accessKey, secretKey, appAccountId, manualDisconnect) should be converted to a parallel fetch using Promise.all to reduce latency; update the block in tuya.getStatus.js that currently calls this.gladys.variable.getValue(GLADYS_VARIABLES.* , this.serviceId) five times to a single Promise.all invocation that returns all five values and then assign them to endpoint, accessKey, secretKey, appAccountId, and manualDisconnect respectively so the rest of the function uses the same variable names.server/services/tuya/lib/tuya.setValue.js (1)
47-47: MakehasLocalConfigexplicitly boolean for readability and safer future edits.Line 47 currently relies on truthy chaining; explicit boolean coercion makes intent clearer.
Proposed change
- const hasLocalConfig = ipAddress && localKey && protocolVersion && localOverride === true; + const hasLocalConfig = Boolean(ipAddress && localKey && protocolVersion && localOverride === true);🤖 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` at line 47, The variable hasLocalConfig is currently set via truthy chaining; change its assignment to produce an explicit boolean by coercing the result (e.g., wrap the existing expression with Boolean(...) or prepend with !!) so hasLocalConfig becomes true/false explicitly; update the assignment that references ipAddress, localKey, protocolVersion, and localOverride (where localOverride is compared to true) to use this boolean coercion.server/test/services/tuya/lib/tuya.setValue.fixtures.test.js (1)
47-53: Add an explicit guard when fixturefeatureExternalIdis missing.Line 47 can return
undefined; adding a direct assertion gives clearer fixture failure messages than lettingsetValuefail downstream.Proposed change
const currentDevice = fixtureCase.load(device); const currentFeature = currentDevice.features.find((feature) => feature.external_id === featureExternalId); +if (!currentFeature) { + throw new Error(`Fixture "${fixtureCase.directoryName}" references unknown feature "${featureExternalId}"`); +} 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 47 - 53, The fixture may yield undefined for currentFeature when using currentDevice.features.find(...) with featureExternalId; add an explicit guard/assert (e.g., assert or expect) right after obtaining currentFeature to fail fast with a clear message if featureExternalId is missing, before calling setValue.call(ctx, currentDevice, currentFeature, inputValue), so tests show a descriptive fixture error instead of a downstream failure in setValue.server/test/services/tuya/lib/device/tuya.convertDevice.fixtures.test.js (1)
44-49: Keep degraded fixture mutations consistent withthing_model_raw.Line 44 through Line 49 strips codes from
thing_model, butthing_model_rawremains unchanged. This can make the “feature removed” test brittle if parser precedence changes.♻️ Proposed refactor
if (input.thing_model && Array.isArray(input.thing_model.services)) { input.thing_model.services = input.thing_model.services.map((service) => ({ ...service, properties: filterEntries(service && service.properties), })); } + + if (typeof input.thing_model_raw === 'string') { + try { + const parsedRawModel = JSON.parse(input.thing_model_raw); + if (parsedRawModel && Array.isArray(parsedRawModel.services)) { + parsedRawModel.services = parsedRawModel.services.map((service) => ({ + ...service, + properties: filterEntries(service && service.properties), + })); + input.thing_model_raw = JSON.stringify(parsedRawModel); + } + } catch (e) { + // keep raw payload as-is if malformed + } + }🤖 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.fixtures.test.js` around lines 44 - 49, The test mutates input.thing_model services by stripping codes via the filterEntries mapping but leaves input.thing_model_raw unchanged, making the fixture inconsistent; update the fixture mutation to apply the same transformation to input.thing_model_raw (when it exists and has services) using the same logic used for input.thing_model (i.e., map services and set properties = filterEntries(service && service.properties)) so both thing_model and thing_model_raw are kept in sync for the “feature removed” test.server/test/services/tuya/index.test.js (1)
169-195: Add a regression test forstop()during an in-flight reconnect attempt.The current timeout cleanup test is good, but it doesn’t cover the async window where
getStatus()resolves afterstop()and a new timeout can be scheduled.🧪 Suggested test shape
+ it('should not schedule a new quick reconnect after stop when attempt resolves late', async () => { + const setTimeoutStub = sinon.stub(global, 'setTimeout').callsFake(() => 456); + const clearTimeoutStub = sinon.stub(global, 'clearTimeout'); + let resolveStatus; + const pendingStatus = new Promise((resolve) => { + resolveStatus = resolve; + }); + + try { + tuyaService.device.getStatus = fake.returns(pendingStatus); + tuyaService.device.getConfiguration = fake.resolves({ config: 'ok' }); + tuyaService.device.connect = fake.resolves(); + + await tuyaService.start(); + tuyaService.device.status = STATUS.ERROR; + tuyaService.device.autoReconnectAllowed = true; + + const inFlight = intervalCallback(); + await tuyaService.stop(); + resolveStatus({ configured: true, manual_disconnect: false }); + await inFlight; + + assert.notCalled(setTimeoutStub); + assert.called(clearTimeoutStub); + } finally { + setTimeoutStub.restore(); + clearTimeoutStub.restore(); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/index.test.js` around lines 169 - 195, Add a regression test that simulates an in-flight reconnect where tuyaService.device.getStatus resolves after tuyaService.stop() and could schedule a new timeout; specifically, stub global.setTimeout and global.clearTimeout, arrange tuyaService.device.getStatus to be a promise you resolve after calling tuyaService.stop(), trigger the reconnect flow via tuyaService.start() and intervalCallback(), then resolve the delayed getStatus and assert that setTimeout was not left active (or that clearTimeout was called with the scheduled id) and no extra reconnect occurs; reference the existing test scaffolding (tuyaService.start, tuyaService.stop, tuyaService.device.getStatus, intervalCallback, and the setTimeout/clearTimeout stubs) so the new test mirrors the current "should clear pending quick reconnect timeouts" structure but injects the delayed resolution to cover the async window.server/test/services/tuya/fixtures/fixtureHelper.js (1)
85-91: Direct array access innormalizeEventscould cause cryptic test failures.The function directly accesses
call.args[1]without checking ifargsexists or has sufficient length. If a test stub is misconfigured, this will throw a confusing error rather than a helpful one.Given this is test helper code and the learning about keeping utilities lean, this is a minor hardening suggestion.
🛡️ Optional defensive access
const normalizeEvents = (calls) => sortByKey( - calls.map((call) => ({ - device_feature_external_id: call.args[1].device_feature_external_id, - state: call.args[1].state, - })), + calls.map((call) => { + const payload = call && call.args && call.args[1]; + return { + device_feature_external_id: payload && payload.device_feature_external_id, + state: payload && payload.state, + }; + }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/test/services/tuya/fixtures/fixtureHelper.js` around lines 85 - 91, normalizeEvents currently assumes each stubbed call has an args array with at least two items and directly accesses call.args[1], which can throw confusing errors when tests misconfigure stubs; update normalizeEvents to defensively validate call.args and that call.args.length > 1 (or that call.args[1] exists) before mapping, and either skip malformed entries or throw a clearer error mentioning normalizeEvents and the offending call, so tests fail with an actionable message; reference the normalizeEvents function and the calls parameter when making the change.server/services/tuya/lib/utils/tuya.report.js (1)
30-68: Repetitive nested access patterns could be simplified with optional chaining.The
createBaseReportfunction has many repetitive patterns likecurrentReport.cloud && currentReport.cloud.assembled ? currentReport.cloud.assembled.specifications || null : null. Modern JS optional chaining (?.) would make this more readable.However, given the learning that utilities should remain lean and focused on actual usage patterns, this is a minor stylistic observation rather than a blocking concern.
♻️ Optional simplification using optional chaining
const createBaseReport = (currentReportInput = {}) => { const currentReport = currentReportInput || {}; + const cloudAssembled = currentReport.cloud?.assembled; + const cloudRaw = 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, + specifications: cloudAssembled?.specifications ?? null, // ... similar for other 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 contains repetitive nested null-checks (e.g., currentReport.cloud && currentReport.cloud.assembled ? currentReport.cloud.assembled.specifications || null : null); replace these patterns with optional chaining and nullish coalescing so fields like schema_version, cloud.assembled.specifications, cloud.assembled.properties, cloud.assembled.thing_model, cloud.raw.device_list_entry, cloud.raw.device_specification, cloud.raw.device_details, cloud.raw.thing_shadow_properties, cloud.raw.thing_model, and local.scan are accessed as currentReport?.cloud?.assembled?.specifications ?? null (and similar) to simplify and improve readability while preserving the same null/undefined fallback behavior; update createBaseReport (and keep REPORT_SCHEMA_VERSION) accordingly.server/services/tuya/lib/mappings/index.js (2)
101-122: Inner functionnormalizeLocalMappingcould be hoisted for clarity.The
normalizeLocalMappingfunction defined insidegetLocalMappingis a pure utility function that doesn't depend on closure variables. Hoisting it to module scope would improve readability and allow potential reuse.This is a minor stylistic suggestion.
🤖 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 101 - 122, Hoist the inner utility function normalizeLocalMapping to module scope and have getLocalMapping call that top-level function instead of declaring it inline; specifically, extract the current normalizeLocalMapping(mapping) logic (creating strict, codeAliases, dps, ignoredDps) into a module-level function named normalizeLocalMapping, then remove the inner declaration inside getLocalMapping and replace its uses (including when returning normalizeLocalMapping(globalLocal) and normalizeLocalMapping(definition.LOCAL_MAPPINGS)); keep references to globalLocal, DEVICE_TYPES.UNKNOWN, DEVICE_TYPE_INDEX, and normalizeCode unchanged so behavior is identical.
70-88: Clarify the logic for matching device types whenrequiredCodesis empty.On Line 82, when
requiredCodes.size === 0,hasRequiredCodebecomestrue, which means any device matching keywords will be typed even without code verification. This seems intentional for devices where codes might not be available, but the logic flow is not immediately obvious.Consider adding a brief comment to clarify this behavior for future maintainers.
📝 Suggested clarification comment
const matchDeviceType = (typeDefinition, context) => { const { category, productId, modelName, codes } = context; if (category && typeDefinition.CATEGORIES.has(category)) { return true; } if (productId && typeDefinition.PRODUCT_IDS.has(productId)) { return true; } const requiredCodes = typeDefinition.REQUIRED_CODES; const keywords = typeDefinition.KEYWORDS; + // If no required codes are defined, allow keyword matching without code verification + // This handles devices where codes may not be available during detection const hasRequiredCode = requiredCodes.size === 0 || Array.from(requiredCodes).some((code) => codes.has(code));🤖 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 70 - 88, In matchDeviceType, the logic sets hasRequiredCode to true when typeDefinition.REQUIRED_CODES is empty which intentionally allows matching by KEYWORDS alone; add a concise comment above the requiredCodes/hasRequiredCode calculation explaining that an empty REQUIRED_CODES means "no code-based restriction — match by keywords only", and mention the related symbols REQUIRED_CODES, hasRequiredCode and KEYWORDS so future maintainers understand why the code-check is skipped when REQUIRED_CODES.size === 0.front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx (2)
486-490: Check if popup is closed before setting location.After the async
checkGithubIssueExistscall, the user may have manually closed the popup. Settingpopup.locationon a closed window could cause issues in some browsers.🛡️ Check popup state before navigation
- if (popup) { + if (popup && !popup.closed) { popup.location = issueUrl; return; }🤖 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 - 490, After the async checkGithubIssueExists call, ensure the popup window is still open before setting its location: check that popup is truthy and popup.closed is false (e.g., if (popup && !popup.closed) { popup.location = issueUrl; return; }), otherwise fall back to opening a new window with window.open(issueUrl, '_blank'); update the logic around the popup/issueUrl handling in TuyaDeviceBox.jsx where popup and issueUrl are used.
706-731: Disabled anchor still has navigable href - consider usingjavascript:void(0)or preventing default.When
disableGithubIssueButtonis true, the anchor still has a validhrefpointing to the GitHub URL. Whilearia-disabledis set and the click handler likely returns early, screen readers and some browsers may still allow navigation.The current implementation sets
href="#"when disabled, which is correct. However, theonClickhandler should also explicitly prevent default when disabled to avoid page scroll to top.🛡️ Ensure default is prevented when disabled
const renderGithubIssueButton = (labelId, extraClass = '') => ( <a class={cx('btn btn-gray', { [extraClass]: !!extraClass, disabled: disableGithubIssueButton })} href={ disableGithubIssueButton ? '#' : createGithubUrl(...) } - onClick={this.handleCreateGithubIssue} + onClick={e => { + if (disableGithubIssueButton) { + e.preventDefault(); + return; + } + this.handleCreateGithubIssue(e); + }} aria-disabled={disableGithubIssueButton}🤖 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 706 - 731, The anchor rendered by renderGithubIssueButton still allows navigation when disableGithubIssueButton is true; update the click handling so that in the onClick handler (handleCreateGithubIssue) you explicitly prevent default and return early when disableGithubIssueButton is true (e.g., if (disableGithubIssueButton) { e.preventDefault(); return; }) or change the disabled href to a non-navigating value like 'javascript:void(0)'; reference renderGithubIssueButton and handleCreateGithubIssue to locate and apply the fix.front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)
199-202: Awkward error handling pattern with thrown string constant.Throwing
new Error('TUYA_START_ERROR')only to check for it in the catch block with string comparison is fragile. Consider using a more structured approach or handling the error case directly.♻️ Alternative approach without throwing
// start service const service = await this.props.httpClient.post('/api/v1/service/tuya/start'); - if (service && service.status === 'ERROR') { - throw new Error('TUYA_START_ERROR'); - } + if (service && service.status === 'ERROR') { + this.setState({ + tuyaSaveSettingsStatus: RequestStatus.Error, + tuyaConnectionError: service.error || 'Start service failed', + tuyaJustSaved: false, + tuyaJustSavedMissing: false + }); + return; + } this.setState({ tuyaSaveSettingsStatus: RequestStatus.Success, tuyaConfigured: true }); } catch (e) { - const responseMessage = - (e && e.response && e.response.data && e.response.data.message) || - (e && e.message && e.message !== 'TUYA_START_ERROR' ? e.message : null); + const responseMessage = + (e && e.response && e.response.data && e.response.data.message) || (e && e.message) || null;🤖 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 199 - 202, The current pattern throws a string-coded Error after calling this.props.httpClient.post('/api/v1/service/tuya/start') (service and TUYA_START_ERROR) which is brittle; update SetupTab.jsx to handle the error case directly instead of throwing a string—either return/handle the failure path when service.status === 'ERROR' (e.g., set component state, call an error handler, or show a user message) or throw a structured Error object with a code property (new Error with error.code = 'TUYA_START_ERROR') so callers can reliably inspect error.code; locate the service variable and the call to this.props.httpClient.post('/api/v1/service/tuya/start') and replace the throw with one of these structured handling approaches.
🤖 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/config/i18n/de.json`:
- Around line 1288-1289: The label and placeholder keys appAccountId and
appAccountIdPlaceholder use “User ID” but should read “App Account UID” to match
surrounding UID-based keys (e.g., errorInvalidAppAccountUid); update the value
for appAccountId to the German equivalent of “App Account UID” and change
appAccountIdPlaceholder to the corresponding German placeholder text using “UID”
instead of “User ID” so both are consistent.
In `@front/src/config/i18n/en.json`:
- Around line 1196-1200: Update the awkward "with success" phrasing in the i18n
entries: replace connectedAfterSave value to use "successfully" (e.g.,
"Connected to the Tuya cloud account successfully!" or "Successfully connected
to the Tuya cloud account."), change disconnectSuccess to "Disconnected from the
Tuya cloud account successfully! Local service still works..." and similarly
update any other keys using "with success" (e.g.,
connectedStatus/disconnectedManual if present) to use natural English like
"successfully" or "Successfully connected/disconnected" while preserving the
existing HTML link and punctuation.
In `@front/src/routes/integration/all/tuya/commons/githubIssue.js`:
- Around line 277-305: checkGithubIssueExists currently throws on fetch/response
failures which can break reporting; make the lookup best-effort: catch any
fetch/JSON/response errors (including non-ok responses and AbortError), log the
error, set exists = false (treat as "not found"), write that result into the
githubIssueCache via setGithubIssueCache(query, { exists, timestamp: Date.now()
}) and return false instead of throwing; keep the existing
timeout/AbortController logic and only change the error path in
checkGithubIssueExists so failures degrade gracefully.
In `@server/services/tuya/index.js`:
- Around line 12-15: The stop() flow can still allow an in-flight runAttempt to
schedule new reconnect timeouts after shutdown; add a cancellation token/flag
(e.g., quickReconnectCancelToken) that runAttempt and any async reconnect helper
check before scheduling or pushing into quickReconnectTimeouts, set that token
in stop() and clear it when starting fresh, and ensure stop() also iterates
quickReconnectTimeouts to clearTimeout and sets quickReconnectInProgress=false
and reconnectInterval=null to prevent any post-stop reconnections; update
runAttempt, the quick reconnect scheduler, and any place that pushes to
quickReconnectTimeouts to bail out if the token is set so no new timers are
created after stop().
In `@server/services/tuya/lib/device/tuya.convertDevice.js`:
- Around line 148-150: The poll_frequency is currently set to
DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS whenever normalizedLocalOverride is
truthy, but that should only happen if the device actually has the necessary
local-mode prerequisites (IP, localKey, and protocol). Change the poll_frequency
assignment (the ternary that uses normalizedLocalOverride) so it only returns
EVERY_10_SECONDS when normalizedLocalOverride is true AND the device info
includes ip, localKey, and protocol (e.g., check the same properties used
elsewhere in this module for local connection); otherwise fall back to
EVERY_30_SECONDS. Ensure you reference normalizedLocalOverride, poll_frequency,
and DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS when making the condition change.
In `@server/services/tuya/lib/tuya.connect.js`:
- Around line 60-63: The API path is interpolating raw appAccountId which can
contain spaces or reserved characters; before building the path in the
connector.request call (the path using API.PUBLIC_VERSION_1_0 and
users/${appAccountId}/devices), trim and URI-encode the appAccountId (e.g.,
apply .trim() and encodeURIComponent) and use that encoded value in the
interpolated path so requests are safe for all valid UIDs.
In `@server/services/tuya/lib/tuya.poll.js`:
- Around line 73-79: The equality check in emitFeatureState currently uses
previousValue !== transformedValue which misclassifies NaN as changed; update
the comparison to use Object.is(previousValue, transformedValue) (invert the
logic to set changed = !Object.is(...)) so NaN is treated as equal to NaN and
transient NaN reads aren’t repeatedly emitted while keeping the existing
null/undefined guard and preserving downstream validation in device.saveState().
In `@server/services/tuya/lib/utils/tuya.normalize.js`:
- Around line 2-5: The current normalization checks trimmed lowercased strings
for 'true'/'on' but doesn't handle trimmed numeric-strings like ' 1 '; update
the normalization in tuya.normalize.js to first compute a trimmed string (e.g.,
const s = typeof value === 'string' ? value.trim() : value), then check
truthiness by: return value === true || value === 1 || s === '1' || (typeof s
=== 'string' && ['true','on'].includes(s.toLowerCase())); this ensures ' 1 ' and
other trimmed numeric-strings are treated as true while preserving existing
boolean/number handling.
---
Nitpick comments:
In `@front/src/routes/integration/all/tuya/commons/deviceHelpers.js`:
- Around line 60-68: The feature code extraction is duplicated and inconsistent
between getKnownDpsKeys and getUnknownSpecificationCodes; create a single helper
(e.g., parseFeatureCode or getFeatureCodeFromExternalId) that takes
feature.external_id (tolerates null/empty), splits on ':' and returns the code
at index 2 or null, then replace the current ad-hoc parsing in getKnownDpsKeys
and getUnknownSpecificationCodes (and the similar logic around lines 99-103) to
call this helper so both functions use the identical extraction logic.
In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx`:
- Around line 199-202: The current pattern throws a string-coded Error after
calling this.props.httpClient.post('/api/v1/service/tuya/start') (service and
TUYA_START_ERROR) which is brittle; update SetupTab.jsx to handle the error case
directly instead of throwing a string—either return/handle the failure path when
service.status === 'ERROR' (e.g., set component state, call an error handler, or
show a user message) or throw a structured Error object with a code property
(new Error with error.code = 'TUYA_START_ERROR') so callers can reliably inspect
error.code; locate the service variable and the call to
this.props.httpClient.post('/api/v1/service/tuya/start') and replace the throw
with one of these structured handling approaches.
In `@front/src/routes/integration/all/tuya/TuyaDeviceBox.jsx`:
- Around line 486-490: After the async checkGithubIssueExists call, ensure the
popup window is still open before setting its location: check that popup is
truthy and popup.closed is false (e.g., if (popup && !popup.closed) {
popup.location = issueUrl; return; }), otherwise fall back to opening a new
window with window.open(issueUrl, '_blank'); update the logic around the
popup/issueUrl handling in TuyaDeviceBox.jsx where popup and issueUrl are used.
- Around line 706-731: The anchor rendered by renderGithubIssueButton still
allows navigation when disableGithubIssueButton is true; update the click
handling so that in the onClick handler (handleCreateGithubIssue) you explicitly
prevent default and return early when disableGithubIssueButton is true (e.g., if
(disableGithubIssueButton) { e.preventDefault(); return; }) or change the
disabled href to a non-navigating value like 'javascript:void(0)'; reference
renderGithubIssueButton and handleCreateGithubIssue to locate and apply the fix.
In `@server/services/tuya/lib/mappings/index.js`:
- Around line 101-122: Hoist the inner utility function normalizeLocalMapping to
module scope and have getLocalMapping call that top-level function instead of
declaring it inline; specifically, extract the current
normalizeLocalMapping(mapping) logic (creating strict, codeAliases, dps,
ignoredDps) into a module-level function named normalizeLocalMapping, then
remove the inner declaration inside getLocalMapping and replace its uses
(including when returning normalizeLocalMapping(globalLocal) and
normalizeLocalMapping(definition.LOCAL_MAPPINGS)); keep references to
globalLocal, DEVICE_TYPES.UNKNOWN, DEVICE_TYPE_INDEX, and normalizeCode
unchanged so behavior is identical.
- Around line 70-88: In matchDeviceType, the logic sets hasRequiredCode to true
when typeDefinition.REQUIRED_CODES is empty which intentionally allows matching
by KEYWORDS alone; add a concise comment above the requiredCodes/hasRequiredCode
calculation explaining that an empty REQUIRED_CODES means "no code-based
restriction — match by keywords only", and mention the related symbols
REQUIRED_CODES, hasRequiredCode and KEYWORDS so future maintainers understand
why the code-check is skipped when REQUIRED_CODES.size === 0.
In `@server/services/tuya/lib/tuya.getStatus.js`:
- Around line 11-15: The sequential awaits for fetching Gladys variables
(endpoint, accessKey, secretKey, appAccountId, manualDisconnect) should be
converted to a parallel fetch using Promise.all to reduce latency; update the
block in tuya.getStatus.js that currently calls
this.gladys.variable.getValue(GLADYS_VARIABLES.* , this.serviceId) five times to
a single Promise.all invocation that returns all five values and then assign
them to endpoint, accessKey, secretKey, appAccountId, and manualDisconnect
respectively so the rest of the function uses the same variable names.
In `@server/services/tuya/lib/tuya.setValue.js`:
- Line 47: The variable hasLocalConfig is currently set via truthy chaining;
change its assignment to produce an explicit boolean by coercing the result
(e.g., wrap the existing expression with Boolean(...) or prepend with !!) so
hasLocalConfig becomes true/false explicitly; update the assignment that
references ipAddress, localKey, protocolVersion, and localOverride (where
localOverride is compared to true) to use this boolean coercion.
In `@server/services/tuya/lib/utils/tuya.report.js`:
- Around line 30-68: The createBaseReport function contains repetitive nested
null-checks (e.g., currentReport.cloud && currentReport.cloud.assembled ?
currentReport.cloud.assembled.specifications || null : null); replace these
patterns with optional chaining and nullish coalescing so fields like
schema_version, cloud.assembled.specifications, cloud.assembled.properties,
cloud.assembled.thing_model, cloud.raw.device_list_entry,
cloud.raw.device_specification, cloud.raw.device_details,
cloud.raw.thing_shadow_properties, cloud.raw.thing_model, and local.scan are
accessed as currentReport?.cloud?.assembled?.specifications ?? null (and
similar) to simplify and improve readability while preserving the same
null/undefined fallback behavior; update createBaseReport (and keep
REPORT_SCHEMA_VERSION) accordingly.
In `@server/test/services/tuya/fixtures/fixtureHelper.js`:
- Around line 85-91: normalizeEvents currently assumes each stubbed call has an
args array with at least two items and directly accesses call.args[1], which can
throw confusing errors when tests misconfigure stubs; update normalizeEvents to
defensively validate call.args and that call.args.length > 1 (or that
call.args[1] exists) before mapping, and either skip malformed entries or throw
a clearer error mentioning normalizeEvents and the offending call, so tests fail
with an actionable message; reference the normalizeEvents function and the calls
parameter when making the change.
In `@server/test/services/tuya/index.test.js`:
- Around line 169-195: Add a regression test that simulates an in-flight
reconnect where tuyaService.device.getStatus resolves after tuyaService.stop()
and could schedule a new timeout; specifically, stub global.setTimeout and
global.clearTimeout, arrange tuyaService.device.getStatus to be a promise you
resolve after calling tuyaService.stop(), trigger the reconnect flow via
tuyaService.start() and intervalCallback(), then resolve the delayed getStatus
and assert that setTimeout was not left active (or that clearTimeout was called
with the scheduled id) and no extra reconnect occurs; reference the existing
test scaffolding (tuyaService.start, tuyaService.stop,
tuyaService.device.getStatus, intervalCallback, and the setTimeout/clearTimeout
stubs) so the new test mirrors the current "should clear pending quick reconnect
timeouts" structure but injects the delayed resolution to cover the async
window.
In `@server/test/services/tuya/lib/device/tuya.convertDevice.fixtures.test.js`:
- Around line 44-49: The test mutates input.thing_model services by stripping
codes via the filterEntries mapping but leaves input.thing_model_raw unchanged,
making the fixture inconsistent; update the fixture mutation to apply the same
transformation to input.thing_model_raw (when it exists and has services) using
the same logic used for input.thing_model (i.e., map services and set properties
= filterEntries(service && service.properties)) so both thing_model and
thing_model_raw are kept in sync for the “feature removed” test.
In `@server/test/services/tuya/lib/tuya.localScan.test.js`:
- Around line 55-68: Wrap each test's fake-timer usage in a try/finally so
clocks are always restored: when you call sinon.useFakeTimers() and assign to
clock (as seen around localScan(1), clock.tickAsync(1100) and clock.restore()),
move the clock.restore() into a finally block that always runs after awaiting
the promise/result; apply the same change to the other occurrences (around the
blocks at lines referenced in the review) to prevent timer leakage between
tests.
In `@server/test/services/tuya/lib/tuya.saveConfiguration.test.js`:
- Around line 27-48: Add a second unit test for tuyaHandler.saveConfiguration
that covers the case where appUsername is empty or omitted: call
saveConfiguration with configuration missing or with appUsername set to '' and
assert the returned config and that gladys.variable.setValue is called for
GLADYS_VARIABLES.APP_USERNAME with an empty string (''), plus the other expected
variable sets (ENDPOINT, ACCESS_KEY, SECRET_KEY, APP_ACCOUNT_UID,
MANUAL_DISCONNECT, LAST_CONNECTED_CONFIG_HASH); ensure the test name and
assertions mirror the existing "should save configuration" test but expect
APP_USERNAME to be cleared to '' to prevent stale config regressions.
In `@server/test/services/tuya/lib/tuya.setValue.fixtures.test.js`:
- Around line 47-53: The fixture may yield undefined for currentFeature when
using currentDevice.features.find(...) with featureExternalId; add an explicit
guard/assert (e.g., assert or expect) right after obtaining currentFeature to
fail fast with a clear message if featureExternalId is missing, before calling
setValue.call(ctx, currentDevice, currentFeature, inputValue), so tests show a
descriptive fixture error instead of a downstream failure in setValue.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
server/services/tuya/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (98)
front/src/config/i18n/de.jsonfront/src/config/i18n/en.jsonfront/src/config/i18n/fr.jsonfront/src/routes/integration/all/tuya/TuyaDeviceBox.jsxfront/src/routes/integration/all/tuya/commons/deviceHelpers.jsfront/src/routes/integration/all/tuya/commons/githubIssue.jsfront/src/routes/integration/all/tuya/discover-page/DiscoverTab.jsxfront/src/routes/integration/all/tuya/discover-page/style.cssfront/src/routes/integration/all/tuya/setup-page/SetupTab.jsxserver/services/tuya/api/tuya.controller.jsserver/services/tuya/index.jsserver/services/tuya/lib/device/tuya.convertDevice.jsserver/services/tuya/lib/device/tuya.convertFeature.jsserver/services/tuya/lib/device/tuya.deviceMapping.jsserver/services/tuya/lib/device/tuya.localMapping.jsserver/services/tuya/lib/index.jsserver/services/tuya/lib/mappings/cloud/global.jsserver/services/tuya/lib/mappings/cloud/smart-meter.jsserver/services/tuya/lib/mappings/cloud/smart-socket.jsserver/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/mappings/local/global.jsserver/services/tuya/lib/mappings/local/smart-meter.jsserver/services/tuya/lib/mappings/local/smart-socket.jsserver/services/tuya/lib/tuya.connect.jsserver/services/tuya/lib/tuya.disconnect.jsserver/services/tuya/lib/tuya.discoverDevices.jsserver/services/tuya/lib/tuya.getConfiguration.jsserver/services/tuya/lib/tuya.getStatus.jsserver/services/tuya/lib/tuya.init.jsserver/services/tuya/lib/tuya.loadDeviceDetails.jsserver/services/tuya/lib/tuya.loadDevices.jsserver/services/tuya/lib/tuya.localPoll.jsserver/services/tuya/lib/tuya.localScan.jsserver/services/tuya/lib/tuya.manualDisconnect.jsserver/services/tuya/lib/tuya.poll.jsserver/services/tuya/lib/tuya.saveConfiguration.jsserver/services/tuya/lib/tuya.setValue.jsserver/services/tuya/lib/utils/tuya.config.jsserver/services/tuya/lib/utils/tuya.constants.jsserver/services/tuya/lib/utils/tuya.deviceParams.jsserver/services/tuya/lib/utils/tuya.normalize.jsserver/services/tuya/lib/utils/tuya.report.jsserver/services/tuya/package.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/cloud-status.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-device.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-events.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-local-mapping.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/input-device.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/local-dps.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/manifest.jsserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/poll-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/cloud-status.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-cloud-events.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-local-events.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/expected-local-mapping.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/input-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/local-dps.jsonserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/manifest.jsserver/test/services/tuya/fixtures/devices/smart-socket-b61eihfqeaexn54g/poll-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/cloud-status.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/expected-cloud-events.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/expected-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/expected-local-events.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/expected-local-mapping.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/input-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/local-dps.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/manifest.jsserver/test/services/tuya/fixtures/devices/smart-socket-basic/poll-device.jsonserver/test/services/tuya/fixtures/fixtureHelper.jsserver/test/services/tuya/index.test.jsserver/test/services/tuya/lib/controllers/tuya.controller.test.jsserver/test/services/tuya/lib/device/feature/tuya.convertFeature.test.jsserver/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.jsserver/test/services/tuya/lib/device/tuya.convertDevice.fixtures.test.jsserver/test/services/tuya/lib/device/tuya.convertDevice.test.jsserver/test/services/tuya/lib/device/tuya.localMapping.fixtures.test.jsserver/test/services/tuya/lib/device/tuya.localMapping.test.jsserver/test/services/tuya/lib/mappings/index.test.jsserver/test/services/tuya/lib/tuya.connect.test.jsserver/test/services/tuya/lib/tuya.disconnect.test.jsserver/test/services/tuya/lib/tuya.discoverDevices.test.jsserver/test/services/tuya/lib/tuya.getConfiguration.test.jsserver/test/services/tuya/lib/tuya.getStatus.test.jsserver/test/services/tuya/lib/tuya.init.test.jsserver/test/services/tuya/lib/tuya.loadDeviceDetails.test.jsserver/test/services/tuya/lib/tuya.loadDevices.test.jsserver/test/services/tuya/lib/tuya.localPoll.test.jsserver/test/services/tuya/lib/tuya.localScan.test.jsserver/test/services/tuya/lib/tuya.manualDisconnect.test.jsserver/test/services/tuya/lib/tuya.poll.fixtures.test.jsserver/test/services/tuya/lib/tuya.poll.test.jsserver/test/services/tuya/lib/tuya.saveConfiguration.test.jsserver/test/services/tuya/lib/tuya.setValue.fixtures.test.jsserver/test/services/tuya/lib/tuya.setValue.test.jsserver/test/services/tuya/lib/utils/tuya.deviceParams.test.jsserver/test/services/tuya/tuya.mock.test.jsserver/utils/constants.js
Comment on lines
+1288
to
+1289
| "appAccountId": "App Account User ID", | ||
| "appAccountIdPlaceholder": "Tuya App User ID eingeben", |
There was a problem hiding this comment.
Use “App Account UID” consistently in the label and placeholder.
Line 1288/Line 1289 use “User ID”, while nearby keys/messages are UID-based (errorInvalidAppAccountUid, setup descriptions). This can confuse users.
✏️ Suggested wording adjustment
- "appAccountId": "App Account User ID",
- "appAccountIdPlaceholder": "Tuya App User ID eingeben",
+ "appAccountId": "App Account UID",
+ "appAccountIdPlaceholder": "Tuya App Account UID eingeben",📝 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
| "appAccountId": "App Account User ID", | |
| "appAccountIdPlaceholder": "Tuya App User ID eingeben", | |
| "appAccountId": "App Account UID", | |
| "appAccountIdPlaceholder": "Tuya App Account UID eingeben", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/config/i18n/de.json` around lines 1288 - 1289, The label and
placeholder keys appAccountId and appAccountIdPlaceholder use “User ID” but
should read “App Account UID” to match surrounding UID-based keys (e.g.,
errorInvalidAppAccountUid); update the value for appAccountId to the German
equivalent of “App Account UID” and change appAccountIdPlaceholder to the
corresponding German placeholder text using “UID” instead of “User ID” so both
are consistent.
Comment on lines
+1196
to
+1200
| "connectedAfterSave": "Connected to the Tuya cloud account with success!", | ||
| "connectedStatus": "Connected to the Tuya cloud account.", | ||
| "disconnectSuccess": "Disconnected from the Tuya cloud account with success! Local service still works (it will not auto-reconnect when Gladys restarts).", | ||
| "disconnectedUnexpected": "You have been disconnected from the Tuya cloud! Please check your credentials and your <a href=\"https://platform.tuya.com/cloud/products?productType=all\" target=\"_blank\" rel=\"noopener noreferrer\">Tuya IoT Core</a> trial status.", | ||
| "disconnectedManual": "Disconnected from the Tuya cloud! Local service still works (it will not auto-reconnect when Gladys restarts).", |
There was a problem hiding this comment.
Tighten English phrasing in connection status messages.
A few new messages use “with success,” which reads awkwardly in EN UI copy.
✏️ Suggested text fix
- "connectedAfterSave": "Connected to the Tuya cloud account with success!",
+ "connectedAfterSave": "Connected to the Tuya cloud account successfully!",
...
- "disconnectSuccess": "Disconnected from the Tuya cloud account with success! Local service still works (it will not auto-reconnect when Gladys restarts).",
+ "disconnectSuccess": "Disconnected from the Tuya cloud account successfully! Local service still works (it will not auto-reconnect when Gladys restarts).",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/config/i18n/en.json` around lines 1196 - 1200, Update the awkward
"with success" phrasing in the i18n entries: replace connectedAfterSave value to
use "successfully" (e.g., "Connected to the Tuya cloud account successfully!" or
"Successfully connected to the Tuya cloud account."), change disconnectSuccess
to "Disconnected from the Tuya cloud account successfully! Local service still
works..." and similarly update any other keys using "with success" (e.g.,
connectedStatus/disconnectedManual if present) to use natural English like
"successfully" or "Successfully connected/disconnected" while preserving the
existing HTML link and punctuation.
Comment on lines
+277
to
+305
| export const checkGithubIssueExists = async title => { | ||
| const query = buildGithubSearchQuery(title); | ||
| const cached = githubIssueCache.get(query); | ||
| if (cached && Date.now() - cached.timestamp < GITHUB_SEARCH_CACHE_TTL_MS) { | ||
| return cached.exists; | ||
| } | ||
|
|
||
| let response; | ||
| if (typeof AbortController === 'undefined') { | ||
| response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`); | ||
| } else { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 5000); | ||
| try { | ||
| response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`, { | ||
| signal: controller.signal | ||
| }); | ||
| } finally { | ||
| clearTimeout(timeoutId); | ||
| } | ||
| } | ||
| if (!response.ok) { | ||
| throw new Error('Github search failed'); | ||
| } | ||
| const data = await response.json(); | ||
| const exists = Boolean(data && typeof data.total_count === 'number' && data.total_count > 0); | ||
| setGithubIssueCache(query, { exists, timestamp: Date.now() }); | ||
| return exists; | ||
| }; |
There was a problem hiding this comment.
Fail open when GitHub issue lookup fails.
Line 299 throws on API failure, which can block or degrade the unsupported-device reporting flow during rate limits/network errors. This check should be best-effort and default to “not found”.
🛠️ Suggested resilient handling
export const checkGithubIssueExists = async title => {
const query = buildGithubSearchQuery(title);
const cached = githubIssueCache.get(query);
if (cached && Date.now() - cached.timestamp < GITHUB_SEARCH_CACHE_TTL_MS) {
return cached.exists;
}
- let response;
- if (typeof AbortController === 'undefined') {
- response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`);
- } else {
- const controller = new AbortController();
- const timeoutId = setTimeout(() => controller.abort(), 5000);
- try {
- response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`, {
- signal: controller.signal
- });
- } finally {
- clearTimeout(timeoutId);
- }
- }
- if (!response.ok) {
- throw new Error('Github search failed');
- }
- const data = await response.json();
- const exists = Boolean(data && typeof data.total_count === 'number' && data.total_count > 0);
- setGithubIssueCache(query, { exists, timestamp: Date.now() });
- return exists;
+ try {
+ let response;
+ if (typeof AbortController === 'undefined') {
+ response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`);
+ } else {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 5000);
+ try {
+ response = await fetch(`${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}`, {
+ signal: controller.signal
+ });
+ } finally {
+ clearTimeout(timeoutId);
+ }
+ }
+ if (!response.ok) {
+ setGithubIssueCache(query, { exists: false, timestamp: Date.now() });
+ return false;
+ }
+ const data = await response.json();
+ const exists = Boolean(data && typeof data.total_count === 'number' && data.total_count > 0);
+ setGithubIssueCache(query, { exists, timestamp: Date.now() });
+ return exists;
+ } catch (e) {
+ setGithubIssueCache(query, { exists: false, timestamp: Date.now() });
+ return false;
+ }
};🤖 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
277 - 305, checkGithubIssueExists currently throws on fetch/response failures
which can break reporting; make the lookup best-effort: catch any
fetch/JSON/response errors (including non-ok responses and AbortError), log the
error, set exists = false (treat as "not found"), write that result into the
githubIssueCache via setGithubIssueCache(query, { exists, timestamp: Date.now()
}) and return false instead of throwing; keep the existing
timeout/AbortController logic and only change the error path in
checkGithubIssueExists so failures degrade gracefully.
Comment on lines
+12
to
+15
| let reconnectInterval = null; | ||
| let quickReconnectTimeouts = []; | ||
| let quickReconnectInProgress = false; | ||
|
|
There was a problem hiding this comment.
Prevent post-stop reconnects by canceling in-flight quick reconnect runs.
stop() clears existing timers, but an already-running runAttempt can still resume and schedule a new timeout afterward. That can restart reconnect attempts after service shutdown.
🔧 Proposed fix (run token cancellation + final cleanup)
let reconnectInterval = null;
let quickReconnectTimeouts = [];
let quickReconnectInProgress = false;
+ let quickReconnectRunId = 0;
@@
function clearQuickReconnects() {
+ quickReconnectRunId += 1;
if (quickReconnectTimeouts.length > 0) {
quickReconnectTimeouts.forEach((timeoutId) => clearTimeout(timeoutId));
quickReconnectTimeouts = [];
}
quickReconnectInProgress = false;
@@
function scheduleQuickReconnects() {
if (quickReconnectInProgress) {
return Promise.resolve();
}
quickReconnectInProgress = true;
+ const runId = quickReconnectRunId;
let attempts = 0;
const runAttempt = async () => {
+ if (runId !== quickReconnectRunId) {
+ return;
+ }
attempts += 1;
const shouldRetry = await tryReconnect();
+ if (runId !== quickReconnectRunId) {
+ return;
+ }
const isConnecting =
tuyaHandler.status === STATUS.CONNECTED ||
tuyaHandler.status === STATUS.CONNECTING ||
tuyaHandler.status === STATUS.DISCOVERING_DEVICES;
@@
if (attempts < QUICK_RECONNECT_ATTEMPTS) {
const timeoutId = setTimeout(runAttempt, QUICK_RECONNECT_DELAY_MS);
@@
quickReconnectTimeouts.push(timeoutId);
return;
}
- quickReconnectInProgress = false;
+ clearQuickReconnects();
};Also applies to: 67-100, 128-135
🤖 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 12 - 15, The stop() flow can
still allow an in-flight runAttempt to schedule new reconnect timeouts after
shutdown; add a cancellation token/flag (e.g., quickReconnectCancelToken) that
runAttempt and any async reconnect helper check before scheduling or pushing
into quickReconnectTimeouts, set that token in stop() and clear it when starting
fresh, and ensure stop() also iterates quickReconnectTimeouts to clearTimeout
and sets quickReconnectInProgress=false and reconnectInterval=null to prevent
any post-stop reconnections; update runAttempt, the quick reconnect scheduler,
and any place that pushes to quickReconnectTimeouts to bail out if the token is
set so no new timers are created after stop().
Comment on lines
+148
to
+150
| poll_frequency: normalizedLocalOverride | ||
| ? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS | ||
| : DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS, |
There was a problem hiding this comment.
Avoid forcing 10s polling when local mode prerequisites are incomplete.
Line 148 currently enables fast polling from local_override alone, but local polling also requires IP/localKey/protocol. Otherwise, this increases cloud fallback churn.
🔧 Proposed fix
const online = tuyaDevice.online !== undefined ? tuyaDevice.online : tuyaDevice.is_online;
const normalizedLocalOverride = normalizeBoolean(localOverride);
+ const hasCompleteLocalConfig = Boolean(localKey && ip && protocolVersion);
@@
- poll_frequency: normalizedLocalOverride
+ poll_frequency: normalizedLocalOverride && hasCompleteLocalConfig
? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS
: DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS,📝 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
| poll_frequency: normalizedLocalOverride | |
| ? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS | |
| : DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS, | |
| const online = tuyaDevice.online !== undefined ? tuyaDevice.online : tuyaDevice.is_online; | |
| const normalizedLocalOverride = normalizeBoolean(localOverride); | |
| const hasCompleteLocalConfig = Boolean(localKey && ip && protocolVersion); | |
| poll_frequency: normalizedLocalOverride && hasCompleteLocalConfig | |
| ? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS | |
| : DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/tuya/lib/device/tuya.convertDevice.js` around lines 148 -
150, The poll_frequency is currently set to
DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS whenever normalizedLocalOverride is
truthy, but that should only happen if the device actually has the necessary
local-mode prerequisites (IP, localKey, and protocol). Change the poll_frequency
assignment (the ternary that uses normalizedLocalOverride) so it only returns
EVERY_10_SECONDS when normalizedLocalOverride is true AND the device info
includes ip, localKey, and protocol (e.g., check the same properties used
elsewhere in this module for local connection); otherwise fall back to
EVERY_30_SECONDS. Ensure you reference normalizedLocalOverride, poll_frequency,
and DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS when making the condition change.
Comment on lines
+60
to
+63
| const response = await connector.request({ | ||
| method: 'GET', | ||
| path: `${API.PUBLIC_VERSION_1_0}/users/${appAccountId}/devices`, | ||
| query: { |
There was a problem hiding this comment.
Encode appAccountId before interpolating it into the API path.
Using raw user-provided UID in the URL path can break requests for values containing reserved characters or leading/trailing whitespace.
🔧 Proposed fix
const validateAppAccount = async (connector, appAccountId) => {
@@
+ const encodedAppAccountId = encodeURIComponent(String(appAccountId).trim());
const response = await connector.request({
method: 'GET',
- path: `${API.PUBLIC_VERSION_1_0}/users/${appAccountId}/devices`,
+ path: `${API.PUBLIC_VERSION_1_0}/users/${encodedAppAccountId}/devices`,
query: {
page_no: 1,
page_size: 1,🤖 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 60 - 63, The API path
is interpolating raw appAccountId which can contain spaces or reserved
characters; before building the path in the connector.request call (the path
using API.PUBLIC_VERSION_1_0 and users/${appAccountId}/devices), trim and
URI-encode the appAccountId (e.g., apply .trim() and encodeURIComponent) and use
that encoded value in the interpolated path so requests are safe for all valid
UIDs.
Comment on lines
+73
to
+79
| const emitFeatureState = (gladys, deviceFeature, transformedValue, previousValue, previousValueChangedAt) => { | ||
| if (transformedValue === null || transformedValue === undefined) { | ||
| return { emitted: false, changed: false }; | ||
| } | ||
|
|
||
| const changed = previousValue !== transformedValue; | ||
| let emitted = changed; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Examine emitFeatureState guard logic (lines 73-97) ==="
sed -n '73,97p' server/services/tuya/lib/tuya.poll.js | cat -n
echo ""
echo "=== Check how transformedValue arrives at emitFeatureState ==="
rg -n "emitFeatureState\(" server/services/tuya/lib/tuya.poll.js -A 1 -B 1
echo ""
echo "=== Verify scaleValue NaN behavior in deviceMapping ==="
sed -n '19,26p' server/services/tuya/lib/device/tuya.deviceMapping.js | cat -n
echo ""
echo "=== Check test for NaN handling in switch power ==="
rg -n "switch power invalid value returns NaN" server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js -A 10 -B 2
echo ""
echo "=== Check device.saveState behavior with NaN ==="
rg -n "should not save NaN as state" server/test/lib/device/device.saveState.test.js -A 15 -B 2Repository: GladysAssistant/Gladys
Length of output: 3319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Complete device.saveState NaN test ==="
rg -n "should not save NaN as state" server/test/lib/device/device.saveState.test.js -A 30
echo ""
echo "=== Check if NaN values are filtered elsewhere in tuya.poll.js ==="
rg -n "Number\.isNaN|NaN" server/services/tuya/lib/tuya.poll.js
echo ""
echo "=== Verify transformedValue origin and transformation ==="
sed -n '150,170p' server/services/tuya/lib/tuya.poll.js | cat -n
echo ""
echo "=== Check if Object.is is used anywhere for NaN handling in codebase ==="
rg -n "Object\.is" server/services/tuya/ --type jsRepository: GladysAssistant/Gladys
Length of output: 2130
Use Object.is() for value comparison to handle NaN equality correctly.
On Line 78, previousValue !== transformedValue will always be true for NaN values (since NaN !== NaN), causing the state to be marked as changed and re-emitted on every poll cycle. This wastes event bandwidth for invalid numeric reads.
Replace the comparison with Object.is(previousValue, transformedValue), which correctly treats NaN === NaN as equal. The existing guard at line 74-76 correctly preserves NaN values to keep invalid payloads visibly invalid; downstream validation in device.saveState() already rejects NaN from persistence, so filtering here is unnecessary and would hide data quality issues.
🔧 Proposed fix
const changed = !Object.is(previousValue, transformedValue);📝 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 emitFeatureState = (gladys, deviceFeature, transformedValue, previousValue, previousValueChangedAt) => { | |
| if (transformedValue === null || transformedValue === undefined) { | |
| return { emitted: false, changed: false }; | |
| } | |
| const changed = previousValue !== transformedValue; | |
| let emitted = changed; | |
| const emitFeatureState = (gladys, deviceFeature, transformedValue, previousValue, previousValueChangedAt) => { | |
| if (transformedValue === null || transformedValue === undefined) { | |
| return { emitted: false, changed: false }; | |
| } | |
| const changed = !Object.is(previousValue, transformedValue); | |
| let emitted = changed; |
🤖 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 73 - 79, The equality
check in emitFeatureState currently uses previousValue !== transformedValue
which misclassifies NaN as changed; update the comparison to use
Object.is(previousValue, transformedValue) (invert the logic to set changed =
!Object.is(...)) so NaN is treated as equal to NaN and transient NaN reads
aren’t repeatedly emitted while keeping the existing null/undefined guard and
preserving downstream validation in device.saveState().
Comment on lines
+2
to
+5
| if (value === true || value === 1 || value === '1') { | ||
| return true; | ||
| } | ||
| return typeof value === 'string' && ['true', 'on'].includes(value.trim().toLowerCase()); |
There was a problem hiding this comment.
Handle trimmed numeric-string truthy values consistently.
At Line 5, strings are trimmed before checking 'true'/'on', but ' 1 ' still resolves to false. This creates an inconsistent normalization path for boolean-like string input.
Proposed fix
const normalizeBoolean = (value) => {
- if (value === true || value === 1 || value === '1') {
+ if (value === true || value === 1) {
return true;
}
- return typeof value === 'string' && ['true', 'on'].includes(value.trim().toLowerCase());
+ if (typeof value === 'string') {
+ const normalized = value.trim().toLowerCase();
+ return ['1', 'true', 'on'].includes(normalized);
+ }
+ return false;
};📝 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
| if (value === true || value === 1 || value === '1') { | |
| return true; | |
| } | |
| return typeof value === 'string' && ['true', 'on'].includes(value.trim().toLowerCase()); | |
| const normalizeBoolean = (value) => { | |
| if (value === true || value === 1) { | |
| return true; | |
| } | |
| if (typeof value === 'string') { | |
| const normalized = value.trim().toLowerCase(); | |
| return ['1', 'true', 'on'].includes(normalized); | |
| } | |
| return false; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/tuya/lib/utils/tuya.normalize.js` around lines 2 - 5, The
current normalization checks trimmed lowercased strings for 'true'/'on' but
doesn't handle trimmed numeric-strings like ' 1 '; update the normalization in
tuya.normalize.js to first compute a trimmed string (e.g., const s = typeof
value === 'string' ? value.trim() : value), then check truthiness by: return
value === true || value === 1 || s === '1' || (typeof s === 'string' &&
['true','on'].includes(s.toLowerCase())); this ensures ' 1 ' and other trimmed
numeric-strings are treated as true while preserving existing boolean/number
handling.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
server/services/tuya/lib/device/tuya.convertDevice.js (1)
153-155:⚠️ Potential issue | 🟠 MajorAvoid forcing 10s polling when local prerequisites are incomplete.
LOCAL_OVERRIDEalone is not enough for local mode; polling at 10s should only be enabled whenip,localKey, andprotocolVersionare also present.🔧 Proposed fix
const online = tuyaDevice.online !== undefined ? tuyaDevice.online : tuyaDevice.is_online; const normalizedLocalOverride = normalizeBoolean(localOverride); + const hasCompleteLocalConfig = Boolean(ip && localKey && protocolVersion); @@ - poll_frequency: normalizedLocalOverride + poll_frequency: normalizedLocalOverride && hasCompleteLocalConfig ? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS : DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/device/tuya.convertDevice.js` around lines 153 - 155, The poll_frequency currently uses normalizedLocalOverride alone to pick DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS; change the condition so 10s polling is only selected when normalizedLocalOverride is true AND the device has local prerequisites (ip, localKey, and protocolVersion present). Update the poll_frequency ternary in tuya.convertDevice.js to check all four: normalizedLocalOverride && device.ip && device.localKey && device.protocolVersion (or their local variable equivalents) before choosing EVERY_10_SECONDS, otherwise use EVERY_30_SECONDS.server/services/tuya/lib/tuya.poll.js (1)
79-79:⚠️ Potential issue | 🟠 MajorUse
Object.isfor change detection to handleNaNcorrectly.Current comparison treats
NaNas always changed, which can repeatedly emit identical invalid states on each poll cycle.🔧 Proposed fix
- const changed = previousValue !== transformedValue; + const changed = !Object.is(previousValue, transformedValue);#!/bin/bash set -euo pipefail echo "=== Current change-detection line ===" rg -nP --type=js -C2 'const\s+changed\s*=' server/services/tuya/lib/tuya.poll.js echo echo "=== Existing NaN behavior tests referenced by Tuya mapping/state persistence ===" rg -nP --type=js -C2 'switch power invalid value returns NaN|should not save NaN as state' \ server/test/services/tuya/lib/device/feature/tuya.deviceMapping.test.js \ server/test/lib/device/device.saveState.test.jsBased on learnings: In
server/services/tuya/lib/device/tuya.deviceMapping.js, invalid numeric payloads intentionally surface asNaN, and downstream persistence explicitly blocks savingNaN.🤖 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` at line 79, The current change detection uses a strict inequality (const changed = previousValue !== transformedValue) which treats NaN as always changed; update the comparison in tuya.poll.js to use Object.is(previousValue, transformedValue) and set changed to the negation of that (i.e., changed = !Object.is(previousValue, transformedValue)) so NaN vs NaN is considered equal and identical invalid states are not repeatedly emitted; ensure you update the expression where changed is declared and used so downstream logic that relies on the changed flag (previousValue, transformedValue) continues to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/services/tuya/lib/device/tuya.convertDevice.js`:
- Around line 153-155: The poll_frequency currently uses normalizedLocalOverride
alone to pick DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS; change the condition so
10s polling is only selected when normalizedLocalOverride is true AND the device
has local prerequisites (ip, localKey, and protocolVersion present). Update the
poll_frequency ternary in tuya.convertDevice.js to check all four:
normalizedLocalOverride && device.ip && device.localKey &&
device.protocolVersion (or their local variable equivalents) before choosing
EVERY_10_SECONDS, otherwise use EVERY_30_SECONDS.
In `@server/services/tuya/lib/tuya.poll.js`:
- Line 79: The current change detection uses a strict inequality (const changed
= previousValue !== transformedValue) which treats NaN as always changed; update
the comparison in tuya.poll.js to use Object.is(previousValue, transformedValue)
and set changed to the negation of that (i.e., changed =
!Object.is(previousValue, transformedValue)) so NaN vs NaN is considered equal
and identical invalid states are not repeatedly emitted; ensure you update the
expression where changed is declared and used so downstream logic that relies on
the changed flag (previousValue, transformedValue) continues to work.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
server/services/tuya/lib/device/tuya.convertDevice.jsserver/services/tuya/lib/tuya.poll.jsserver/services/tuya/lib/utils/tuya.cloudStrategy.jsserver/services/tuya/lib/utils/tuya.constants.jsserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/cloud-status.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-device.jsonserver/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/poll-device.jsonserver/test/services/tuya/fixtures/devices/smart-socket-basic/expected-device.jsonserver/test/services/tuya/lib/device/tuya.convertDevice.test.jsserver/test/services/tuya/lib/tuya.discoverDevices.test.jsserver/test/services/tuya/lib/tuya.poll.test.js
✅ Files skipped from review due to trivial changes (1)
- server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/cloud-status.json
🚧 Files skipped from review as they are similar to previous changes (2)
- server/test/services/tuya/lib/device/tuya.convertDevice.test.js
- server/test/services/tuya/fixtures/devices/smart-meter-bbcg1hrkrj5rifsd/expected-device.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tuya.localPoll.js`:
- Around line 167-168: The code currently unconditionally assigns
device.protocol_version and device.ip from the incoming payload, which can erase
existing values when the payload is partial; change the assignments to only set
these fields when the incoming values are present (e.g., check protocolVersion
!== undefined / ip !== undefined or use nullish checks) so existing
device.protocol_version and device.ip are preserved when the payload omits them.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/services/tuya/lib/tuya.localPoll.jsserver/test/services/tuya/lib/tuya.localPoll.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- server/test/services/tuya/lib/tuya.localPoll.test.js
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
front/src/config/i18n/en.json (1)
1198-1200:⚠️ Potential issue | 🟡 MinorKeep the cloud success messages idiomatic in English.
"with success"still reads awkwardly in UI copy here.successfullyis the natural phrasing.✏️ Proposed text fix
- "connectedAfterSave": "Connected to the Tuya cloud account with success!", + "connectedAfterSave": "Connected to the Tuya cloud account successfully!", ... - "disconnectSuccess": "Disconnected from the Tuya cloud account with success! Local service still works (it will not auto-reconnect when Gladys restarts).", + "disconnectSuccess": "Disconnected from the Tuya cloud account successfully! Local service still works (it will not auto-reconnect when Gladys restarts).",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/config/i18n/en.json` around lines 1198 - 1200, The UI copy uses "with success" which is non-idiomatic; update the Tuya i18n entries connectedAfterSave and disconnectSuccess to use "successfully" instead of "with success" (e.g., change "Connected to the Tuya cloud account with success!" and "Disconnected from the Tuya cloud account with success! Local service still works…" to use "successfully"), leaving connectedStatus unchanged.server/services/tuya/lib/device/tuya.convertDevice.js (1)
179-181:⚠️ Potential issue | 🟠 MajorKeep 10s polling behind complete local config.
local_overridealone does not mean local polling can actually work. Withoutlocal_key,ip, andprotocol_version, this still pushes the device to 10-second polling while it can only fall back to cloud.🔧 Suggested fix
- poll_frequency: normalizedLocalOverride + poll_frequency: normalizedLocalOverride && Boolean(localKey && ip && protocolVersion) ? DEVICE_POLL_FREQUENCIES.EVERY_10_SECONDS : DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/tuya/lib/device/tuya.convertDevice.js` around lines 179 - 181, The poll_frequency logic currently uses normalizedLocalOverride to decide 10s polling but must require a full working local config; change the condition that sets poll_frequency to EVERY_10_SECONDS so it only returns 10s when normalizedLocalOverride is true AND the device has local_key, ip, and protocol_version present/valid (check the same fields used elsewhere in tuya.device handling), otherwise fall back to DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS; update the condition around poll_frequency (referencing normalizedLocalOverride, DEVICE_POLL_FREQUENCIES, and the device properties local_key, ip, protocol_version) accordingly.front/src/config/i18n/de.json (1)
1290-1291:⚠️ Potential issue | 🟡 MinorUse “App Account UID” consistently here.
These two strings still say “User ID”, while the surrounding Tuya setup copy and
errorInvalidAppAccountUiduse UID. Keeping the label and placeholder aligned avoids user confusion.✏️ Suggested wording
- "appAccountId": "App Account User ID", - "appAccountIdPlaceholder": "Tuya App User ID eingeben", + "appAccountId": "App Account UID", + "appAccountIdPlaceholder": "Tuya App Account UID eingeben",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/config/i18n/de.json` around lines 1290 - 1291, The label and placeholder values for the Tuya app account are inconsistent (they use "User ID" while other keys use "UID"); update the JSON entries "appAccountId" and "appAccountIdPlaceholder" to replace "User ID" with "UID" so the label reads "App Account UID" and the placeholder uses the same UID terminology (and adjust language/wording to match surrounding strings if needed).
🧹 Nitpick comments (2)
server/test/services/tuya/lib/tuya.localPoll.test.js (1)
488-503: Assert thestateManager.getcontract instead of returning blindly.This stub returns a device for any call, so the test still passes if
updateDiscoveredDeviceAfterLocalPoll()queries the wrong store key or forgets the external id. Tighten it withcalledOnceWithExactly('deviceByExternalId', 'tuya:device1')or acallsFakeassertion so the test actually covers the production integration point.Suggested patch
gladys: { stateManager: { - get: sinon.stub().returns({ - external_id: 'tuya:device1', - params: [{ name: DEVICE_PARAM_NAME.LOCAL_OVERRIDE, value: '1' }], - features: [], - }), + get: sinon.stub().callsFake((key, externalId) => { + expect(key).to.equal('deviceByExternalId'); + expect(externalId).to.equal('tuya:device1'); + return { + external_id: 'tuya:device1', + params: [{ name: DEVICE_PARAM_NAME.LOCAL_OVERRIDE, value: '1' }], + features: [], + }; + }), }, }, @@ expect(updated).to.have.property('updatable'); expect(updated.local_override).to.equal(true); + expect(tuyaManager.gladys.stateManager.get.calledOnce).to.equal(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.localPoll.test.js` around lines 488 - 503, The stateManager.get stub in the test is too permissive; tighten it so the test asserts the correct store key is used by updateDiscoveredDeviceAfterLocalPoll. Replace the current sinon.stub().returns(...) with either a stub that asserts its call like sinon.stub().calledOnceWithExactly('deviceByExternalId','tuya:device1') after invoking updateDiscoveredDeviceAfterLocalPoll, or use sinon.stub().callsFake((key, id) => { expect(key).to.equal('deviceByExternalId'); expect(id).to.equal('tuya:device1'); return { external_id: 'tuya:device1', params: [{ name: DEVICE_PARAM_NAME.LOCAL_OVERRIDE, value: '1' }], features: [] }; }); and keep the rest of the test unchanged to ensure the production lookup contract is validated.front/src/config/i18n/fr.json (1)
1398-1431: StandardizeUIDvsUser IDacross the Tuya setup copy.These adjacent strings appear to describe the same field, but the wording alternates between
App account UIDandApp account User ID. Pick one term and use it everywhere in this block to avoid copy/paste mistakes during setup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/config/i18n/fr.json` around lines 1398 - 1431, The French copy mixes "App account UID" and "App account User ID" for the same field; standardize to a single term (pick "App account UID") across this block by updating the strings referenced by descriptionGetAppAccountUid, descriptionGetAppAccountUid2, appAccountId, appAccountIdPlaceholder and errorInvalidAppAccountUid so they consistently use "App account UID" (and adjust any French wording around it if needed).
🤖 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/config/i18n/fr.json`:
- Line 1357: Revert the value of the JSON key
integration.tuya.device.localPollRequired to the established French wording
"Lancez une lecture locale des DP..." (replace the current longer sentence) and,
if you want to keep the extra guidance, move that additional text into the
integration.tuya.device.localPollHelp entry instead so the standardized message
remains unchanged.
In `@front/src/routes/integration/all/tuya/commons/deviceHelpers.js`:
- Around line 138-151: The code currently marks every code from
device.thing_model.services and device.properties as known by calling
addKnownCode unconditionally; change both loops to only call addKnownCode when
the code corresponds to a mapped/supported feature (i.e., run a predicate like
hasMappedFeature(code) or lookup in the existing feature map before calling
addKnownCode). Update the service.properties iteration and the
propertiesPayload/properties iteration to fetch property.code into a local var
and only invoke addKnownCode(code) if the mapping lookup (e.g., featureMap[code]
or getFeatureForCode(code)) returns a valid mapping.
In `@front/src/routes/integration/all/tuya/discover-page/githubIssue.js`:
- Around line 304-311: The search query builder currently wraps raw title in
quotes in buildGithubSearchQuery (and is used by buildGithubSearchUrl and
buildGithubSearchApiUrl) without escaping internal quotes; update
buildGithubSearchQuery to escape any double quotes (and optionally backslashes)
in the title before surrounding it with quotes so phrase searches remain valid
(e.g., replace " with \" in the title), leaving the rest of the callers
(buildGithubSearchUrl/buildGithubSearchApiUrl) unchanged so encodeURIComponent
still runs on the final query string.
In
`@front/src/routes/integration/all/tuya/discover-page/TuyaGithubIssueSection.jsx`:
- Around line 31-36: The component is vulnerable to stale async completions
repopulating state or navigating for a previous device; in
TuyaGithubIssueSection.jsx (componentWillReceiveProps and the async flows that
call checkGithubIssues and perform setState/navigation) capture the current
device.external_id into a local const at the start of each async flow (e.g.,
inside the methods that call await checkGithubIssues(...), and inside any
handlers that eventually call setState or navigation), and before every
post-await setState or navigation compare this.props.device?.external_id (or the
latest prop) to the captured external_id and bail if they differ; apply the same
guard to the other async blocks referenced (around lines 75-95, 103-149,
236-260) so stale responses are ignored.
- Around line 62-69: The click handler currently opens a popup (const popup =
window.open('about:blank', '_blank')) and then awaits checkGithubIssues(...)
before doing the fallback window.open(issueUrl, '_blank'), which loses the user
gesture if the initial open was blocked; change the flow so that if popup is
null you immediately attempt the fallback open(issueUrl, '_blank') synchronously
(still within the click handler) before any awaits, and only use the opened
window (popup) for later document updates after await; apply the same change for
the other occurrence around the second checkGithubIssues call (the block
involving the popup variable and the fallback at lines ~236-259) so both
fallbacks run inside the original user gesture.
In `@front/src/routes/integration/all/tuya/TuyaLocalPollSection.jsx`:
- Around line 136-143: The current use of "||" when computing protocolVersion,
ipAddress, cloudIp, pollProtocolLabel (and similar logic in commons/localPoll.js
and TuyaDeviceBox.jsx) collapses explicit empty strings to persisted values,
preventing users from clearing fields to trigger the "empty for auto-detect"
flow; update these computations to distinguish unset (undefined/null) from an
explicit empty string by checking for null/undefined (e.g., using ?? or explicit
!== undefined/null checks) so cleared values remain '' end-to-end, and propagate
the same change to commons/localPoll.js and TuyaDeviceBox.jsx to ensure polling
and UI respect an explicit empty value.
In `@server/services/tuya/lib/device/tuya.convertDevice.js`:
- Around line 135-147: currentProperties is only populated when properties is an
object with a properties array, so bare-array shadow payloads are ignored;
change the initialization of currentProperties to accept either a plain array or
the wrapped form (i.e., treat properties as the array when
Array.isArray(properties) is true, otherwise fall back to
Array.isArray(properties && properties.properties) ? properties.properties :
[]), leaving the rest of the loop (groups, property.code,
property.custom_name/name) unchanged.
In `@server/services/tuya/lib/tuya.localPoll.js`:
- Around line 222-239: mergeDevices overwrites device.params with
newDevice.params causing saved Gladys-only params to be lost; before calling
normalizeExistingDevice/mergeDevices, retrieve the existing device params via
tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id)
(as done with normalizeExistingDevice), then merge those existing params into
the local device.params (preserving existing entries keyed by param name) —
e.g., collect existing.params, iterate and upsert any entries not present in
device.params using the same upsertParam helper (or dedupe by DEVICE_PARAM_NAME
keys) so CLOUD_READ_STRATEGY and other persisted params are retained, then call
normalizeExistingDevice and mergeDevices as before.
---
Duplicate comments:
In `@front/src/config/i18n/de.json`:
- Around line 1290-1291: The label and placeholder values for the Tuya app
account are inconsistent (they use "User ID" while other keys use "UID"); update
the JSON entries "appAccountId" and "appAccountIdPlaceholder" to replace "User
ID" with "UID" so the label reads "App Account UID" and the placeholder uses the
same UID terminology (and adjust language/wording to match surrounding strings
if needed).
In `@front/src/config/i18n/en.json`:
- Around line 1198-1200: The UI copy uses "with success" which is non-idiomatic;
update the Tuya i18n entries connectedAfterSave and disconnectSuccess to use
"successfully" instead of "with success" (e.g., change "Connected to the Tuya
cloud account with success!" and "Disconnected from the Tuya cloud account with
success! Local service still works…" to use "successfully"), leaving
connectedStatus unchanged.
In `@server/services/tuya/lib/device/tuya.convertDevice.js`:
- Around line 179-181: The poll_frequency logic currently uses
normalizedLocalOverride to decide 10s polling but must require a full working
local config; change the condition that sets poll_frequency to EVERY_10_SECONDS
so it only returns 10s when normalizedLocalOverride is true AND the device has
local_key, ip, and protocol_version present/valid (check the same fields used
elsewhere in tuya.device handling), otherwise fall back to
DEVICE_POLL_FREQUENCIES.EVERY_30_SECONDS; update the condition around
poll_frequency (referencing normalizedLocalOverride, DEVICE_POLL_FREQUENCIES,
and the device properties local_key, ip, protocol_version) accordingly.
---
Nitpick comments:
In `@front/src/config/i18n/fr.json`:
- Around line 1398-1431: The French copy mixes "App account UID" and "App
account User ID" for the same field; standardize to a single term (pick "App
account UID") across this block by updating the strings referenced by
descriptionGetAppAccountUid, descriptionGetAppAccountUid2, appAccountId,
appAccountIdPlaceholder and errorInvalidAppAccountUid so they consistently use
"App account UID" (and adjust any French wording around it if needed).
In `@server/test/services/tuya/lib/tuya.localPoll.test.js`:
- Around line 488-503: The stateManager.get stub in the test is too permissive;
tighten it so the test asserts the correct store key is used by
updateDiscoveredDeviceAfterLocalPoll. Replace the current
sinon.stub().returns(...) with either a stub that asserts its call like
sinon.stub().calledOnceWithExactly('deviceByExternalId','tuya:device1') after
invoking updateDiscoveredDeviceAfterLocalPoll, or use
sinon.stub().callsFake((key, id) => {
expect(key).to.equal('deviceByExternalId'); expect(id).to.equal('tuya:device1');
return { external_id: 'tuya:device1', params: [{ name:
DEVICE_PARAM_NAME.LOCAL_OVERRIDE, value: '1' }], features: [] }; }); and keep
the rest of the test unchanged to ensure the production lookup contract is
validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 082f5fad-a336-4dd0-a0c6-be824bc30fad
⛔ Files ignored due to path filters (1)
server/services/tuya/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
front/src/config/i18n/de.jsonfront/src/config/i18n/en.jsonfront/src/config/i18n/fr.jsonfront/src/routes/integration/all/tuya/TuyaDeviceBox.jsxfront/src/routes/integration/all/tuya/TuyaLocalPollSection.jsxfront/src/routes/integration/all/tuya/commons/deviceHelpers.jsfront/src/routes/integration/all/tuya/commons/localPoll.jsfront/src/routes/integration/all/tuya/discover-page/TuyaGithubIssueSection.jsxfront/src/routes/integration/all/tuya/discover-page/githubIssue.jsserver/services/tuya/lib/device/tuya.convertDevice.jsserver/services/tuya/lib/mappings/index.jsserver/services/tuya/lib/tuya.localPoll.jsserver/services/tuya/package.jsonserver/test/services/tuya/lib/device/tuya.convertDevice.test.jsserver/test/services/tuya/lib/mappings/index.test.jsserver/test/services/tuya/lib/tuya.localPoll.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- server/services/tuya/package.json
- server/test/services/tuya/lib/device/tuya.convertDevice.test.js
| "localPollButton": "Lecture locale des DP", | ||
| "localPollInProgress": "Lecture locale en cours... protocole {{protocol}}.", | ||
| "localPollHelp": "Si vous connaissez le protocole, sélectionnez-le puis cliquez pour vérifier. Sinon, cliquez directement pour une analyse complète (plus longue).", | ||
| "localPollRequired": "Le mode local est activé et les paramètres locaux ont changé. Lancez une lecture locale des DP avec succès avant de sauvegarder. Si vous n'arrivez pas à faire fonctionner le local, repassez en mode cloud.", |
There was a problem hiding this comment.
Keep localPollRequired on the established FR wording.
This key was previously standardized, and the longer warning here drifts from the agreed FR copy. If you still want the extra guidance, localPollHelp is the better place for it.
✏️ Suggested copy
- "localPollRequired": "Le mode local est activé et les paramètres locaux ont changé. Lancez une lecture locale des DP avec succès avant de sauvegarder. Si vous n'arrivez pas à faire fonctionner le local, repassez en mode cloud.",
+ "localPollRequired": "Lancez une lecture locale des DP..."Based on learnings: in front/src/config/i18n/fr.json, integration.tuya.device.localPollRequired should keep the standard wording Lancez une lecture locale des DP....
📝 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
| "localPollRequired": "Le mode local est activé et les paramètres locaux ont changé. Lancez une lecture locale des DP avec succès avant de sauvegarder. Si vous n'arrivez pas à faire fonctionner le local, repassez en mode cloud.", | |
| "localPollRequired": "Lancez une lecture locale des DP...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/config/i18n/fr.json` at line 1357, Revert the value of the JSON key
integration.tuya.device.localPollRequired to the established French wording
"Lancez une lecture locale des DP..." (replace the current longer sentence) and,
if you want to keep the extra guidance, move that additional text into the
integration.tuya.device.localPollHelp entry instead so the standardized message
remains unchanged.
Comment on lines
+138
to
+151
| const services = Array.isArray(device && device.thing_model && device.thing_model.services) | ||
| ? device.thing_model.services | ||
| : []; | ||
| services.forEach(service => { | ||
| const properties = Array.isArray(service && service.properties) ? service.properties : []; | ||
| properties.forEach(property => addKnownCode(property && property.code)); | ||
| }); | ||
| const propertiesPayload = device && device.properties; | ||
| const properties = Array.isArray(propertiesPayload) | ||
| ? propertiesPayload | ||
| : Array.isArray(propertiesPayload && propertiesPayload.properties) | ||
| ? propertiesPayload.properties | ||
| : []; | ||
| properties.forEach(property => addKnownCode(property && property.code)); |
There was a problem hiding this comment.
Only mapped features should mark a specification code as known.
Adding thing_model and properties codes to knownCodes makes unknown_specification_codes disappear for unsupported devices, because every code already present in the raw cloud payload is treated as “known” before the comparison. That hides exactly the codes maintainers need to map when opening support issues.
🧰 Tools
🪛 Biome (2.4.4)
[error] 143-143: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
[error] 151-151: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 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
138 - 151, The code currently marks every code from device.thing_model.services
and device.properties as known by calling addKnownCode unconditionally; change
both loops to only call addKnownCode when the code corresponds to a
mapped/supported feature (i.e., run a predicate like hasMappedFeature(code) or
lookup in the existing feature map before calling addKnownCode). Update the
service.properties iteration and the propertiesPayload/properties iteration to
fetch property.code into a local var and only invoke addKnownCode(code) if the
mapping lookup (e.g., featureMap[code] or getFeatureForCode(code)) returns a
valid mapping.
Comment on lines
+304
to
+311
| const buildGithubSearchQuery = title => `repo:GladysAssistant/Gladys in:title "${title}"`; | ||
|
|
||
| export const buildGithubSearchUrl = title => | ||
| `${GITHUB_SEARCH_BASE_URL}${encodeURIComponent(buildGithubSearchQuery(title))}`; | ||
|
|
||
| const buildGithubSearchApiUrl = title => { | ||
| const query = buildGithubSearchQuery(title); | ||
| return `${GITHUB_SEARCH_API_URL}${encodeURIComponent(query)}&sort=created&order=desc&per_page=1`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n front/src/routes/integration/all/tuya/discover-page/githubIssue.js | head -350 | tail -100Repository: GladysAssistant/Gladys
Length of output: 5018
🏁 Script executed:
# Check where device.model comes from and if there's any validation
grep -r "device\.model" front/src/routes/integration/all/tuya --include="*.js" | head -20Repository: GladysAssistant/Gladys
Length of output: 1190
🏁 Script executed:
node << 'EOF'
// Test what encodeURIComponent does with quotes in search query
const buildGithubSearchQuery = title => `repo:GladysAssistant/Gladys in:title "${title}"`;
const testTitle = 'LSC Plug "FR"';
const query = buildGithubSearchQuery(testTitle);
console.log("Query:", query);
console.log("encodeURIComponent result:", encodeURIComponent(query));
console.log("Decoded:", decodeURIComponent(encodeURIComponent(query)));
EOFRepository: GladysAssistant/Gladys
Length of output: 289
🏁 Script executed:
node << 'EOF'
// Test GitHub API query escaping behavior
const scenarios = [
{
name: "Current (unescaped)",
query: 'repo:GladysAssistant/Gladys in:title "LSC Plug "FR""'
},
{
name: "Escaped backslash",
query: 'repo:GladysAssistant/Gladys in:title "LSC Plug \\"FR\\""'
},
{
name: "Without inner quotes",
query: 'repo:GladysAssistant/Gladys in:title LSC Plug FR'
}
];
console.log("GitHub API search query escaping analysis:");
console.log("=" .repeat(60));
scenarios.forEach(s => {
const encoded = encodeURIComponent(s.query);
console.log(`\n${s.name}:`);
console.log(` Query: ${s.query}`);
console.log(` Encoded: ${encoded}`);
console.log(` URL would be: https://api.github.com/search/issues?q=${encoded}`);
});
// Show the specific problem
console.log("\n" + "=".repeat(60));
console.log("\nProblem analysis:");
console.log("In GitHub's quoted phrase search syntax:");
console.log(' "text with "quotes"" - the inner quotes terminate the phrase');
console.log(" This makes the query malformed and may cause:");
console.log(" - Different search interpretation");
console.log(" - Query syntax errors");
console.log(" - Missing results for duplicate detection");
EOFRepository: GladysAssistant/Gladys
Length of output: 1391
🏁 Script executed:
# Search for escaping functions or quote handling
grep -r "escape\|sanitize" front/src/routes/integration/all/tuya/discover-page/ --include="*.js" | grep -v node_modulesRepository: GladysAssistant/Gladys
Length of output: 1050
🏁 Script executed:
grep -n "const escapeJsString\|const sanitizeIssueValue\|const sanitizeParams" front/src/routes/integration/all/tuya/discover-page/githubIssue.js -A 10Repository: GladysAssistant/Gladys
Length of output: 1263
🏁 Script executed:
# Search for test data or examples of device models
grep -r "product_name\|device.*model\|thing_model" front/src/routes/integration/all/tuya --include="*.js" | grep -E "(test|fixture|example|mock|=\s*['\"])" | head -20Repository: GladysAssistant/Gladys
Length of output: 48
🏁 Script executed:
node << 'EOF'
// Test to understand GitHub API phrase search escaping
// GitHub API documentation states that quoted phrase searches should escape inner quotes with backslash
console.log("GitHub Search API - Phrase Search Escaping Rules:");
console.log("=" .repeat(70));
console.log("\nWhen searching for a phrase in GitHub:");
console.log("- Use double quotes to search for exact phrase: \"hello world\"");
console.log("- Backslash escapes special characters inside quotes");
console.log("- Example: to find 'test\"case', use: \"test\\\"case\"");
console.log("\nThe key issue:");
console.log("- Without escaping: in:title \"Plug \"FR\"\"");
console.log(" GitHub interprets this as: search in title for 'Plug' then literal text '\"FR\"'");
console.log("- With escaping: in:title \"Plug \\\"FR\\\"\"");
console.log(" GitHub interprets this correctly as: search phrase with quotes");
console.log("\n" + "=".repeat(70));
console.log("Conclusion: The unescaped version WILL cause incorrect search results");
console.log("because the inner quotes will break the phrase boundary.");
EOFRepository: GladysAssistant/Gladys
Length of output: 833
🏁 Script executed:
# Search for any examples or documentation about Tuya device names
find front/src/routes/integration/all/tuya -type f -name "*.md" -o -name "*.txt" | xargs cat 2>/dev/null | head -50Repository: GladysAssistant/Gladys
Length of output: 48
🏁 Script executed:
find . -path ./node_modules -prune -o -name "*.test.js" -o -name "*.spec.js" | xargs grep -l "tuya.*model\|device.*model" 2>/dev/null | head -5Repository: GladysAssistant/Gladys
Length of output: 366
🏁 Script executed:
node << 'EOF'
// Simulate actual GitHub API request with malformed query
const baseUrl = "https://api.github.com/search/issues";
// Example with unescaped quotes
const malformedQuery = 'repo:GladysAssistant/Gladys in:title "LSC Plug "FR""';
const encodedMalformed = encodeURIComponent(malformedQuery);
const urlMalformed = `${baseUrl}?q=${encodedMalformed}`;
// Example with escaped quotes
const escapedQuery = 'repo:GladysAssistant/Gladys in:title "LSC Plug \\"FR\\""';
const encodedEscaped = encodeURIComponent(escapedQuery);
const urlEscaped = `${baseUrl}?q=${encodedEscaped}`;
console.log("When GitHub API decodes these URLs:\n");
console.log("Malformed query URL-decodes to:");
console.log(decodeURIComponent(encodedMalformed));
console.log("\nGitHub interprets this as broken phrase search\n");
console.log("Escaped query URL-decodes to:");
console.log(decodeURIComponent(encodedEscaped));
console.log("\nGitHub interprets this as a valid phrase search\n");
console.log("Result: encodeURIComponent only makes it URL-safe,");
console.log("but doesn't fix the underlying GitHub search syntax issue.");
EOFRepository: GladysAssistant/Gladys
Length of output: 481
🏁 Script executed:
sed -n '287,312p' front/src/routes/integration/all/tuya/discover-page/githubIssue.jsRepository: GladysAssistant/Gladys
Length of output: 1304
Escape quotes in GitHub search query to prevent malformed phrase searches.
buildGithubSearchQuery() wraps the raw title in double quotes for a phrase search without escaping. If a device model like LSC Plug "FR" is used, the unescaped quotes break the phrase boundary, causing GitHub to misinterpret the search. This results in checkGithubIssues() failing to find duplicates and suggesting duplicate issues.
Suggested patch
+const escapeGithubSearchPhrase = value =>
+ String(value || '')
+ .replace(/\\/g, '\\\\')
+ .replace(/"/g, '\\"')
+ .replace(/\r?\n/g, ' ');
+
-const buildGithubSearchQuery = title => `repo:GladysAssistant/Gladys in:title "${title}"`;
+const buildGithubSearchQuery = title =>
+ `repo:GladysAssistant/Gladys in:title "${escapeGithubSearchPhrase(title)}"`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/routes/integration/all/tuya/discover-page/githubIssue.js` around
lines 304 - 311, The search query builder currently wraps raw title in quotes in
buildGithubSearchQuery (and is used by buildGithubSearchUrl and
buildGithubSearchApiUrl) without escaping internal quotes; update
buildGithubSearchQuery to escape any double quotes (and optionally backslashes)
in the title before surrounding it with quotes so phrase searches remain valid
(e.g., replace " with \" in the title), leaving the rest of the callers
(buildGithubSearchUrl/buildGithubSearchApiUrl) unchanged so encodeURIComponent
still runs on the final query string.
Comment on lines
+31
to
+36
| componentWillReceiveProps(nextProps) { | ||
| const currentDevice = this.props.device; | ||
| const nextDevice = nextProps.device; | ||
| if (!currentDevice || !nextDevice || currentDevice.external_id !== nextDevice.external_id) { | ||
| this.setState(buildInitialState()); | ||
| } |
There was a problem hiding this comment.
Ignore stale async completions after the device changes.
Both flows keep using the captured device after await checkGithubIssues(...). componentWillReceiveProps resets the state for a new external_id, but the older request can still repopulate the component or open an issue for the previous device. Capture the starting external_id and bail before every post-await setState/navigation if this.props.device no longer matches.
Also applies to: 75-95, 103-149, 236-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front/src/routes/integration/all/tuya/discover-page/TuyaGithubIssueSection.jsx`
around lines 31 - 36, The component is vulnerable to stale async completions
repopulating state or navigating for a previous device; in
TuyaGithubIssueSection.jsx (componentWillReceiveProps and the async flows that
call checkGithubIssues and perform setState/navigation) capture the current
device.external_id into a local const at the start of each async flow (e.g.,
inside the methods that call await checkGithubIssues(...), and inside any
handlers that eventually call setState or navigation), and before every
post-await setState or navigation compare this.props.device?.external_id (or the
latest prop) to the captured external_id and bail if they differ; apply the same
guard to the other async blocks referenced (around lines 75-95, 103-149,
236-260) so stale responses are ignored.
Comment on lines
+62
to
+69
| const popup = window.open('about:blank', '_blank'); | ||
| if (popup) { | ||
| popup.opener = null; | ||
| if (!skipDuplicateCheck) { | ||
| popup.document.title = 'GitHub'; | ||
| popup.document.body.innerText = 'Searching for existing issues...'; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the TuyaGithubIssueSection.jsx file
find . -name "TuyaGithubIssueSection.jsx" -type fRepository: GladysAssistant/Gladys
Length of output: 146
🏁 Script executed:
# Read the file to understand the popup handling
cat -n front/src/routes/integration/all/tuya/discover-page/TuyaGithubIssueSection.jsxRepository: GladysAssistant/Gladys
Length of output: 14962
Handle blocked popups before async operations.
When window.open('about:blank', '_blank') is blocked and returns null, the fallback window.open(issueUrl, '_blank') at line 155 (and similar locations) executes only after await checkGithubIssues(...) at line 75. This places the fallback outside the original click gesture context, causing strict popup blockers to drop it again, leaving the user without an issue page.
Also applies to: 236-259 (line 258 fallback occurs after await at line 236).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@front/src/routes/integration/all/tuya/discover-page/TuyaGithubIssueSection.jsx`
around lines 62 - 69, The click handler currently opens a popup (const popup =
window.open('about:blank', '_blank')) and then awaits checkGithubIssues(...)
before doing the fallback window.open(issueUrl, '_blank'), which loses the user
gesture if the initial open was blocked; change the flow so that if popup is
null you immediately attempt the fallback open(issueUrl, '_blank') synchronously
(still within the click handler) before any awaits, and only use the opened
window (popup) for later document updates after await; apply the same change for
the other occurrence around the second checkGithubIssues call (the block
involving the popup variable and the fallback at lines ~236-259) so both
fallbacks run inside the original user gesture.
Comment on lines
+136
to
+143
| const protocolVersion = params.PROTOCOL_VERSION || (device && device.protocol_version) || ''; | ||
| const ipAddress = params.IP_ADDRESS || (device && device.ip) || ''; | ||
| const cloudIp = params.CLOUD_IP || (device && device.cloud_ip) || ''; | ||
| const localOverride = normalizeBoolean(getLocalOverrideValue(device)); | ||
| const showCloudIp = localOverride !== true; | ||
| const displayIp = showCloudIp ? cloudIp : ipAddress; | ||
| const canPollLocal = localOverride === true && !!localKey && !!deviceId; | ||
| const pollProtocolLabel = localPollProtocol || protocolVersion || '-'; |
There was a problem hiding this comment.
Don’t collapse an explicit empty IP/protocol back to the persisted value.
Using || here makes '' indistinguishable from “unset”. On devices that already have device.protocol_version or device.ip, clearing the select/input snaps back to the old value, so users cannot trigger the advertised “leave empty for full scan / auto-detect” flow. The same fallback pattern is also used in front/src/routes/integration/all/tuya/commons/localPoll.js, so polling will still reuse the stale values even if the field was cleared.
🛠️ Suggested direction
render({ device, deviceIndex, localPollStatus, localPollError, localPollProtocol }) {
const params = buildParamsMap(device);
+ const hasParam = name => Object.prototype.hasOwnProperty.call(params, name);
const deviceId = params.DEVICE_ID || getTuyaDeviceId(device);
const localKey = params.LOCAL_KEY || (device && device.local_key) || '';
- const protocolVersion = params.PROTOCOL_VERSION || (device && device.protocol_version) || '';
- const ipAddress = params.IP_ADDRESS || (device && device.ip) || '';
+ const protocolVersion = hasParam('PROTOCOL_VERSION') ? params.PROTOCOL_VERSION : (device && device.protocol_version) || '';
+ const ipAddress = hasParam('IP_ADDRESS') ? params.IP_ADDRESS : (device && device.ip) || '';Apply the same distinction in commons/localPoll.js and TuyaDeviceBox.jsx so cleared values stay cleared end-to-end.
📝 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 protocolVersion = params.PROTOCOL_VERSION || (device && device.protocol_version) || ''; | |
| const ipAddress = params.IP_ADDRESS || (device && device.ip) || ''; | |
| const cloudIp = params.CLOUD_IP || (device && device.cloud_ip) || ''; | |
| const localOverride = normalizeBoolean(getLocalOverrideValue(device)); | |
| const showCloudIp = localOverride !== true; | |
| const displayIp = showCloudIp ? cloudIp : ipAddress; | |
| const canPollLocal = localOverride === true && !!localKey && !!deviceId; | |
| const pollProtocolLabel = localPollProtocol || protocolVersion || '-'; | |
| const hasParam = name => Object.prototype.hasOwnProperty.call(params, name); | |
| const protocolVersion = hasParam('PROTOCOL_VERSION') ? params.PROTOCOL_VERSION : (device && device.protocol_version) || ''; | |
| const ipAddress = hasParam('IP_ADDRESS') ? params.IP_ADDRESS : (device && device.ip) || ''; | |
| const cloudIp = params.CLOUD_IP || (device && device.cloud_ip) || ''; | |
| const localOverride = normalizeBoolean(getLocalOverrideValue(device)); | |
| const showCloudIp = localOverride !== true; | |
| const displayIp = showCloudIp ? cloudIp : ipAddress; | |
| const canPollLocal = localOverride === true && !!localKey && !!deviceId; | |
| const pollProtocolLabel = localPollProtocol || protocolVersion || '-'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/routes/integration/all/tuya/TuyaLocalPollSection.jsx` around lines
136 - 143, The current use of "||" when computing protocolVersion, ipAddress,
cloudIp, pollProtocolLabel (and similar logic in commons/localPoll.js and
TuyaDeviceBox.jsx) collapses explicit empty strings to persisted values,
preventing users from clearing fields to trigger the "empty for auto-detect"
flow; update these computations to distinguish unset (undefined/null) from an
explicit empty string by checking for null/undefined (e.g., using ?? or explicit
!== undefined/null checks) so cleared values remain '' end-to-end, and propagate
the same change to commons/localPoll.js and TuyaDeviceBox.jsx to ensure polling
and UI respect an explicit empty value.
Comment on lines
+135
to
+147
| const currentProperties = Array.isArray(properties && properties.properties) ? properties.properties : []; | ||
| currentProperties.forEach((property) => { | ||
| const { code } = property || {}; | ||
| if (!code || groups[code]) { | ||
| return; | ||
| } | ||
| groups[code] = { | ||
| code, | ||
| name: property.custom_name || property.name || code, | ||
| values: {}, | ||
| readOnly: true, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Handle array-shaped shadow properties here too.
server/services/tuya/lib/mappings/index.js already accepts properties as either an array or { properties: [...] }, but this block only reads the wrapped form. When the shadow payload arrives as a bare array, type detection still succeeds while currentProperties stays empty, so shadow-only devices won't create any features.
🔧 Suggested fix
- const currentProperties = Array.isArray(properties && properties.properties) ? properties.properties : [];
+ const currentProperties = Array.isArray(properties)
+ ? properties
+ : Array.isArray(properties && properties.properties)
+ ? properties.properties
+ : [];📝 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 currentProperties = Array.isArray(properties && properties.properties) ? properties.properties : []; | |
| currentProperties.forEach((property) => { | |
| const { code } = property || {}; | |
| if (!code || groups[code]) { | |
| return; | |
| } | |
| groups[code] = { | |
| code, | |
| name: property.custom_name || property.name || code, | |
| values: {}, | |
| readOnly: true, | |
| }; | |
| }); | |
| const currentProperties = Array.isArray(properties) | |
| ? properties | |
| : Array.isArray(properties && properties.properties) | |
| ? properties.properties | |
| : []; | |
| currentProperties.forEach((property) => { | |
| const { code } = property || {}; | |
| if (!code || groups[code]) { | |
| return; | |
| } | |
| groups[code] = { | |
| code, | |
| name: property.custom_name || property.name || code, | |
| values: {}, | |
| readOnly: true, | |
| }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/services/tuya/lib/device/tuya.convertDevice.js` around lines 135 -
147, currentProperties is only populated when properties is an object with a
properties array, so bare-array shadow payloads are ignored; change the
initialization of currentProperties to accept either a plain array or the
wrapped form (i.e., treat properties as the array when Array.isArray(properties)
is true, otherwise fall back to Array.isArray(properties &&
properties.properties) ? properties.properties : []), leaving the rest of the
loop (groups, property.code, property.custom_name/name) unchanged.
Comment on lines
+222
to
+239
| device.params = Array.isArray(device.params) ? [...device.params] : []; | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.IP_ADDRESS, resolvedIp); | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.PROTOCOL_VERSION, resolvedProtocolVersion); | ||
| if (resolvedLocalKey) { | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_KEY, resolvedLocalKey); | ||
| } | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_OVERRIDE, true); | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_ID, resolvedProductId); | ||
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_KEY, resolvedProductKey); | ||
|
|
||
| device = addFallbackBinaryFeature(device, dps); | ||
|
|
||
| if (tuyaManager.gladys && tuyaManager.gladys.stateManager) { | ||
| const existing = normalizeExistingDevice( | ||
| tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id), | ||
| ); | ||
| device = mergeDevices(device, existing); | ||
| } |
There was a problem hiding this comment.
Preserve saved params before calling mergeDevices.
mergeDevices() replaces params with newDevice.params; it does not deep-merge them. Here device.params is rebuilt from the discovered-device copy only, so any saved params that exist only on the Gladys device get dropped after a local poll. That is especially risky for params like CLOUD_READ_STRATEGY, which this PR now relies on for Thing Model shadow-read behavior.
Suggested patch
if (tuyaManager.gladys && tuyaManager.gladys.stateManager) {
const existing = normalizeExistingDevice(
tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id),
);
+ const existingParams = Array.isArray(existing && existing.params) ? existing.params : [];
+ const currentParamNames = new Set(device.params.map((param) => param.name));
+ device.params = [
+ ...device.params,
+ ...existingParams.filter((param) => !currentParamNames.has(param.name)),
+ ];
device = mergeDevices(device, existing);
}Based on learnings: prefer returning a partial device object with whatever data is available and keep Tuya flows resilient instead of dropping usable data.
📝 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
| device.params = Array.isArray(device.params) ? [...device.params] : []; | |
| upsertParam(device.params, DEVICE_PARAM_NAME.IP_ADDRESS, resolvedIp); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PROTOCOL_VERSION, resolvedProtocolVersion); | |
| if (resolvedLocalKey) { | |
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_KEY, resolvedLocalKey); | |
| } | |
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_OVERRIDE, true); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_ID, resolvedProductId); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_KEY, resolvedProductKey); | |
| device = addFallbackBinaryFeature(device, dps); | |
| if (tuyaManager.gladys && tuyaManager.gladys.stateManager) { | |
| const existing = normalizeExistingDevice( | |
| tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id), | |
| ); | |
| device = mergeDevices(device, existing); | |
| } | |
| device.params = Array.isArray(device.params) ? [...device.params] : []; | |
| upsertParam(device.params, DEVICE_PARAM_NAME.IP_ADDRESS, resolvedIp); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PROTOCOL_VERSION, resolvedProtocolVersion); | |
| if (resolvedLocalKey) { | |
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_KEY, resolvedLocalKey); | |
| } | |
| upsertParam(device.params, DEVICE_PARAM_NAME.LOCAL_OVERRIDE, true); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_ID, resolvedProductId); | |
| upsertParam(device.params, DEVICE_PARAM_NAME.PRODUCT_KEY, resolvedProductKey); | |
| device = addFallbackBinaryFeature(device, dps); | |
| if (tuyaManager.gladys && tuyaManager.gladys.stateManager) { | |
| const existing = normalizeExistingDevice( | |
| tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id), | |
| ); | |
| const existingParams = Array.isArray(existing && existing.params) ? existing.params : []; | |
| const currentParamNames = new Set(device.params.map((param) => param.name)); | |
| device.params = [ | |
| ...device.params, | |
| ...existingParams.filter((param) => !currentParamNames.has(param.name)), | |
| ]; | |
| device = mergeDevices(device, existing); | |
| } |
🤖 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 222 - 239,
mergeDevices overwrites device.params with newDevice.params causing saved
Gladys-only params to be lost; before calling
normalizeExistingDevice/mergeDevices, retrieve the existing device params via
tuyaManager.gladys.stateManager.get('deviceByExternalId', device.external_id)
(as done with normalizeExistingDevice), then merge those existing params into
the local device.params (preserving existing entries keyed by param name) —
e.g., collect existing.params, iterate and upsert any entries not present in
device.params using the same upsertParam helper (or dedupe by DEVICE_PARAM_NAME
keys) so CLOUD_READ_STRATEGY and other persisted params are retained, then call
normalizeExistingDevice and mergeDevices as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
npm teston both front/server)npm run eslinton both front/server)npm run prettieron both front/server)npm run compare-translationson front)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
#2460.tuya-follow-up-hardening.Details
smart-socket-b61eihfqeaexn54gsetValueWhy this matters
Scope
Compared to
tuya-follow-up-hardening:Server (prod)
+147 lines / -29 linesServer Tests
+1110 lines / -214 linesNo front changes.
No translation changes.
No package changes.
Total
31 files changed+1257 lines / -243 linesSummary by CodeRabbit
Release Notes