refactor: rework the gui/heightmap store#2425
Open
meteyou wants to merge 78 commits intomainsail-crew:developfrom
Open
refactor: rework the gui/heightmap store#2425meteyou wants to merge 78 commits intomainsail-crew:developfrom
meteyou wants to merge 78 commits intomainsail-crew:developfrom
Conversation
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process # Conflicts: # src/plugins/webSocketClient.ts
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nraker-types-emitAndWait
…/init-server-process # Conflicts: # src/components/dialogs/SpoolmanChangeSpoolDialog.vue # src/components/panels/SpoolmanPanel.vue
…he moonraker docs Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process # Conflicts: # src/types/MoonrakerRPCInterface.ts
…files Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…/init-server-process
…sponses Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nraker-types-emitAndWait
…/init-server-process
Replace complex dual-timer polling system with unified async approach: - Consolidate 7 actions into 4 by extracting shared checkAndUpdateKlippyState() - Replace callback-based emit() with async emitAndWait() - Merge klippy_connected_timer and klippy_state_timer into single klippy_polling_timer - Load printer/initGcodes only when gcode.commands is empty - Add printer.info RPC type definition Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…cessing Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…rver-process # Conflicts: # src/plugins/webSocketClient.ts # src/types/moonraker/PrinterRPC.ts # src/types/moonraker/ServerRPC.ts # src/types/moonraker/index.ts
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
… conversion Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ity toggle logic Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…tion Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ssion Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ity handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ate state management Signed-off-by: Stefan Dej <meteyou@gmail.com>
…lity handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…setter Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…nd streamline layout handling Signed-off-by: Stefan Dej <meteyou@gmail.com>
…mutations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…operations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…operations Signed-off-by: Stefan Dej <meteyou@gmail.com>
…ult values Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive initialization system redesign with multi-stage startup flow, modernizes Vuex store architecture by consolidating mutations and socket-based persistence, refactors component state management to use store getters and actions instead of direct mutations, expands Moonraker RPC type definitions with database and history contracts, enhances WebSocket client with timeout support and improved error handling, and adds localization for initialization progress tracking. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UI as TheConnectingDialog
participant Socket
participant Server
participant Store as Vuex Store
Client->>Socket: connect()
Socket->>Store: emit initialize event
activate Store
Store->>Server: initIdentifyClient
Store->>UI: setInitializationStep('Identifying client...')
UI->>Client: display: "Identifying client..."
Server-->>Store: client identified
Store->>Server: initServerInfo
Store->>UI: setInitializationStep('Checking Klipper...')
UI->>Client: display: "Checking Klipper..."
Server-->>Store: server info loaded
Store->>Server: initServerConfig
Store->>UI: setInitializationStepComponent('Database')
UI->>Client: display: "Loading Database..."
UI->>Client: setInitializationProgress(25)
Server-->>Store: config loaded
Store->>Server: initServerComponents(['Announcements', 'History', ...])
loop for each component
Store->>UI: setInitializationStepComponent(component)
UI->>Client: display: "Loading {component}..."
UI->>Client: setInitializationProgress(updated)
Server-->>Store: component ready
end
Store->>Server: initKlippyConnection
Server-->>Store: Klippy ready
Store->>UI: resetInitialization()
UI->>Client: display: fully connected
deactivate Store
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/mixins/history.ts (1)
31-37:⚠️ Potential issue | 🟠 MajorStore config refactoring incomplete - base.ts still uses old nested pattern.
The change from
server.config?.configtoserver.configin history.ts is correct and aligns with the flattened store structure. However, this refactoring is inconsistent across the codebase:src/components/mixins/base.ts(line 152) still uses the old nested patternthis.$store.state.server.config.config?.spoolman?.server, which will break at runtime.Either base.ts needs to be updated to use the new flat pattern, or history.ts should revert to match. Verify the store structure definition and update the remaining file to maintain consistency.
src/plugins/webSocketClient.ts (1)
70-74:⚠️ Potential issue | 🟡 MinorAdd a guard to prevent late RPC responses from reaching the notification handler.
When a timed-out request eventually responds,
waitwill be missing and the message is dispatched tosocket/onMessage. The handler checkspayload.methodfor notification patterns and has no explicit handling for responses with anidfield—these fall through to a debug log. Add a guard to drop responses before dispatching them.Suggested fix
- if (!wait) { - this.store?.dispatch('socket/onMessage', data) - return - } + if (!wait) { + if (data.id !== undefined) return + this.store?.dispatch('socket/onMessage', data) + return + }src/store/socket/mutations.ts (1)
70-85:⚠️ Potential issue | 🟠 MajorFix multi‑removal bug in
removeInitComponent.Removing by ascending indexes shifts the array and can skip targets or delete the wrong item when multiple matches exist. Prefer filtering or splicing from the end.
✅ Safer removal via filter
removeInitComponent(state, payload) { - const list = [...state.initializationList] - - // remove all components witch starts with payload - const indexes = list.reduce((acc: number[], item, index) => { - if (item.startsWith(payload)) acc.push(index) - return acc - }, []) - - // stop if no items found - if (!indexes.length) return - - // remove all items - indexes.forEach((index) => list.splice(index, 1)) - - Vue.set(state, 'initializationList', list) + const list = state.initializationList.filter((item) => !item.startsWith(payload)) + if (list.length === state.initializationList.length) return + Vue.set(state, 'initializationList', list) },
🤖 Fix all issues with AI agents
In `@src/plugins/webSocketClient.ts`:
- Around line 199-203: The code is creating WebSocketTimeoutError with a string
that already includes "Timeout:" causing a double prefix; update the timeout
rejection to pass the raw method (or other plain identifier) to the
WebSocketTimeoutError constructor instead of a preformatted message (so replace
new WebSocketTimeoutError(`Timeout: ${method}`) with new
WebSocketTimeoutError(method)), and make the same change in the other timeout
block later in the file (the one that also calls this.removeWaitById and rejects
with WebSocketTimeoutError) so the constructor's own prefix is the only
"Timeout:" present.
In `@src/store/gui/actions.ts`:
- Around line 24-38: The init action dispatches restoreValues without awaiting
it, causing concurrent mutations to values (e.g., deletion of
values.remoteprinters and calls to deletePath) to race with restoreValues;
change the call to await the dispatch so restoreValues finishes before mutating
values (await dispatch('restoreValues', values) or await
this.dispatch('restoreValues', values)), keeping the rest of init (mutations and
deletePath calls) after that await to prevent inconsistent persistence and
unhandled rejections.
In `@src/store/gui/mutations.ts`:
- Around line 16-26: The deepSet helper inside saveSetting creates intermediate
objects even when payload.value is undefined, leading to unintended state
mutations; update saveSetting to guard early by returning immediately when
payload.value === undefined (so deletions are handled by deleteSetting instead)
or modify deepSet to not call Vue.set(...) to create new nodes when value is
undefined — ensure the check prevents creating empty objects while still
allowing valid assignments in saveSetting/deepSet.
In `@src/store/server/actions.ts`:
- Around line 187-196: The current creation of regexFilters via new
RegExp(filter) in initGcodeStore and the socket message handler risks ReDoS from
user-supplied patterns; validate patterns before compiling by using a safe-regex
check (e.g., add the safe-regex library or an equivalent timeout/AST-based
validator) when filters are persisted or immediately before constructing RegExp
in the arrays mapped to regexFilters and the socket handler, reject or sanitize
patterns that fail the safety check and log the rejected pattern via logError
(reference symbols: regexFilters, filters, initGcodeStore and the socket event
handler where incoming messages are filtered) so only safe, pre-validated
regexes are compiled and used during high-frequency .test() calls.
In `@src/store/server/mutations.ts`:
- Around line 101-105: The early return in setThrottledState when payload ===
null leaves stale values in state.throttled_state; change the handler so that
when payload is null it explicitly resets/clears the throttled state (e.g.
Vue.set(state.throttled_state, 'bits', null/0/empty) and
Vue.set(state.throttled_state, 'flags', null/0/empty) or restores your chosen
defaults) and otherwise keep the existing conditional Vue.set updates for 'bits'
and 'flags'; update the function setThrottledState and the uses of
state.throttled_state to ensure the cleared default values represent "no
throttling info."
🧹 Nitpick comments (5)
src/components/panels/Gcodefiles/GcodefilesPanelHeaderSettings.vue (1)
76-83: Consider reusing the shared toggle helper.This manual toggle logic can be simplified and kept consistent with other panels by using
toggleArrayItem.♻️ Suggested refactor
-import { Component, Mixins } from 'vue-property-decorator' +import { Component, Mixins } from 'vue-property-decorator' +import { toggleArrayItem } from '@/plugins/helpers' @@ changeMetadataVisible(name: string) { - const value = [...(this.$store.state.gui.view.gcodefiles.hideMetadataColumns ?? [])] - const index = value.findIndex((value: string) => value === name) - - if (index !== -1) value.splice(index, 1) - else value.push(name) + const current = this.$store.state.gui.view.gcodefiles.hideMetadataColumns ?? [] + const value = toggleArrayItem(current, name) this.$store.dispatch('gui/saveSetting', { name: 'view.gcodefiles.hideMetadataColumns', value }) }src/store/gui/types.ts (1)
10-12: Consider array handling inDeepPartial.The current implementation treats arrays as objects, making their elements optional. This is typically fine for state initialization payloads, but if you need to preserve array types exactly, consider adding an array check:
export type DeepPartial<T> = { [P in keyof T]?: T[P] extends (infer U)[] ? U[] : T[P] extends object ? DeepPartial<T[P]> : T[P] }This is a minor edge case and the current implementation should work for the intended use cases in this PR.
src/store/gui/getters.ts (1)
32-35: Consider guardingdatasetSettingsitself to avoid null/undefined access.If
datasetSettingscan be unset during early init, optional chaining on the map access prevents a runtime error.💡 Suggested defensive tweak
- const additionalSensors = state.view.tempchart.datasetSettings[payload.name]?.additionalSensors ?? {} + const additionalSensors = + state.view.tempchart.datasetSettings?.[payload.name]?.additionalSensors ?? {}src/store/gui/macros/actions.ts (1)
61-82: Handle RPC failure to avoid local/remote divergence.
groupDeletemutates local state before awaiting the server delete; if the RPC fails, the UI will show the group removed while the server still has it. Consider deleting first (or add rollback handling).🛠️ Suggested reorder
async groupDelete({ commit, dispatch, rootState }, id) { - commit('groupDelete', id) - await Vue.$socket.emitAndWait('server.database.delete_item', { + await Vue.$socket.emitAndWait('server.database.delete_item', { namespace: 'mainsail', key: `macros.macrogroups.${id}`, }) + commit('groupDelete', id) type DashboardLayoutKey = Exclude<keyof GuiState['dashboard'], 'nonExpandPanels'> const layouts = Object.keys(rootState.gui?.dashboard ?? {}).filter( (key) => key !== 'nonExpandPanels' ) as DashboardLayoutKey[]src/store/gui/heightmap/actions.ts (1)
6-8: Consider unifying persistence patterns.
saveActiveColorSchemecommits directly to the local mutation, whilesaveSettingdispatches to the centralizedgui/saveSettingaction. For consistency, consider whethersaveActiveColorSchemeshould also persist to the database through the centralized action.
Comment on lines
+199
to
+203
| if (options.timeout) { | ||
| timeoutId = window.setTimeout(() => { | ||
| this.removeWaitById(id) | ||
| reject(new WebSocketTimeoutError(`Timeout: ${method}`)) | ||
| }, options.timeout) |
There was a problem hiding this comment.
Avoid double “Timeout:” prefix in WebSocketTimeoutError.
Constructor already prefixes; passing a preformatted string yields “Timeout: Timeout: …”.
🐛 Proposed fix
- reject(new WebSocketTimeoutError(`Timeout: ${method}`))
+ reject(new WebSocketTimeoutError(method))Also applies to: 325-329
🤖 Prompt for AI Agents
In `@src/plugins/webSocketClient.ts` around lines 199 - 203, The code is creating
WebSocketTimeoutError with a string that already includes "Timeout:" causing a
double prefix; update the timeout rejection to pass the raw method (or other
plain identifier) to the WebSocketTimeoutError constructor instead of a
preformatted message (so replace new WebSocketTimeoutError(`Timeout: ${method}`)
with new WebSocketTimeoutError(method)), and make the same change in the other
timeout block later in the file (the one that also calls this.removeWaitById and
rejects with WebSocketTimeoutError) so the constructor's own prefix is the only
"Timeout:" present.
Comment on lines
+24
to
38
| async init({ commit, dispatch, rootGetters, rootState }) { | ||
| logDebug('init') | ||
|
|
||
| async initStore({ commit, dispatch, rootGetters, rootState }, payload) { | ||
| const baseUrl = rootGetters['socket/getUrl'] + '/server/database/item' | ||
| const mainsailUrl = baseUrl + '?namespace=mainsail' | ||
| let values: GuiStateInitPayload = {} | ||
| try { | ||
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | ||
| values = payload.value as GuiStateInitPayload | ||
| } catch (error) { | ||
| logDebug('create Mainsail namespace') | ||
|
|
||
| if ('remoteprinters' in payload.value) { | ||
| if (rootState.instancesDB === 'moonraker') | ||
| dispatch('remoteprinters/initStore', payload.value.remoteprinters.printers) | ||
| delete payload.value.remoteprinters | ||
| } | ||
| values = await this.dispatch('gui/getDefaults') | ||
| values.initVersion = rootGetters['getVersion'] | ||
|
|
||
| // delete currentPath if exists | ||
| if (payload.value?.view?.gcodefiles?.currentPath) { | ||
| window.console.debug('remove currentPath from gui namespace') | ||
| await fetch(mainsailUrl + '&key=view.gcodefiles.currentPath', { method: 'DELETE' }) | ||
| dispatch('restoreValues', values) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full init function to see the complete context
cat -n src/store/gui/actions.ts | head -50Repository: mainsail-crew/mainsail
Length of output: 2182
🏁 Script executed:
# Let's also search for the restoreValues action definition
rg "restoreValues" src/store/gui/actions.ts -A 10 -B 2Repository: mainsail-crew/mainsail
Length of output: 1047
🏁 Script executed:
# Get file size to understand how much content we're dealing with
wc -l src/store/gui/actions.tsRepository: mainsail-crew/mainsail
Length of output: 94
🏁 Script executed:
rg "async restoreValues" src/store/gui/actions.ts -A 30Repository: mainsail-crew/mainsail
Length of output: 1305
🏁 Script executed:
# Also check if there are any other async operations after line 37
sed -n '37,100p' src/store/gui/actions.tsRepository: mainsail-crew/mainsail
Length of output: 2403
Await restoreValues to prevent race condition with value mutations.
restoreValues is dispatched without await but the values object is mutated immediately afterward (line 44: delete values.remoteprinters, lines 47-59: deletePath(values, key)). Since restoreValues is async and iterates through the values object while making socket calls, mutations during this process can cause inconsistent data to be persisted and hide errors as unhandled rejections.
🛠️ Suggested fix
- dispatch('restoreValues', values)
+ await dispatch('restoreValues', values)📝 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
| async init({ commit, dispatch, rootGetters, rootState }) { | |
| logDebug('init') | |
| async initStore({ commit, dispatch, rootGetters, rootState }, payload) { | |
| const baseUrl = rootGetters['socket/getUrl'] + '/server/database/item' | |
| const mainsailUrl = baseUrl + '?namespace=mainsail' | |
| let values: GuiStateInitPayload = {} | |
| try { | |
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | |
| values = payload.value as GuiStateInitPayload | |
| } catch (error) { | |
| logDebug('create Mainsail namespace') | |
| if ('remoteprinters' in payload.value) { | |
| if (rootState.instancesDB === 'moonraker') | |
| dispatch('remoteprinters/initStore', payload.value.remoteprinters.printers) | |
| delete payload.value.remoteprinters | |
| } | |
| values = await this.dispatch('gui/getDefaults') | |
| values.initVersion = rootGetters['getVersion'] | |
| // delete currentPath if exists | |
| if (payload.value?.view?.gcodefiles?.currentPath) { | |
| window.console.debug('remove currentPath from gui namespace') | |
| await fetch(mainsailUrl + '&key=view.gcodefiles.currentPath', { method: 'DELETE' }) | |
| dispatch('restoreValues', values) | |
| } | |
| async init({ commit, dispatch, rootGetters, rootState }) { | |
| logDebug('init') | |
| let values: GuiStateInitPayload = {} | |
| try { | |
| const payload = await Vue.$socket.emitAndWait('server.database.get_item', { namespace: 'mainsail' }) | |
| values = payload.value as GuiStateInitPayload | |
| } catch (error) { | |
| logDebug('create Mainsail namespace') | |
| values = await this.dispatch('gui/getDefaults') | |
| values.initVersion = rootGetters['getVersion'] | |
| await dispatch('restoreValues', values) | |
| } |
🤖 Prompt for AI Agents
In `@src/store/gui/actions.ts` around lines 24 - 38, The init action dispatches
restoreValues without awaiting it, causing concurrent mutations to values (e.g.,
deletion of values.remoteprinters and calls to deletePath) to race with
restoreValues; change the call to await the dispatch so restoreValues finishes
before mutating values (await dispatch('restoreValues', values) or await
this.dispatch('restoreValues', values)), keeping the rest of init (mutations and
deletePath calls) after that await to prevent inconsistent persistence and
unhandled rejections.
Comment on lines
16
to
+26
| saveSetting(state, payload: { name: string; value: any }) { | ||
| // eslint-disable-next-line | ||
| const deepSet = (obj: any, is: string[] | string, value: any): any => { | ||
| if (is !== undefined && typeof is === 'string') return deepSet(obj, is.split('.'), value) | ||
| else if (is.length == 1 && value !== undefined) return (obj[is[0]] = value) | ||
| else if (is.length == 0) return obj | ||
| else if (!(is[0] in obj)) obj[is[0]] = {} | ||
| return deepSet(obj[is[0]], is.slice(1), value) | ||
| } | ||
|
|
||
| deepSet(state, payload.name, payload.value) | ||
| }, | ||
|
|
||
| setHeaterChartVisibility(state, payload) { | ||
| const index = state.view.tempchart.hiddenDataset.indexOf(payload.name.toUpperCase()) | ||
|
|
||
| if (payload.hidden && index === -1) state.view.tempchart.hiddenDataset.push(payload.name.toUpperCase()) | ||
| else if (payload.hidden !== true && index > -1) state.view.tempchart.hiddenDataset.splice(index, 1) | ||
| }, | ||
|
|
||
| setGcodefilesMetadata(state, data) { | ||
| const array = [...state.view.gcodefiles.hideMetadataColumns] | ||
| const index = array.findIndex((value: string) => value === data.name) | ||
|
|
||
| if (data.value && index !== -1) array.splice(index, 1) | ||
| else if (!data.value && index === -1) array.push(data.name) | ||
|
|
||
| Vue.set(state.view.gcodefiles, 'hideMetadataColumns', array) | ||
| }, | ||
| else if (is?.length === 1 && value !== undefined) return Vue.set(obj, is[0], value) | ||
| else if (is?.length === 0) return obj | ||
| else if (!(is[0] in obj)) Vue.set(obj, is[0], {}) | ||
|
|
||
| setGcodefilesShowHiddenFiles(state, value) { | ||
| Vue.set(state.view.gcodefiles, 'showHiddenFiles', value) | ||
| }, | ||
|
|
||
| setCurrentWebcam(state, payload) { | ||
| Vue.set(state.view.webcam.currentCam, payload.page, payload.value) | ||
| }, | ||
|
|
||
| setHistoryColumns(state, data) { | ||
| if (data.value && state.view.history.hideColums.includes(data.name)) { | ||
| state.view.history.hideColums.splice(state.view.history.hideColums.indexOf(data.name), 1) | ||
| } else if (!data.value && !state.view.history.hideColums.includes(data.name)) { | ||
| state.view.history.hideColums.push(data.name) | ||
| return deepSet(obj[is[0]], is?.slice(1), value) | ||
| } | ||
| }, | ||
|
|
||
| setHistoryHidePrintStatus(state, payload) { | ||
| Vue.set(state.view.history, 'hidePrintStatus', payload) | ||
| }, | ||
|
|
||
| addClosePanel(state, payload) { | ||
| const nonExpandPanels = [...state.dashboard.nonExpandPanels[payload.viewport]] | ||
|
|
||
| if (!nonExpandPanels.includes(payload.name)) { | ||
| nonExpandPanels.push(payload.name) | ||
|
|
||
| Vue.set(state.dashboard.nonExpandPanels, payload.viewport, nonExpandPanels) | ||
| } | ||
| }, | ||
|
|
||
| removeClosePanel(state, payload) { | ||
| const nonExpandPanels = [...state.dashboard.nonExpandPanels[payload.viewport]] | ||
| const index = nonExpandPanels.indexOf(payload.name) | ||
| if (index > -1) { | ||
| nonExpandPanels.splice(index, 1) | ||
|
|
||
| Vue.set(state.dashboard.nonExpandPanels, payload.viewport, nonExpandPanels) | ||
| } | ||
| }, | ||
|
|
||
| deleteFromDashboardLayout(state, payload) { | ||
| // @ts-ignore | ||
| const layoutArray = [...state.dashboard[payload.layoutname]] | ||
| layoutArray.splice(payload.index, 1) | ||
| Vue.set(state.dashboard, payload.layoutname, layoutArray) | ||
| }, | ||
|
|
||
| setChartDatasetStatus(state, payload: { objectName: string; dataset: string; value: boolean }) { | ||
| // set new value if object doesn't exist in view.tempchart.datasetSettings | ||
| if (!(payload.objectName in state.view.tempchart.datasetSettings)) { | ||
| const newVal: { [key: string]: any } = {} | ||
| newVal[payload.dataset] = payload.value | ||
|
|
||
| Vue.set(state.view.tempchart.datasetSettings, payload.objectName, newVal) | ||
| return | ||
| } | ||
|
|
||
| Vue.set(state.view.tempchart.datasetSettings[payload.objectName], payload.dataset, payload.value) | ||
| }, | ||
|
|
||
| setDatasetAdditionalSensorStatus(state, payload: { objectName: string; dataset: string; value: boolean }) { | ||
| // set new value if object doesn't exist in view.tempchart.datasetSettings | ||
| if (!(payload.objectName in state.view.tempchart.datasetSettings)) { | ||
| const newVal: { additionalSensors: { [key: string]: any } } = { additionalSensors: {} } | ||
| newVal.additionalSensors[payload.dataset] = payload.value | ||
|
|
||
| Vue.set(state.view.tempchart.datasetSettings, payload.objectName, newVal) | ||
| return | ||
| } | ||
|
|
||
| // set new value if additionalSensor object doesn't exist in view.tempchart.datasetSettings | ||
| if (!('additionalSensors' in state.view.tempchart.datasetSettings[payload.objectName])) { | ||
| const newVal: { [key: string]: any } = {} | ||
| newVal[payload.dataset] = payload.value | ||
|
|
||
| Vue.set(state.view.tempchart.datasetSettings[payload.objectName], 'additionalSensors', newVal) | ||
| return | ||
| } | ||
|
|
||
| Vue.set( | ||
| state.view.tempchart.datasetSettings[payload.objectName].additionalSensors, | ||
| payload.dataset, | ||
| payload.value | ||
| ) | ||
| deepSet(state, payload.name, payload.value) |
There was a problem hiding this comment.
Guard against unintended state mutations when value is undefined.
If saveSetting is called with value: undefined, deepSet still creates intermediate objects and leaves empty nodes in state. An early return keeps state stable and pushes removals to deleteSetting.
🛠️ Suggested fix
const deepSet = (obj: any, is: string[] | string, value: any): any => {
+ if (value === undefined) return obj
if (is !== undefined && typeof is === 'string') return deepSet(obj, is.split('.'), value)
else if (is?.length === 1 && value !== undefined) return Vue.set(obj, is[0], value)
else if (is?.length === 0) return obj
else if (!(is[0] in obj)) Vue.set(obj, is[0], {})📝 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
| saveSetting(state, payload: { name: string; value: any }) { | |
| // eslint-disable-next-line | |
| const deepSet = (obj: any, is: string[] | string, value: any): any => { | |
| if (is !== undefined && typeof is === 'string') return deepSet(obj, is.split('.'), value) | |
| else if (is.length == 1 && value !== undefined) return (obj[is[0]] = value) | |
| else if (is.length == 0) return obj | |
| else if (!(is[0] in obj)) obj[is[0]] = {} | |
| return deepSet(obj[is[0]], is.slice(1), value) | |
| } | |
| deepSet(state, payload.name, payload.value) | |
| }, | |
| setHeaterChartVisibility(state, payload) { | |
| const index = state.view.tempchart.hiddenDataset.indexOf(payload.name.toUpperCase()) | |
| if (payload.hidden && index === -1) state.view.tempchart.hiddenDataset.push(payload.name.toUpperCase()) | |
| else if (payload.hidden !== true && index > -1) state.view.tempchart.hiddenDataset.splice(index, 1) | |
| }, | |
| setGcodefilesMetadata(state, data) { | |
| const array = [...state.view.gcodefiles.hideMetadataColumns] | |
| const index = array.findIndex((value: string) => value === data.name) | |
| if (data.value && index !== -1) array.splice(index, 1) | |
| else if (!data.value && index === -1) array.push(data.name) | |
| Vue.set(state.view.gcodefiles, 'hideMetadataColumns', array) | |
| }, | |
| else if (is?.length === 1 && value !== undefined) return Vue.set(obj, is[0], value) | |
| else if (is?.length === 0) return obj | |
| else if (!(is[0] in obj)) Vue.set(obj, is[0], {}) | |
| setGcodefilesShowHiddenFiles(state, value) { | |
| Vue.set(state.view.gcodefiles, 'showHiddenFiles', value) | |
| }, | |
| setCurrentWebcam(state, payload) { | |
| Vue.set(state.view.webcam.currentCam, payload.page, payload.value) | |
| }, | |
| setHistoryColumns(state, data) { | |
| if (data.value && state.view.history.hideColums.includes(data.name)) { | |
| state.view.history.hideColums.splice(state.view.history.hideColums.indexOf(data.name), 1) | |
| } else if (!data.value && !state.view.history.hideColums.includes(data.name)) { | |
| state.view.history.hideColums.push(data.name) | |
| return deepSet(obj[is[0]], is?.slice(1), value) | |
| } | |
| }, | |
| setHistoryHidePrintStatus(state, payload) { | |
| Vue.set(state.view.history, 'hidePrintStatus', payload) | |
| }, | |
| addClosePanel(state, payload) { | |
| const nonExpandPanels = [...state.dashboard.nonExpandPanels[payload.viewport]] | |
| if (!nonExpandPanels.includes(payload.name)) { | |
| nonExpandPanels.push(payload.name) | |
| Vue.set(state.dashboard.nonExpandPanels, payload.viewport, nonExpandPanels) | |
| } | |
| }, | |
| removeClosePanel(state, payload) { | |
| const nonExpandPanels = [...state.dashboard.nonExpandPanels[payload.viewport]] | |
| const index = nonExpandPanels.indexOf(payload.name) | |
| if (index > -1) { | |
| nonExpandPanels.splice(index, 1) | |
| Vue.set(state.dashboard.nonExpandPanels, payload.viewport, nonExpandPanels) | |
| } | |
| }, | |
| deleteFromDashboardLayout(state, payload) { | |
| // @ts-ignore | |
| const layoutArray = [...state.dashboard[payload.layoutname]] | |
| layoutArray.splice(payload.index, 1) | |
| Vue.set(state.dashboard, payload.layoutname, layoutArray) | |
| }, | |
| setChartDatasetStatus(state, payload: { objectName: string; dataset: string; value: boolean }) { | |
| // set new value if object doesn't exist in view.tempchart.datasetSettings | |
| if (!(payload.objectName in state.view.tempchart.datasetSettings)) { | |
| const newVal: { [key: string]: any } = {} | |
| newVal[payload.dataset] = payload.value | |
| Vue.set(state.view.tempchart.datasetSettings, payload.objectName, newVal) | |
| return | |
| } | |
| Vue.set(state.view.tempchart.datasetSettings[payload.objectName], payload.dataset, payload.value) | |
| }, | |
| setDatasetAdditionalSensorStatus(state, payload: { objectName: string; dataset: string; value: boolean }) { | |
| // set new value if object doesn't exist in view.tempchart.datasetSettings | |
| if (!(payload.objectName in state.view.tempchart.datasetSettings)) { | |
| const newVal: { additionalSensors: { [key: string]: any } } = { additionalSensors: {} } | |
| newVal.additionalSensors[payload.dataset] = payload.value | |
| Vue.set(state.view.tempchart.datasetSettings, payload.objectName, newVal) | |
| return | |
| } | |
| // set new value if additionalSensor object doesn't exist in view.tempchart.datasetSettings | |
| if (!('additionalSensors' in state.view.tempchart.datasetSettings[payload.objectName])) { | |
| const newVal: { [key: string]: any } = {} | |
| newVal[payload.dataset] = payload.value | |
| Vue.set(state.view.tempchart.datasetSettings[payload.objectName], 'additionalSensors', newVal) | |
| return | |
| } | |
| Vue.set( | |
| state.view.tempchart.datasetSettings[payload.objectName].additionalSensors, | |
| payload.dataset, | |
| payload.value | |
| ) | |
| deepSet(state, payload.name, payload.value) | |
| saveSetting(state, payload: { name: string; value: any }) { | |
| const deepSet = (obj: any, is: string[] | string, value: any): any => { | |
| if (value === undefined) return obj | |
| if (is !== undefined && typeof is === 'string') return deepSet(obj, is.split('.'), value) | |
| else if (is?.length === 1 && value !== undefined) return Vue.set(obj, is[0], value) | |
| else if (is?.length === 0) return obj | |
| else if (!(is[0] in obj)) Vue.set(obj, is[0], {}) | |
| return deepSet(obj[is[0]], is?.slice(1), value) | |
| } | |
| deepSet(state, payload.name, payload.value) | |
| } |
🤖 Prompt for AI Agents
In `@src/store/gui/mutations.ts` around lines 16 - 26, The deepSet helper inside
saveSetting creates intermediate objects even when payload.value is undefined,
leading to unintended state mutations; update saveSetting to guard early by
returning immediately when payload.value === undefined (so deletions are handled
by deleteSetting instead) or modify deepSet to not call Vue.set(...) to create
new nodes when value is undefined — ensure the check prevents creating empty
objects while still allowing valid assignments in saveSetting/deepSet.
Comment on lines
+187
to
+196
| const regexFilters = filters | ||
| .map((filter) => { | ||
| try { | ||
| return new RegExp(filter) | ||
| } catch { | ||
| logError('invalid console filter:', filter) | ||
| return null | ||
| } | ||
| }) | ||
| .filter((r): r is RegExp => r !== null) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file and the specific lines mentioned
head -200 src/store/server/actions.ts | tail -30Repository: mainsail-crew/mainsail
Length of output: 1455
🏁 Script executed:
# Also check lines 327-333
sed -n '320,340p' src/store/server/actions.tsRepository: mainsail-crew/mainsail
Length of output: 829
🏁 Script executed:
# Search for any existing regex validation or safe-regex imports
rg -i 'safe-regex|safeRegex|recheck' src/store/server/actions.tsRepository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Check if safe-regex is in dependencies
cat package.json | jq '.dependencies, .devDependencies' | grep -i 'safe\|regex'Repository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Search for how filters are used/applied throughout the file
rg 'filter' src/store/server/actions.ts -A 2 -B 2 | head -50Repository: mainsail-crew/mainsail
Length of output: 1497
🏁 Script executed:
# Find where console filters are set/validated
rg 'setConsolefilterRules|getConsolefilterRules' --type ts --type tsx -B 5 -A 5Repository: mainsail-crew/mainsail
Length of output: 93
🏁 Script executed:
# Look for the gui/console module to understand filter management
fd -e ts -e tsx gui | grep -i consoleRepository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Check the full context of how filters are persisted
rg 'ConsolefilterRules' --type ts --type tsx -B 2 -A 2Repository: mainsail-crew/mainsail
Length of output: 93
🏁 Script executed:
# Search for console filter rules without file type flag
rg 'setConsolefilterRules|getConsolefilterRules' -A 5 -B 5Repository: mainsail-crew/mainsail
Length of output: 2270
🏁 Script executed:
# Find files related to console module
fd -name '*.ts' -path '*/console*' -o -name 'console.ts'Repository: mainsail-crew/mainsail
Length of output: 296
🏁 Script executed:
# Search for where filters might be saved/validated
rg 'Consolefilter' -A 3 -B 3Repository: mainsail-crew/mainsail
Length of output: 2549
🏁 Script executed:
# Check for any RegExp validation patterns in the codebase
rg 'ReDoS|catastrophic|backtrack'Repository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Check the console getter implementation more fully
sed -n '1,100p' src/store/gui/console/getters.tsRepository: mainsail-crew/mainsail
Length of output: 1501
🏁 Script executed:
# Look for console mutations/actions where filters are saved
fd -type f -name '*.ts' -path '*/console/*' | head -20Repository: mainsail-crew/mainsail
Length of output: 236
🏁 Script executed:
# Search for where user-supplied filters are created or edited
rg 'consolefilters\[' src/store/gui/console/ -A 3 -B 3Repository: mainsail-crew/mainsail
Length of output: 2419
🏁 Script executed:
# Check if there are any other regex pattern compilations in the codebase that apply to user input
rg 'new RegExp' src/store/server/actions.ts -B 5 -A 5Repository: mainsail-crew/mainsail
Length of output: 896
🏁 Script executed:
# Check if safe-regex or similar is listed in package.json
cat package.json | grep -A 50 '"dependencies"' | grep -E 'safe|regex|redos'Repository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Look for any validation of regex patterns before they're used
rg 'test.*regex|regex.*test' src/store/gui/console/ -B 3 -A 3Repository: mainsail-crew/mainsail
Length of output: 48
Add ReDoS validation for user-supplied console filters to prevent UI lockups.
new RegExp(filter) is compiled from persisted user input and applied to every GCode event in high-frequency operations. A catastrophic regex pattern (e.g., (a+)+b) will compile without error but hang the UI during .test() execution. Current try-catch blocks only handle syntax errors, not catastrophic backtracking.
This occurs in two places:
- Line 187-196:
initGcodeStore— filters all stored gcode events - Line 327-333: socket event handler — filters every incoming message
Validate filter patterns at save time (preferred) or use a runtime safety check (e.g., safe-regex library) before regex compilation.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 189-189: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(filter)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
In `@src/store/server/actions.ts` around lines 187 - 196, The current creation of
regexFilters via new RegExp(filter) in initGcodeStore and the socket message
handler risks ReDoS from user-supplied patterns; validate patterns before
compiling by using a safe-regex check (e.g., add the safe-regex library or an
equivalent timeout/AST-based validator) when filters are persisted or
immediately before constructing RegExp in the arrays mapped to regexFilters and
the socket handler, reject or sanitize patterns that fail the safety check and
log the rejected pattern via logError (reference symbols: regexFilters, filters,
initGcodeStore and the socket event handler where incoming messages are
filtered) so only safe, pre-validated regexes are compiled and used during
high-frequency .test() calls.
Comment on lines
+101
to
+105
| setThrottledState(state, payload: ServerState['throttled_state'] | null) { | ||
| if (payload === null) return | ||
|
|
||
| setSystemBootAt(state, payload) { | ||
| Vue.set(state, 'system_boot_at', payload) | ||
| if ('bits' in payload) Vue.set(state.throttled_state, 'bits', payload.bits) | ||
| if ('flags' in payload) Vue.set(state.throttled_state, 'flags', payload.flags) |
There was a problem hiding this comment.
Null payload should probably clear throttled state.
Returning early on null can leave old bits/flags in place and misrepresent current throttling. If null means “no throttling info,” consider resetting to defaults.
🧹 Suggested reset on null
setThrottledState(state, payload: ServerState['throttled_state'] | null) {
- if (payload === null) return
+ if (payload === null) {
+ Vue.set(state.throttled_state, 'bits', 0)
+ Vue.set(state.throttled_state, 'flags', [])
+ return
+ }
if ('bits' in payload) Vue.set(state.throttled_state, 'bits', payload.bits)
if ('flags' in payload) Vue.set(state.throttled_state, 'flags', payload.flags)
},📝 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
| setThrottledState(state, payload: ServerState['throttled_state'] | null) { | |
| if (payload === null) return | |
| setSystemBootAt(state, payload) { | |
| Vue.set(state, 'system_boot_at', payload) | |
| if ('bits' in payload) Vue.set(state.throttled_state, 'bits', payload.bits) | |
| if ('flags' in payload) Vue.set(state.throttled_state, 'flags', payload.flags) | |
| setThrottledState(state, payload: ServerState['throttled_state'] | null) { | |
| if (payload === null) { | |
| Vue.set(state.throttled_state, 'bits', 0) | |
| Vue.set(state.throttled_state, 'flags', []) | |
| return | |
| } | |
| if ('bits' in payload) Vue.set(state.throttled_state, 'bits', payload.bits) | |
| if ('flags' in payload) Vue.set(state.throttled_state, 'flags', payload.flags) | |
| } |
🤖 Prompt for AI Agents
In `@src/store/server/mutations.ts` around lines 101 - 105, The early return in
setThrottledState when payload === null leaves stale values in
state.throttled_state; change the handler so that when payload is null it
explicitly resets/clears the throttled state (e.g.
Vue.set(state.throttled_state, 'bits', null/0/empty) and
Vue.set(state.throttled_state, 'flags', null/0/empty) or restores your chosen
defaults) and otherwise keep the existing conditional Vue.set updates for 'bits'
and 'flags'; update the function setThrottledState and the uses of
state.throttled_state to ensure the cleared default values represent "no
throttling info."
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.
Description
This PR refactor the complete gui/heightmap store and is based on #2421 .
Related Tickets & Documents
none
Mobile & Desktop Screenshots/Recordings
none
[optional] Are there any post-deployment tasks we need to perform?
none