Conversation
|
Follow-up fix pushed for the blank window from scripts/start-all.sh. Root cause: dist was rebuilt, but the existing Tauri binary still embedded the old frontend assets. cargo build did not rerun because src-tauri/build.rs did not track ../dist or tauri.conf.json as inputs. The script also only checked that dist/index.html existed, so it could launch a stale binary. Fix:
Verification:
|
|
Follow-up pushed for script behavior requested after testing start-all.sh. Changes:
Validation:
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR completes “Phase 10” client UI moderation by upgrading the device browser UX (list/grid controls + split detail workspace), adding batch group/tag operations, enabling local JSON/ZIP UI template imports, and improving script-based operational workflows (bundle-based desktop launch, IoT simulation, stop/status helpers). It also adds MQTT status publishing in StationService and client-side MQTT discovery/runtime updates via the Aggregator bridge.
Changes:
- Add stop/status tooling and an IoT simulation launcher (generated docker compose + Mosquitto + N simulated stations).
- Rework CC-rClient device browser UI (view modes, column controls, filtering/sorting by group/tag/value, two-pane layout) plus group/tag batch operations and related tests.
- Enable local JSON/ZIP UI template imports and run the desktop app using bundled assets (reduced reliance on Vite dev server).
Reviewed changes
Copilot reviewed 36 out of 41 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/stop-all.sh | New stop/status script to terminate host components and optionally stop broker / IoT sim. |
| scripts/start-iot-sim.sh | New launcher to generate docker-compose, build a lightweight image, and run N simulated devices. |
| scripts/start-all.sh | Switch default to release; start client via Tauri flows; print stop commands; IoT sim status detection. |
| scripts/mosquitto.iot-sim.conf | Mosquitto config for embedded broker in IoT sim. |
| scripts/build-all.sh | Build client via tauri:build scripts instead of separate Vite + cargo steps. |
| scripts/README.md | Document stop-all.sh usage and new stop flows. |
| next_task.md | Mark Phase 10 tasks completed. |
| README.md | Document IoT simulation launcher and generated compose output. |
| CC_project_tasks.md | Update phase tracking/status and roadmap table. |
| CC-rStationService/src/state.rs | Add mqtt_status_enabled configuration helper. |
| CC-rStationService/src/app.rs | Add MQTT status publishing loop guarded by config. |
| CC-rStationService/packaging/docker/iot-sim-entrypoint.sh | Entrypoint to render config template and run stationservice in containers. |
| CC-rStationService/packaging/docker/Dockerfile.iot-sim | Minimal runtime image for IoT sim stations. |
| CC-rStationService/packaging/docker/CC-rStationService.iot.toml.template | Config template injected into IoT sim container. |
| CC-rStationService/README.md | Document IoT simulation container usage. |
| CC-rStationService/.dockerignore | Docker ignore rules for stationservice context (general). |
| CC-rClient/src/test/template-marketplace.test.ts | Tests for JSON/ZIP template file imports. |
| CC-rClient/src/test/tags-page.test.tsx | Tests for tag multi-device picker filtering and batch apply. |
| CC-rClient/src/test/stations-browser.test.ts | Tests for new station filtering/sorting/tag summary helpers. |
| CC-rClient/src/test/setup.ts | Ensure RTL cleanup after each test. |
| CC-rClient/src/test/group-card.test.tsx | Tests for group batch add filtered selection. |
| CC-rClient/src/styles.css | New stations workspace layout + banner behavior tweaks. |
| CC-rClient/src/stations-browser.ts | New filtering/sorting helpers for device browser UI. |
| CC-rClient/src/plugin/groups/TagsPage.tsx | Add multi-device selection/picker and batch tag assignment UI. |
| CC-rClient/src/plugin/groups/TagsPage.test.tsx | Update existing tags tests to match device picker flow/text. |
| CC-rClient/src/plugin/groups/GroupsPage.tsx | Wire batch add callback into group list/cards. |
| CC-rClient/src/plugin/groups/GroupsContext.tsx | Add batchAddStationsToGroup helper in context. |
| CC-rClient/src/plugin/groups/GroupEditor.tsx | Text updates (“Stations” → “Devices”). |
| CC-rClient/src/plugin/groups/GroupCard.tsx | Add filtered multi-select + batch add UI for group membership. |
| CC-rClient/src/plugin/components/TemplateMarketplace.tsx | Add ZIP parsing/import support alongside JSON file import. |
| CC-rClient/src/App.tsx | Major device browser UX refresh + MQTT event listeners + banner updates. |
| CC-rClient/src-tauri/src/lib.rs | Start MQTT WS bridge on app setup (remove global lazy singleton). |
| CC-rClient/src-tauri/build.rs | Rebuild triggers for tauri config and bundled dist assets. |
| CC-rClient/package.json | Add tauri:* and desktop:* scripts for dev/build/run. |
| CC-rClient/README.md | Document new build/run flows for bundled assets. |
| CC-Aggregator/src/main.rs | Adjust MQTT broadcast forwarding logic. |
| .gitignore | Ignore generated IoT sim logs/compose under logs/iot-sim. |
Comments suppressed due to low confidence (4)
scripts/stop-all.sh:1
- The stop logic assumes the broker container is named exactly
mosquitto, butstart-iot-sim.shgeneratescontainer_name: ${PROJECT_NAME}-mosquitto(e.g.cc-iot-sim-mosquitto). As-is,--brokerwon’t stop the embedded sim broker and the status check may be misleading. Fix by stopping/checking the IoT sim broker name (e.g.cc-iot-sim-mosquitto) and/or checking both names (and consider usingdocker compose -p cc-iot-sim downas the canonical stop for embedded broker).
scripts/stop-all.sh:1 - The stop logic assumes the broker container is named exactly
mosquitto, butstart-iot-sim.shgeneratescontainer_name: ${PROJECT_NAME}-mosquitto(e.g.cc-iot-sim-mosquitto). As-is,--brokerwon’t stop the embedded sim broker and the status check may be misleading. Fix by stopping/checking the IoT sim broker name (e.g.cc-iot-sim-mosquitto) and/or checking both names (and consider usingdocker compose -p cc-iot-sim downas the canonical stop for embedded broker).
scripts/stop-all.sh:1 - These process match patterns won’t catch the new debug workflow (
npm run tauri:dev/tauri dev). That meansstop-all.shand--statuscan miss (and fail to stop) the dev runner you now start instart-all.sh. Update the patterns to also includenpm run tauri:devand/ortauri dev.
scripts/stop-all.sh:1 - These process match patterns won’t catch the new debug workflow (
npm run tauri:dev/tauri dev). That meansstop-all.shand--statuscan miss (and fail to stop) the dev runner you now start instart-all.sh. Update the patterns to also includenpm run tauri:devand/ortauri dev.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| const unlistenFns: UnlistenFn[] = []; | ||
|
|
||
| const registerListeners = async () => { | ||
| const unlistenTelemetry = await listen<MqttTelemetryEventPayload>("telemetry", (event) => { | ||
| const { station_id: stationId, data } = event.payload; | ||
| const runtime = buildRuntimeFromMqttTelemetry(stationId, data); | ||
|
|
||
| setStations((current) => { | ||
| if (current.some((station) => station.id === stationId)) { | ||
| return current.map((station) => | ||
| station.id === stationId | ||
| ? { | ||
| ...station, | ||
| metadata: { | ||
| ...station.metadata, | ||
| source: station.metadata.source || MQTT_DISCOVERY_SOURCE, | ||
| mqttLastSeen: String(data.ts), | ||
| }, | ||
| } | ||
| : station, | ||
| ); | ||
| } | ||
|
|
||
| return [ | ||
| createDiscoveredStation(stationId, { | ||
| mqttLastSeen: String(data.ts), | ||
| }), | ||
| ...current, | ||
| ]; | ||
| }); | ||
|
|
||
| setRuntimeByStation((current) => ({ ...current, [stationId]: runtime })); | ||
| setHistoryByStation((current) => { | ||
| const prev = current[stationId] ?? []; | ||
| const next = [ | ||
| ...prev.slice(-(MAX_HISTORY - 1)), | ||
| { | ||
| cpu: runtime.cpu, | ||
| memory: runtime.currentMemory, | ||
| ts: Date.now(), | ||
| rxbps: 0, | ||
| txbps: 0, | ||
| }, | ||
| ]; | ||
| return { ...current, [stationId]: next }; | ||
| }); | ||
| }); | ||
| unlistenFns.push(unlistenTelemetry); | ||
|
|
||
| const unlistenStatus = await listen<MqttStatusEventPayload>("station-status", (event) => { | ||
| const { station_id: stationId, status } = event.payload; | ||
|
|
||
| setStations((current) => { | ||
| const existing = current.find((station) => station.id === stationId); | ||
| if (!existing) { | ||
| return [ | ||
| createDiscoveredStation(stationId, { | ||
| mqttOnline: String(status.online), | ||
| mqttLastSeen: String(status.last_seen), | ||
| }), | ||
| ...current, | ||
| ]; | ||
| } | ||
|
|
||
| return current.map((station) => | ||
| station.id === stationId | ||
| ? { | ||
| ...station, | ||
| metadata: { | ||
| ...station.metadata, | ||
| source: station.metadata.source || MQTT_DISCOVERY_SOURCE, | ||
| mqttOnline: String(status.online), | ||
| mqttLastSeen: String(status.last_seen), | ||
| }, | ||
| } | ||
| : station, | ||
| ); | ||
| }); | ||
|
|
||
| if (!status.online) { | ||
| setRuntimeByStation((current) => { | ||
| const next = { ...current }; | ||
| delete next[stationId]; | ||
| return next; | ||
| }); | ||
| } | ||
| }); | ||
| unlistenFns.push(unlistenStatus); | ||
| }; | ||
|
|
||
| void registerListeners(); | ||
|
|
||
| return () => { | ||
| unlistenFns.forEach((fn) => fn()); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
There’s a race where the component can unmount before registerListeners() resolves. In that case, the cleanup runs with an empty unlistenFns array, but the async listen() calls can still complete afterward and register handlers that are never removed. Fix by adding a cancellation flag: set let cancelled = false; and in cleanup set it true; after each await listen(...) either push the unlisten fn or immediately call it if already cancelled.
| let binary = ''; | ||
| content.forEach((value) => { | ||
| binary += String.fromCharCode(value); | ||
| }); | ||
|
|
||
| return { | ||
| type: 'thumbnail' as const, | ||
| url: `data:${mimeType};base64,${btoa(binary)}`, | ||
| alt: name.split('/').pop() || 'Preview image', | ||
| }; |
There was a problem hiding this comment.
Building binary via repeated string concatenation inside a byte loop can be quadratic and expensive for larger previews. Prefer a chunked conversion approach or avoid base64 entirely by using a Blob/object URL for the preview image (with proper URL.revokeObjectURL lifecycle handling).
| async function parseTemplateZip(file: File): Promise<UITemplatePackage> { | ||
| const entries = await parseZipEntries(await readBinaryFile(file)); | ||
| const templateJson = findZipEntry(entries, 'template.json'); | ||
|
|
||
| if (!templateJson) { | ||
| throw new Error('ZIP template package must include template.json'); | ||
| } |
There was a problem hiding this comment.
ZIP import currently has no upper bounds on (a) input ZIP size, (b) per-entry compressed/uncompressed size, (c) total inflated bytes, or (d) number of entries. This makes the UI vulnerable to decompression bombs / memory exhaustion when users import arbitrary ZIPs. Add explicit limits (fail fast) and enforce them both before and after inflation (e.g., cap file size, cap entry count, cap total extracted bytes, and refuse entries with sizes exceeding thresholds).
| if (groupId) { | ||
| const group = groups.find((candidate) => candidate.id === groupId); | ||
| if (group && !(group.stationIds || []).includes(station.id)) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Group membership filtering only checks group.stationIds. Elsewhere in the UI/groups code paths, group objects also use station_ids (snake_case). If groups here contains station_ids (common for RPC/JSON payloads), the group filter will silently fail and show incorrect results. Fix by normalizing via a helper (e.g., const stationIds = group.stationIds ?? group.station_ids ?? []) and using that consistently across the client.
| <style>{` | ||
| /* Device picker styles */ |
There was a problem hiding this comment.
Embedding a large CSS block inside the component recreates this string on every render and makes styles harder to share/test/maintain. Move these styles into styles.css (or a module/scoped stylesheet) and keep the component focused on behavior; this also makes it easier to deduplicate similar picker styles used in GroupCard.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
Summary
Tests
Notes