Repository navigation
[DAS-Dashboard#1126] Update web configuration schema and migrate logic to back-end. - #296
Merged
Merged
Conversation
…eneral layout fixes
Use a single config.json under /opt/web-das/.das, centralize LOCAL_HOSTS, and add flat merge helpers so save/load/export share one shape with the UI.
Updated configuration preview
Added informative text for the context mapping paths configuration. Fixed context mapping paths bug when saving in 'path' mode.
…he front-end. Fixed reset button not working.
removed unused files and old logic.
Contributor
WalkthroughThe pull request adds a CodeRabbit configuration and replaces the dashboard’s file-upload configuration flow with DTO-based backend APIs and a new frontend configuration UI. It also introduces config builders, mappers, remote SCP utilities, workspace setup, and updated layout and export flows. ChangesCodeRabbit Integration Config
Dashboard Configuration System Overhaul
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ConfigurationProvider
participant ConfigAPI
participant config_controllers
participant ConfigServices
participant RemoteScpService
Browser->>ConfigurationProvider: fetch defaults and apply loaded config
ConfigurationProvider->>ConfigAPI: getConfigDefaults(), saveConfig(), loadConfig()
ConfigAPI->>config_controllers: GET /config/defaults, POST /config/save, POST /config/load
config_controllers->>ConfigServices: get_config_defaults(), save_config(), load_config()
ConfigServices->>ConfigServices: merge, build, persist, flatten
ConfigServices->>RemoteScpService: transfer_bytes() for remote export
RemoteScpService-->>ConfigServices: remote_path
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 24
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@das-dashboard/backend/controllers/config_controllers.py`:
- Around line 43-50: The export_config_scp function only catches ValueError
exceptions, but the underlying SSH, SCP, and subprocess operations can raise
paramiko exceptions, SCPException, and other transfer-related errors that
currently bubble up as generic 500 errors. Add additional exception handlers in
the try-except block of export_config_scp to catch connection failures (such as
paramiko.SSHException) and transfer failures (such as SCPException and
subprocess errors), then raise HTTPException with appropriate status codes (502
for connection issues, 400 for transfer issues) and clear user-facing error
messages that explain what failed.
In `@das-dashboard/backend/services/config_services.py`:
- Around line 72-85: The export_config_scp method makes two separate SSH
connections to the same host: once with self.remote_scp.connect to get the
remote home directory (which is closed in the finally block), and again inside
self.remote_scp.transfer_bytes to upload the file. Refactor to eliminate the
redundant connection by either passing the established ssh session to
transfer_bytes instead of just the ip, or by keeping the single connection open
and calling both get_remote_home and the file transfer operation within the same
connect-to-close block. This reduces unnecessary connection handshakes and
improves performance on the export path.
- Around line 61-92: The async method export_config_scp contains synchronous
blocking operations that run directly in the coroutine, blocking the event loop.
Wrap all blocking I/O calls with await run_in_threadpool() from
fastapi.concurrency: the self.remote_scp.connect() call,
self.remote_scp.get_remote_home() call, self.remote_scp.transfer_bytes() call,
and the set_das_cli_config() function call. Apply the same pattern to the
save_config and load_config methods which also contain blocking operations like
subprocess or SSH calls.
- Around line 119-126: When nested is a list and that list is empty, accessing
nested[0] raises an IndexError that is not caught by the except clause, causing
the endpoint to crash. Fix this by either guarding the index access with a check
to ensure the nested list is not empty before accessing nested[0], or add
IndexError to the tuple of caught exceptions on the except statement alongside
OSError, json.JSONDecodeError, TypeError, and ValueError to handle this case
gracefully.
In `@das-dashboard/backend/shared/builders/atom_db_builder.py`:
- Around line 142-156: The mongodb dictionary constructed in the
_build_mork_mongo method is missing the cluster_secret_key field that exists in
the _build_redis_mongo method, causing inconsistency in the mongodb schema
across different atomdb types. Add the cluster_secret_key field with a value of
None to the mongodb dictionary in _build_mork_mongo to match the structure
returned by _build_redis_mongo and ensure consistency with downstream validation
expectations.
In `@das-dashboard/backend/shared/internal/web_configuration.py`:
- Around line 61-73: The int(port) conversion in the _register_endpoint method
lacks error handling and will raise an uncaught ValueError if the port string
contains non-numeric characters, causing unhandled 500 errors during export
operations. Wrap the int(port) conversion in a try/except block to catch
ValueError exceptions, and default the port value to 0 when the conversion fails
due to non-numeric input.
In `@das-dashboard/backend/shared/mappers/nested_config_mapper.py`:
- Around line 25-27: The _environment_to_flat method and multiple other
locations in this file use inconsistent default hosts when creating endpoint
fallbacks, with some using localhost while canonical defaults use 0.0.0.0.
Define a single DEFAULT_HOST constant with the value 0.0.0.0 at the module
level, then replace all hardcoded localhost fallback values in the
_environment_to_flat method and all other endpoint construction calls throughout
the file (including the split_endpoint calls and similar patterns) with
references to this constant to ensure consistent default host behavior across
all config mapping operations.
In `@das-dashboard/backend/shared/utils/das_cli_config.py`:
- Around line 55-60: The subprocess.run call executing the das-cli command lacks
a timeout parameter, which can cause indefinite blocking if the CLI process
hangs. Add a timeout parameter to the subprocess.run call to prevent save/export
requests from blocking indefinitely. The timeout value should be appropriate for
the expected execution time of the das-cli command.
In `@das-dashboard/backend/shared/utils/flat_config_utils.py`:
- Around line 10-15: The merge_flat_config function has two issues: it only
checks if a section exists but doesn't merge missing keys from defaults into
partial sections, and it returns direct references to the mutable default
objects which can cause state leaks across requests. Fix this by implementing a
deep merge approach where for each key in FLAT_SECTION_ORDER, merge the current
section with the corresponding default section to populate missing keys, and
create defensive copies of the merged results instead of returning references to
the original defaults object.
In `@das-dashboard/backend/shared/utils/remote_scp.py`:
- Around line 29-31: The SSHClient connection in the SSH initialization block
uses AutoAddPolicy() which automatically accepts unknown host keys without
verification, creating a MITM attack vulnerability. Either replace AutoAddPolicy
with RejectPolicy and load a trusted known_hosts file for proper host key
verification, or if that's not possible, document in code comments why
auto-accepting host keys is an acceptable trust assumption for the DAS
architecture. Ensure the solution addresses the security concern while being
appropriate for your deployment model.
- Around line 39-60: The remote_file_exists and transfer_fileobj methods are
vulnerable to shell command injection because file_path and remote_dir are
interpolated directly into SSH shell commands without proper escaping. Import
shlex at the top of the file, then wrap each interpolated path variable with
shlex.quote() before it is used in the f-string passed to ssh.exec_command —
specifically quote file_path in the test -f command within remote_file_exists,
and quote remote_dir in the mkdir -p command within transfer_fileobj to prevent
malicious filenames from being executed as shell commands.
- Around line 26-33: The `ssh.connect()` call in the `connect` method is missing
a timeout parameter, which causes indefinite blocking when the host is
unreachable or unresponsive, blocking the event loop in FastAPI endpoints. Add
an explicit `timeout` parameter to the `ssh.connect()` method call with an
appropriate timeout value in seconds to prevent the connection from hanging
indefinitely.
In `@das-dashboard/scripts/start_interface.sh`:
- Around line 22-25: The docker rm commands in the script are removing
containers with outdated names (ui_backend and das-dashboard) while the script
creates containers with new names (web-interface-backend and
web-interface-frontend). Update the docker rm commands to remove the same
container names that are being created by the subsequent docker run commands.
Specifically, replace the container names in the cleanup sections at the
beginning of the script to match web-interface-backend and
web-interface-frontend so that previous containers are properly cleaned up
before re-running the script, preventing container-name conflicts.
In
`@das-dashboard/src/components/configuration_page/Agents/AgentConnectionFields.jsx`:
- Line 3: The endpoint IP field in AgentConnectionFields component is not
validating IPv4 addresses, allowing arbitrary text that produces invalid
endpoint values in agent payloads. Import an IPv4 validator from the same
formValidation module where portField is imported (look for a validator function
related to IPv4 addresses), then locate where the endpoint field is defined
around lines 9-16 and apply this IPv4 validator to ensure only valid IP
addresses are accepted.
In `@das-dashboard/src/components/configuration_page/Agents/LinkCreation.jsx`:
- Line 36: The updateField function in ConfigurationProvider.jsx performs only a
shallow assignment, so passing a nested key path like "agents.link_creation"
creates a flat key instead of properly nesting it in the config object. Modify
the updateField function implementation (around Lines 46-51 in
ConfigurationProvider.jsx) to handle nested key paths by splitting the key on
dots and creating the proper nested structure, so that "agents.link_creation"
properly updates config.agents.link_creation instead of creating a flat
config["agents.link_creation"] key.
In `@das-dashboard/src/components/configuration_page/AtomDB/ClusterForm.jsx`:
- Around line 67-121: The map function iterating over nodes uses the array index
i as the React key, which causes component state confusion when rows are deleted
via removeServer. Replace key={i} with a stable, unique identifier for each node
(such as a persistent node ID or UUID). If nodes lack unique IDs, generate and
store a unique identifier when each node is created and added to nodes.current,
then use that identifier as the key instead of the array index to ensure form
values remain correctly bound to their respective nodes.
In
`@das-dashboard/src/components/configuration_page/AtomDB/RemoteDB/MorkMongoSubForm.jsx`:
- Around line 47-50: When the port input field is cleared, Number("") coerces to
0, which stores an invalid port value. In the onChange handler for the mork_port
input field, validate that the input value is not empty before converting it to
a number using Number(). If the input is empty, either skip the update or set
form.current.mork_port to a valid default value (like null or undefined). Apply
the same validation logic to the other port input field mentioned at lines 78-80
to ensure consistency across all port inputs.
In
`@das-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RedisMongoSubForm.jsx`:
- Around line 47-50: The onChange handler for the redis_port field uses
Number(e.target.value) which converts empty strings to 0, storing invalid port
values in form.current.redis_port. Add a conditional check in the handler to
only convert to a number if the input value is not empty, otherwise store an
empty string or appropriate empty value. This same issue also applies to the
similar port field handler around lines 78-80, so apply the same fix pattern to
both occurrences.
In
`@das-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RemoteDB.jsx`:
- Around line 82-87: The filter function for validating peers does not check the
local_persistence field for "redismongodb" and "morkdb" types. When these types
are selected, local_persistence is initialized as an empty object and validation
only checks main connection fields like redis_port, mongo_port, or mork_port,
allowing invalid local payloads to be persisted. Extend the filter logic to also
validate that the local_persistence object contains required fields (redis_port,
mongo_port, or mork_port as appropriate) when peer.type is "redismongodb" or
"morkdb", rejecting the peer if these nested fields are missing or empty.
- Around line 33-46: The id variable using Date.now() and uid variable using
peers.length + 1 are not stable unique identifiers and can cause collisions and
reuse after peer removals. Replace the Date.now() call with a proper unique
identifier generator such as a UUID or nanoid library. Additionally, replace the
peers.length + 1 approach for uid with the same unique identifier generator or
use a persistent counter that increments regardless of peer removals, ensuring
peersRefs.current keys remain truly unique and do not get overwritten when peers
are added after removals or in rapid succession.
In `@das-dashboard/src/components/configuration_page/ConfigurationPreview.jsx`:
- Around line 105-131: The formatPreviewValue function currently renders
sensitive password fields like mongo_password and db_password as plain text,
creating a security risk. Add a check at the beginning of the formatPreviewValue
function to detect if the key matches sensitive field names like mongo_password
or db_password, and return a masked value such as asterisks or dots instead of
the actual value, before processing any other formatting logic.
In `@das-dashboard/src/components/configuration_page/ExportConfigDialog.jsx`:
- Around line 29-55: Remove `showToast` from the dependency array of the
useEffect hook in the ExportConfigDialog component. Change the dependency array
from `[open, flatConfig, showToast]` to `[open, flatConfig]` since showToast is
recreated on every render and causes the effect to retrigger unnecessarily,
creating a re-fetch loop when errors occur. Since showToast is only called in
the error handler and does not need to be tracked for identity changes,
excluding it from the dependencies prevents the cascading effect retriggers.
In `@das-dashboard/src/components/configuration_page/formValidation.js`:
- Line 2: The IPv4 pattern validation in formValidation.js is too permissive and
accepts invalid octet values (0-255 range). Replace the current pattern that
simply matches any 1-3 digits with a stricter regex pattern that validates each
octet is in the correct range of 0-255 by accounting for three ranges: 250-255
(25X where X is 0-5), 200-249 (2X where X is 0-4 followed by any digit), and
0-199 (optional leading 0 or 1 followed by 1-2 digits). Apply this validation
pattern to each of the four octets separated by dots to ensure only valid IPv4
addresses pass validation.
In `@das-dashboard/src/pages/setup_das/SetupDas.jsx`:
- Around line 84-87: The catch blocks in SetupDas.jsx are displaying generic
error messages to users without including the actual error details from the
backend. Extract the error message from the error object caught in each
exception handler (the ones at lines 84-87, 96-99, and 289-292) and include it
in the showToast message parameter. Access the error message using error.message
or error.response?.data?.message if the error is structured from an API
response, and concatenate it with the generic message so users see what actually
went wrong on the server side.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d208727e-1bc5-4a2d-9ffc-30d74d84340d
⛔ Files ignored due to path filters (1)
das-dashboard/public/icons.svgis excluded by!**/*.svg
📒 Files selected for processing (97)
.coderabbit.yamldas-dashboard/backend/Dockerfiledas-dashboard/backend/controllers/config_controllers.pydas-dashboard/backend/main.pydas-dashboard/backend/services/config_services.pydas-dashboard/backend/services/container_services.pydas-dashboard/backend/services/database_services.pydas-dashboard/backend/services/metrics_services.pydas-dashboard/backend/services/workspace_services.pydas-dashboard/backend/services_init.pydas-dashboard/backend/shared/builders/agents_builder.pydas-dashboard/backend/shared/builders/atom_db_builder.pydas-dashboard/backend/shared/builders/builder_helpers.pydas-dashboard/backend/shared/builders/environment_builder.pydas-dashboard/backend/shared/builders/loaders_builder.pydas-dashboard/backend/shared/dtos/configuration_entries_dto.pydas-dashboard/backend/shared/dtos/dashboard_action_dto.pydas-dashboard/backend/shared/dtos/dashboard_get_metrics_dto.pydas-dashboard/backend/shared/dtos/dashboard_profile_dto.pydas-dashboard/backend/shared/internal/configuration_constants.pydas-dashboard/backend/shared/internal/constants.pydas-dashboard/backend/shared/internal/web_configuration.pydas-dashboard/backend/shared/mappers/das_config_mapper.pydas-dashboard/backend/shared/mappers/nested_config_mapper.pydas-dashboard/backend/shared/utils/adapter_context_mapping.pydas-dashboard/backend/shared/utils/das_cli_config.pydas-dashboard/backend/shared/utils/flat_config_utils.pydas-dashboard/backend/shared/utils/path_utils.pydas-dashboard/backend/shared/utils/remote_scp.pydas-dashboard/package.jsondas-dashboard/scripts/start_interface.shdas-dashboard/src/App.jsxdas-dashboard/src/api/APIUtils.jsdas-dashboard/src/api/ConfigAPI.jsdas-dashboard/src/assets/default_json.jsxdas-dashboard/src/components/configuration_page/Agents/AgentConnectionFields.jsxdas-dashboard/src/components/configuration_page/Agents/Agents.jsxdas-dashboard/src/components/configuration_page/Agents/Agents.styled.jsdas-dashboard/src/components/configuration_page/Agents/AgentsParams/BaseParams.jsxdas-dashboard/src/components/configuration_page/Agents/AgentsParams/ContextParams.jsxdas-dashboard/src/components/configuration_page/Agents/AgentsParams/EvolutionParams.jsxdas-dashboard/src/components/configuration_page/Agents/AgentsParams/InferenceParams.jsxdas-dashboard/src/components/configuration_page/Agents/AgentsParams/LinkCreationParams.jsxdas-dashboard/src/components/configuration_page/Agents/AgentsParams/QueryParams.jsxdas-dashboard/src/components/configuration_page/Agents/AtomDbBroker.jsxdas-dashboard/src/components/configuration_page/Agents/AttentionBroker.jsxdas-dashboard/src/components/configuration_page/Agents/BaseParams.jsxdas-dashboard/src/components/configuration_page/Agents/CommandRouter.jsxdas-dashboard/src/components/configuration_page/Agents/ContextBroker.jsxdas-dashboard/src/components/configuration_page/Agents/EvolutionAgent.jsxdas-dashboard/src/components/configuration_page/Agents/InferenceAgent.jsxdas-dashboard/src/components/configuration_page/Agents/LinkCreation.jsxdas-dashboard/src/components/configuration_page/Agents/QueryAgent.jsxdas-dashboard/src/components/configuration_page/Agents/agentRegistry.jsdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/AdapterBackendOptions.jsxdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/AdapterDB.jsxdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/adapterConstants.jsdas-dashboard/src/components/configuration_page/AtomDB/AtomDB.jsxdas-dashboard/src/components/configuration_page/AtomDB/AtomDBStyled.jsdas-dashboard/src/components/configuration_page/AtomDB/ClusterForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/InMemory/InMemory.jsxdas-dashboard/src/components/configuration_page/AtomDB/MorkMongo/MorkMongo.jsxdas-dashboard/src/components/configuration_page/AtomDB/RedisMongo/RedisMongo.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/MorkMongoSubForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RedisMongoSubForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RemoteDB.jsxdas-dashboard/src/components/configuration_page/ConfigForm.jsxdas-dashboard/src/components/configuration_page/ConfigurationPreview.jsxdas-dashboard/src/components/configuration_page/ConfigurationPreview.styled.jsdas-dashboard/src/components/configuration_page/Environment/Environment.jsxdas-dashboard/src/components/configuration_page/ExportConfigDialog.jsxdas-dashboard/src/components/configuration_page/configFormUtils.jsdas-dashboard/src/components/configuration_page/formValidation.jsdas-dashboard/src/components/dashboard/Dialogs/DialogBoxes.jsxdas-dashboard/src/components/dashboard/MainContent/sidebar/SideBar.jsxdas-dashboard/src/components/dashboard/MainContent/sidebar/sidebar.styled.jsdas-dashboard/src/components/form_parts/Agents/Agents.cssdas-dashboard/src/components/form_parts/Agents/Agents.jsxdas-dashboard/src/components/form_parts/AgentsParams/AgentsParams.jsxdas-dashboard/src/components/form_parts/AtomDB/AtomDB.jsxdas-dashboard/src/components/form_parts/AtomDB/ClusterForm.jsxdas-dashboard/src/components/form_parts/AtomDB/InMemory.jsxdas-dashboard/src/components/form_parts/AtomDB/MorkMongo.jsxdas-dashboard/src/components/form_parts/AtomDB/RedisMongo.jsxdas-dashboard/src/components/form_parts/AtomDB/RemoteDB/MorkMongoSubForm.jsxdas-dashboard/src/components/form_parts/AtomDB/RemoteDB/RedisMongoSubForm.jsxdas-dashboard/src/components/form_parts/AtomDB/RemoteDB/RemoteDB.jsxdas-dashboard/src/components/form_parts/Brokers/Brokers.jsxdas-dashboard/src/components/form_parts/Environment/Environment.jsxdas-dashboard/src/components/global_providers/ConfigurationProvider.jsxdas-dashboard/src/components/top_nav_bar/NavBar.jsxdas-dashboard/src/index.cssdas-dashboard/src/pages/dashboard/Dashboard.jsxdas-dashboard/src/pages/profile/profilepage.styled.jsdas-dashboard/src/pages/setup_das/SetupDas.cssdas-dashboard/src/pages/setup_das/SetupDas.jsxdas-dashboard/src/pages/setup_das/SetupDasStyled.js
💤 Files with no reviewable changes (18)
- das-dashboard/src/components/form_parts/Agents/Agents.jsx
- das-dashboard/src/components/dashboard/Dialogs/DialogBoxes.jsx
- das-dashboard/src/components/form_parts/AtomDB/InMemory.jsx
- das-dashboard/src/components/form_parts/AtomDB/ClusterForm.jsx
- das-dashboard/src/components/form_parts/AgentsParams/AgentsParams.jsx
- das-dashboard/backend/shared/dtos/dashboard_get_metrics_dto.py
- das-dashboard/src/components/form_parts/Environment/Environment.jsx
- das-dashboard/backend/shared/dtos/dashboard_profile_dto.py
- das-dashboard/src/components/form_parts/AtomDB/RemoteDB/RedisMongoSubForm.jsx
- das-dashboard/backend/shared/dtos/dashboard_action_dto.py
- das-dashboard/src/pages/setup_das/SetupDas.css
- das-dashboard/src/components/form_parts/AtomDB/RedisMongo.jsx
- das-dashboard/src/components/form_parts/AtomDB/MorkMongo.jsx
- das-dashboard/src/components/form_parts/AtomDB/RemoteDB/RemoteDB.jsx
- das-dashboard/src/assets/default_json.jsx
- das-dashboard/src/components/form_parts/Brokers/Brokers.jsx
- das-dashboard/src/components/form_parts/AtomDB/AtomDB.jsx
- das-dashboard/src/components/form_parts/AtomDB/RemoteDB/MorkMongoSubForm.jsx
- enhanced exception handling for SSH/SCP methods and implemented HTTP exception handling for the API.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
das-dashboard/backend/services/database_services.py (1)
92-98: 🩺 Stability & Availability | 🟠 MajorAdd a timeout to
das-dashboard/backend/services/database_services.py::load_metta_file_into_db. Thissubprocess.run()is reached from the/services/atomdb/metta/loadAPI, so a stalleddas-cli metta loadcan block the request indefinitely.timeout=300is a reasonable guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/backend/services/database_services.py` around lines 92 - 98, The subprocess call in load_metta_file_into_db can hang indefinitely when das-cli metta load stalls, blocking the /services/atomdb/metta/load request. Update the subprocess.run invocation in load_metta_file_into_db to include a timeout guard of 300 seconds, and ensure any timeout handling in that function preserves the existing error flow and logging.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@das-dashboard/backend/services/config_services.py`:
- Around line 71-92: The cleanup in this config upload flow can mask the real
error because `ssh.close()` is always called in `finally` even when `ip` is
invalid or `self.remote_scp.connect(ip)` fails before `ssh` is assigned. Update
the logic in the `config_services` method that uses `known_ips`, `connect()`,
and `transfer_fileobj` so `ssh` is initialized safely and only closed if it was
successfully created. Keep the existing `CustomValueError` handling for invalid
IPs and let connection failures surface without being replaced by an
`UnboundLocalError`.
In `@das-dashboard/backend/services/database_services.py`:
- Around line 34-46: Reject client-controlled path separators in
UploadFile.filename before using it in database_services upload flow, since
remote_save_path currently concatenates the filename directly and allows
traversal outside DEFAULT_METTA_FILES_PATH or the remote Metta directory. Update
the upload handling around the remote_save_path construction and SCPClient.putfo
path usage to validate or sanitize filenames so only a safe base name is
accepted, and apply the same protection in the related upload path logic in the
same module.
In `@das-dashboard/backend/shared/utils/remote_scp.py`:
- Around line 78-83: The remote SSH helpers are not waiting for command
completion or checking exit status, so `get_remote_home()` and
`ensure_remote_dir()` can silently fail before SCP uses them. Update the
`RemoteScp` methods to use a checked exec helper for the `ssh.exec_command`
calls, verify the command finishes successfully, and raise
`RemoteSshTransferError` when `echo $HOME` or `mkdir -p` fails so callers never
proceed with an invalid path or uncreated directory.
---
Outside diff comments:
In `@das-dashboard/backend/services/database_services.py`:
- Around line 92-98: The subprocess call in load_metta_file_into_db can hang
indefinitely when das-cli metta load stalls, blocking the
/services/atomdb/metta/load request. Update the subprocess.run invocation in
load_metta_file_into_db to include a timeout guard of 300 seconds, and ensure
any timeout handling in that function preserves the existing error flow and
logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6942cac5-38ca-479e-acef-512ee321300a
📒 Files selected for processing (6)
das-dashboard/backend/controllers/config_controllers.pydas-dashboard/backend/services/config_services.pydas-dashboard/backend/services/database_services.pydas-dashboard/backend/shared/exceptions/custom_exceptions.pydas-dashboard/backend/shared/exceptions/exception_handlers.pydas-dashboard/backend/shared/utils/remote_scp.py
- Added threaded execution to SCP/SSH methods to avoid congestioning the API. - Small fixes to adapterdb mongodb subsection. - Reworked default dictionary merging to fill missing sub-keys. - Added better treatment to metta file uploads
- fixed wrong container names on startup script
- Added better parsing and validation for port values - Added better peerId formation on remoteDB section. More secure localPersistence saving. - Better error handling for toasts and API error messages
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
das-dashboard/backend/services/config_services.py (1)
43-54: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExport the saved config when the request body is omitted.
The controller allows an optional body, but this fallback exports
CONSTANTSinstead of the savedCONFIG_PATH. That makes/config/export/targetsand/config/export/scp/{ip}ignore previously saved remote targets unless the frontend always reposts the full form.Suggested fix
if configuration_entries is not None: flat_payload = configuration_entries.model_dump(by_alias=True, exclude_none=True) if flat_payload: flat = merge_flat_config(flat_payload, CONSTANTS) return ConfigMapper.build_config(flat) + if os.path.exists(CONFIG_PATH): + try: + with open(CONFIG_PATH, "r", encoding="utf-8") as config_file: + saved = json.load(config_file) + + if isinstance(saved, list) and saved: + saved = saved[0] + + if isinstance(saved, dict): + return saved + except (OSError, json.JSONDecodeError, TypeError, ValueError, IndexError): + pass + flat = merge_flat_config({}, CONSTANTS) return ConfigMapper.build_config(flat)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/backend/services/config_services.py` around lines 43 - 54, The fallback in _build_export_config currently builds exports from CONSTANTS when configuration_entries is omitted, so saved targets in CONFIG_PATH are ignored. Update the no-body path to load and merge the persisted config source used by the config service instead of starting from an empty payload, while keeping the existing configuration_entries handling and ConfigMapper.build_config flow intact. Ensure /config/export/targets and /config/export/scp/{ip} can export previously saved remote targets even when the request body is absent.das-dashboard/backend/shared/internal/web_configuration.py (1)
81-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid service-label collisions for nested AtomDB backends.
_map_remote_peerand adapter mapping pass uniquedb_namevalues, but these helpers still register Redis/Mork as"redis"/"morkdb". Later nested backends overwrite earlier entries inservices, somap_hosts()can drop export targets.Suggested fix
`@classmethod` def _map_redis_mongo(cls, services: dict, section: dict, *, db_name: str = "db") -> None: redis = section.get("redis") or {} mongo = section.get("mongodb") or {} + redis_name = "redis" if db_name == "db" else f"{db_name}-redis" cls._register_endpoint(services, db_name, mongo.get("endpoint")) - cls._register_endpoint(services, "redis", redis.get("endpoint")) + cls._register_endpoint(services, redis_name, redis.get("endpoint")) `@classmethod` def _map_mork_mongo(cls, services: dict, section: dict, *, db_name: str = "db") -> None: mongo = section.get("mongodb") or {} mork = section.get("morkdb") or {} + mork_name = "morkdb" if db_name == "db" else f"{db_name}-morkdb" cls._register_endpoint(services, db_name, mongo.get("endpoint")) - cls._register_endpoint(services, "morkdb", mork.get("endpoint")) + cls._register_endpoint(services, mork_name, mork.get("endpoint"))Also applies to: 125-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/backend/shared/internal/web_configuration.py` around lines 81 - 94, The nested backend mappers are still registering Redis and Mork under fixed labels, which can overwrite other entries in services and cause map_hosts() to lose export targets. Update _map_redis_mongo and _map_mork_mongo to use the provided db_name (or another unique per-adapter label) consistently when calling _register_endpoint, and ensure _map_remote_peer and the adapter mapping paths pass distinct names so each nested AtomDB backend keeps its own service entry.das-dashboard/backend/services/database_services.py (1)
59-79: 🩺 Stability & Availability | 🟠 MajorOffload the remote SCP upload path
save_metta_file()is async, but the remote branch calls synchronous_transfer_file_scp()directly, and the controller awaits this method. That means the SCP/SSH transfer blocks the event loop for the full upload. Run this path inrun_in_threadpool/asyncio.to_threadand add a regression test for the remote save flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/backend/services/database_services.py` around lines 59 - 79, The async save_metta_file flow blocks the event loop because the remote host branch calls the synchronous _transfer_file_scp() directly. Update save_metta_file in DatabaseServices to offload the non-LOCAL_HOSTS path with run_in_threadpool or asyncio.to_thread while preserving the existing host, file, and force_overwrite arguments, and add a regression test covering the remote save flow to verify the controller await no longer runs the SCP transfer inline.das-dashboard/backend/shared/builders/atom_db_builder.py (1)
312-318: 🎯 Functional Correctness | 🟡 MinorMake
export_metta_output_dirconsistently required or optional_require(atomdb, *self._ADAPTER_FIELDS, ...)already enforces this field, so the""fallback never runs. Either drop the default or remove the field from_ADAPTER_FIELDSand handle the disabled-export case explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@das-dashboard/backend/shared/builders/atom_db_builder.py` around lines 312 - 318, The _build_adapter_db method currently treats export_metta_output_dir as optional with a default fallback, but _require(atomdb, *self._ADAPTER_FIELDS, label="atomdb") already makes it required, so the fallback is dead code. Update AtomDbBuilder so export_metta_output_dir is handled consistently by either keeping it in _ADAPTER_FIELDS and removing the default from _get, or removing it from _ADAPTER_FIELDS and explicitly handling the disabled export path in _build_adapter_db.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@das-dashboard/backend/services/config_services.py`:
- Around line 122-123: The config load flow currently persists `nested_config`
before `NestedConfigMapper.to_flat()` and `merge_flat_config(...)` validate it,
which can leave `CONFIG_PATH` updated even when the config is malformed. Reorder
the logic in the config loading method so flattening and merging happen before
`_persist_config`, and only call `run_in_threadpool(self._persist_config,
nested_config)` after the config has successfully passed
`NestedConfigMapper.to_flat()` and `merge_flat_config`.
---
Outside diff comments:
In `@das-dashboard/backend/services/config_services.py`:
- Around line 43-54: The fallback in _build_export_config currently builds
exports from CONSTANTS when configuration_entries is omitted, so saved targets
in CONFIG_PATH are ignored. Update the no-body path to load and merge the
persisted config source used by the config service instead of starting from an
empty payload, while keeping the existing configuration_entries handling and
ConfigMapper.build_config flow intact. Ensure /config/export/targets and
/config/export/scp/{ip} can export previously saved remote targets even when the
request body is absent.
In `@das-dashboard/backend/services/database_services.py`:
- Around line 59-79: The async save_metta_file flow blocks the event loop
because the remote host branch calls the synchronous _transfer_file_scp()
directly. Update save_metta_file in DatabaseServices to offload the
non-LOCAL_HOSTS path with run_in_threadpool or asyncio.to_thread while
preserving the existing host, file, and force_overwrite arguments, and add a
regression test covering the remote save flow to verify the controller await no
longer runs the SCP transfer inline.
In `@das-dashboard/backend/shared/builders/atom_db_builder.py`:
- Around line 312-318: The _build_adapter_db method currently treats
export_metta_output_dir as optional with a default fallback, but
_require(atomdb, *self._ADAPTER_FIELDS, label="atomdb") already makes it
required, so the fallback is dead code. Update AtomDbBuilder so
export_metta_output_dir is handled consistently by either keeping it in
_ADAPTER_FIELDS and removing the default from _get, or removing it from
_ADAPTER_FIELDS and explicitly handling the disabled export path in
_build_adapter_db.
In `@das-dashboard/backend/shared/internal/web_configuration.py`:
- Around line 81-94: The nested backend mappers are still registering Redis and
Mork under fixed labels, which can overwrite other entries in services and cause
map_hosts() to lose export targets. Update _map_redis_mongo and _map_mork_mongo
to use the provided db_name (or another unique per-adapter label) consistently
when calling _register_endpoint, and ensure _map_remote_peer and the adapter
mapping paths pass distinct names so each nested AtomDB backend keeps its own
service entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 36ec6a20-57b0-4d1c-9f11-4678ae5a92ec
📒 Files selected for processing (24)
das-dashboard/backend/services/config_services.pydas-dashboard/backend/services/database_services.pydas-dashboard/backend/shared/builders/atom_db_builder.pydas-dashboard/backend/shared/internal/web_configuration.pydas-dashboard/backend/shared/mappers/nested_config_mapper.pydas-dashboard/backend/shared/utils/das_cli_config.pydas-dashboard/backend/shared/utils/flat_config_utils.pydas-dashboard/backend/shared/utils/remote_scp.pydas-dashboard/backend/shared/utils/upload_utils.pydas-dashboard/scripts/start_interface.shdas-dashboard/src/components/configuration_page/Agents/AgentConnectionFields.jsxdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/AdapterBackendOptions.jsxdas-dashboard/src/components/configuration_page/AtomDB/AdapterDB/AdapterDB.jsxdas-dashboard/src/components/configuration_page/AtomDB/ClusterForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/MorkMongo/MorkMongo.jsxdas-dashboard/src/components/configuration_page/AtomDB/RedisMongo/RedisMongo.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/MorkMongoSubForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RedisMongoSubForm.jsxdas-dashboard/src/components/configuration_page/AtomDB/RemoteDB/RemoteDB.jsxdas-dashboard/src/components/configuration_page/Environment/Environment.jsxdas-dashboard/src/components/configuration_page/ExportConfigDialog.jsxdas-dashboard/src/components/configuration_page/configFormUtils.jsdas-dashboard/src/components/configuration_page/formValidation.jsdas-dashboard/src/pages/setup_das/SetupDas.jsx
andre-senna
approved these changes
Jun 26, 2026
This was referenced Aug 7, 2026
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
3 participants
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.
Refactored the Configuration page on the dashboard.
All existing features have been preserved and moved to the back end, where the heavy processing is now handled.
The front end and back end communicate through a defined data contract. The front end sends a flat map containing all configuration keys and values, and the back end is responsible for assembling the final nested configuration JSON.
The back end also provides all default values, identifies available IPs for configuration export, maps the configuration, and stores it internally for use by the dashboard.
New Features