feat: reconcile desktop feature branch into a building, tested baseline (Slice 0) - #29
Merged
Merged
Conversation
Land the in-progress feature set as a clean, compiling, tested baseline: fleet-master heartbeat API, settings screen, multi-currency selector, a new sidebar/topbar app shell, and container-lifecycle hardening (Start action, volume cleanup on Remove, stale-deployment reconciliation, tray-on-DomReady + off-screen window recovery, confirm dialogs). - Fix build-breaking `//go:embed build/appicon.png` (icon restored). - Fix `tsc` failure on `import "./style.css"` via CSS/asset module decls (frontend/src/vite-env.d.ts) so `wails build` produces a real .app. - Add unit tests for the new backend surface: store fleet CRUD + AES-GCM credential round-trip, config defaults/coercion, services stale reconciliation, and fleet_server handlers + bearer auth. Green on macOS/arm64 (CGO). - Add PR CI (build + vet + race test + coverage on ubuntu-latest); the repo previously only ran on version tags. - Make codecov realistic: project coverage informational, patch gate 80%, ignore cgo/UI glue (tray_*.go) consistent with app.go/main.go. - README: correct the affiliate disclosure and drop the unimplemented "replace referral codes in Settings" claim.
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds a Fleet API server for LAN worker/mobile heartbeat and device management, persisted via a new store table. App config and state are extended with settings, notifications, currencies, and fleet fields. The frontend is restructured with a sidebar/topbar shell adding catalog, settings, and fleet views. Runtime gains a Start method and volume-aware removal, Manager reconciles stale deployments, and macOS tray/window positioning is refined. CI workflow and coverage config are added/updated alongside a new docs audit file. ChangesBackend: config, runtime, store, fleet API, app wiring
Frontend shell, settings, and fleet UI
CI, docs, and repository metadata
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/runtime/runtime.go (1)
226-246: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAttempt all volume removals. A single
VolumeRemoveerror aborts the loop, leaving later named volumes behind. Once the container is gone,managedContainerVolumescan’t rediscover them, so a retry won’t clean them up. Return a joined error after trying every volume.🤖 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 `@internal/runtime/runtime.go` around lines 226 - 246, The Remove method in DockerProvider stops deleting volumes on the first VolumeRemove failure, which can leave later managed volumes behind; update the loop to attempt removal of every volume returned by managedContainerVolumes, collect any errors instead of returning immediately, and return a joined/combined error after all deletions are attempted. Keep the fix localized to DockerProvider.Remove and preserve the existing container removal flow while changing only the per-volume error handling.internal/services/manager.go (1)
115-144: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAvoid deleting deployments on a single runtime snapshot miss.
Refreshtreats onem.runtime.List(ctx)result as authoritative and removes any stored deployment not returned. If the runtime briefly returns an empty or incomplete list, those records disappear until a later refresh. Require consecutive misses or confirm the container is gone before deleting.🤖 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 `@internal/services/manager.go` around lines 115 - 144, Refresh in Manager currently deletes any deployment missing from a single m.runtime.List(ctx) snapshot, which can remove valid records on transient runtime gaps. Update Manager.Refresh to avoid immediate deletion by tracking misses across refreshes or by verifying the container is truly gone before calling m.store.DeleteDeployment and m.store.RecordEvent; keep the upsert path for active containers and only remove entries after confirmed consecutive absence.
🧹 Nitpick comments (3)
codecov.yml (1)
7-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePatch coverage bar lowered from 90% to 80%.
Combined with
projectnow being informational-only, the only enforced gate is 80% patch coverage. Comments explain the rationale (UI-glue heavy codebase), so this looks intentional rather than an oversight.🤖 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 `@codecov.yml` around lines 7 - 20, The coverage policy change in codecov.yml appears intentional, so no functional fix is needed; keep the `status.project.default.informational` setting and the `status.patch.default.target` at 80% as the enforced gate. If you touch this area, verify the comments still accurately describe the intended UI-glue rationale and that no other coverage thresholds were unintentionally changed.fleet_server.go (1)
48-52: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
ReadTimeout/WriteTimeout/IdleTimeoutto bound slow clients.
ReadHeaderTimeoutalone doesn't cap body read duration.MaxBytesReaderlimits size, not time, so a slow client trickling a body can still hold a connection open (Slowloris-style) on this LAN-exposed listener.🔒️ Proposed hardening
server := &http.Server{ Addr: addr, Handler: mux, ReadHeaderTimeout: 5 * time.Second, + ReadTimeout: 15 * time.Second, + WriteTimeout: 15 * time.Second, + IdleTimeout: 60 * time.Second, }🤖 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 `@fleet_server.go` around lines 48 - 52, The http.Server setup in fleet_server.go only sets ReadHeaderTimeout, which still allows slow clients to hold connections open while reading request bodies. Update the server configuration where the server variable is created to also set ReadTimeout, WriteTimeout, and IdleTimeout so the listener is bounded against slowloris-style clients; keep the change localized to the existing http.Server construction.Source: Linters/SAST tools
frontend/src/main.ts (1)
1415-1419: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider
Intl.NumberFormatfor currency formatting.The hand-rolled symbol map always uses 2 decimals, which is incorrect for zero-decimal currencies like JPY, and any currency not in the map falls back to a non-standard
"12.34 XYZ"format.Intl.NumberFormathandles per-currency decimal rules and symbol placement automatically.♻️ Proposed refactor
function formatBalance(value: number, currency: string) { - const symbols: Record<string, string> = {USD: "$", EUR: "€", GBP: "£", JPY: "¥", CAD: "C$", AUD: "A$", BRL: "R$"}; - if (symbols[currency]) { - return `${symbols[currency]}${value.toFixed(2)}`; - } - return `${value.toFixed(2)} ${currency}`; + try { + return new Intl.NumberFormat(undefined, {style: "currency", currency}).format(value); + } catch { + return `${value.toFixed(2)} ${currency}`; + } }🤖 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 `@frontend/src/main.ts` around lines 1415 - 1419, The currency formatting logic in the helper that builds the symbol map should be replaced with `Intl.NumberFormat` so decimal precision and symbol placement follow locale/currency rules automatically. Update the formatting path in the function that currently checks `symbols[currency]` to format with `Intl.NumberFormat` for the requested currency instead of hardcoding a 2-decimal symbol table and fallback string, so currencies like JPY render correctly and unsupported currencies still use standard formatting.
🤖 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 @.github/workflows/ci.yml:
- Line 20: The checkout step in the CI workflow should disable credential
persistence because this job does not need to push or write back to git. Update
the existing actions/checkout@v4 entry in the workflow to set
persist-credentials to false so the GITHUB_TOKEN is not left in git config after
checkout.
In `@app.go`:
- Around line 211-231: SaveSettings currently updates fleetBindAddress and
fleetPort in config without restarting the running fleet API listener, so the
active server can keep using the old address/port. Update the SaveSettings flow
to trigger a fleet API restart or rebind after a successful cfg.Save, and make
sure the restart uses the updated cfg values before returning from
a.GetSettingsState. Reference the SaveSettings method and the
FleetState/GetFleetState path when wiring the restart so the UI and the actual
listener stay in sync.
In `@frontend/src/main.ts`:
- Around line 168-193: The Total Balance metric card is hardcoding USD instead
of using the selected display currency. Update the dashboard render logic in
main.ts so the metricCard call for “Total Balance” uses the same currency source
as topbar, namely current.config.displayCurrency || "USD". Keep the shared
totalBalance value unchanged and ensure the currency label is consistent across
the page.
- Around line 303-364: The currency change handler in wireShellNav currently
awaits SaveSettings without handling rejection, so invalid settings can fail
silently and skip state/render updates. Wrap the async SaveSettings/GetAppState
flow in try/catch, and surface an error to the user if SaveSettings rejects;
then apply the same error handling pattern to the other Wails call sites called
out in saveSettingsFromForm, addFleetDevice, and removeFleetDevice.
- Around line 445-537: The Fleet API key field is exposed as an editable
environment input, but it cannot be persisted because renderEnvSetting() uses
envInputName() and SaveSettings() only handles mapped keys. Update the settings
UI in renderEnvSetting/renderSettings so CASHPILOT_API_KEY is rendered read-only
or hidden from editing until rotation exists, and make sure envInputName() does
not imply it is writable. If you choose to support editing, wire
CASHPILOT_API_KEY through the same save path and backend handling used by the
other settings keys.
In `@frontend/src/style.css`:
- Around line 1385-1388: The mobile responsive rule is targeting the wrong
sidebar selector, so the fixed sidebar in main.ts will not collapse on narrow
screens. Update the `@media` (max-width: 900px) styles in the stylesheet to use
the .cp-sidebar selector instead of .sidebar, and make sure the existing sidebar
collapse behavior is applied to the aside rendered by the main.ts component.
- Around line 741-745: The active tab styling in .tab-btn.active and
.filter-tab.active uses white text on --accent, which does not meet the needed
contrast. Update the .tab-btn.active and .filter-tab.active rules in the
stylesheet to use a higher-contrast pairing by either changing the background
from --accent to a darker fill or changing the text color to a darker accessible
color, and apply the same contrast fix to .notify-badge so both use consistent
accessible colors.
In `@internal/config/config.go`:
- Around line 22-31: Move FleetAPIKey out of AppConfig plaintext serialization
and store it through the encrypted credential path used by internal/store and
internal/keyring. Update the config handling around AppConfig.Save/Load so the
token is retrieved from the secure store instead of being marshaled into
config.json, and keep the FleetAPIKey field out of the JSON-persisted config
shape. Ensure the new storage and lookup behavior is wired through the existing
config accessors so callers still read/write the API key transparently.
In `@internal/store/store.go`:
- Around line 287-319: The UpsertFleetHeartbeat path currently does a
read-then-update flow on fleet_devices using kind and name, which can race and
still insert duplicates. Add a UNIQUE constraint on the fleet_devices(kind,
name) pair in the store schema/migration, then change UpsertFleetHeartbeat to
use a single atomic INSERT ... ON CONFLICT(kind, name) DO UPDATE upsert instead
of QueryRow plus UPDATE, keeping the existing defaulting and service marshaling
behavior.
---
Outside diff comments:
In `@internal/runtime/runtime.go`:
- Around line 226-246: The Remove method in DockerProvider stops deleting
volumes on the first VolumeRemove failure, which can leave later managed volumes
behind; update the loop to attempt removal of every volume returned by
managedContainerVolumes, collect any errors instead of returning immediately,
and return a joined/combined error after all deletions are attempted. Keep the
fix localized to DockerProvider.Remove and preserve the existing container
removal flow while changing only the per-volume error handling.
In `@internal/services/manager.go`:
- Around line 115-144: Refresh in Manager currently deletes any deployment
missing from a single m.runtime.List(ctx) snapshot, which can remove valid
records on transient runtime gaps. Update Manager.Refresh to avoid immediate
deletion by tracking misses across refreshes or by verifying the container is
truly gone before calling m.store.DeleteDeployment and m.store.RecordEvent; keep
the upsert path for active containers and only remove entries after confirmed
consecutive absence.
---
Nitpick comments:
In `@codecov.yml`:
- Around line 7-20: The coverage policy change in codecov.yml appears
intentional, so no functional fix is needed; keep the
`status.project.default.informational` setting and the
`status.patch.default.target` at 80% as the enforced gate. If you touch this
area, verify the comments still accurately describe the intended UI-glue
rationale and that no other coverage thresholds were unintentionally changed.
In `@fleet_server.go`:
- Around line 48-52: The http.Server setup in fleet_server.go only sets
ReadHeaderTimeout, which still allows slow clients to hold connections open
while reading request bodies. Update the server configuration where the server
variable is created to also set ReadTimeout, WriteTimeout, and IdleTimeout so
the listener is bounded against slowloris-style clients; keep the change
localized to the existing http.Server construction.
In `@frontend/src/main.ts`:
- Around line 1415-1419: The currency formatting logic in the helper that builds
the symbol map should be replaced with `Intl.NumberFormat` so decimal precision
and symbol placement follow locale/currency rules automatically. Update the
formatting path in the function that currently checks `symbols[currency]` to
format with `Intl.NumberFormat` for the requested currency instead of hardcoding
a 2-decimal symbol table and fallback string, so currencies like JPY render
correctly and unsupported currencies still use standard formatting.
🪄 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: CHILL
Plan: Pro
Run ID: 2de8057a-edd1-424b-8996-a7e94c2bd50d
⛔ Files ignored due to path filters (5)
frontend/wailsjs/go/main/App.d.tsis excluded by!frontend/wailsjs/**frontend/wailsjs/go/main/App.jsis excluded by!frontend/wailsjs/**frontend/wailsjs/go/models.tsis excluded by!frontend/wailsjs/**trayicons/solarized-sun.pngis excluded by!**/*.pngtrayicons/solarized-sun.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
.github/workflows/ci.yml.gitignoreREADME.mdapp.gocodecov.ymldocs/edge-case-audit.mdfleet_server.gofleet_server_test.gofrontend/src/main.tsfrontend/src/style.cssfrontend/src/vite-env.d.tsfrontend/src/wails.d.tsinternal/config/config.gointernal/config/config_test.gointernal/runtime/runtime.gointernal/runtime/runtime_test.gointernal/services/manager.gointernal/services/manager_test.gointernal/store/store.gointernal/store/store_test.gomain.gotray_darwin.gotray_other.go
Comment on lines
+211
to
+231
| if value := strings.TrimSpace(values["fleetBindAddress"]); value != "" { | ||
| cfg.FleetBindAddress = value | ||
| } | ||
| if value := strings.TrimSpace(values["collectIntervalMinutes"]); value != "" { | ||
| minutes, err := strconv.Atoi(value) | ||
| if err != nil || minutes <= 0 { | ||
| return SettingsState{}, fmt.Errorf("collect interval must be a positive number") | ||
| } | ||
| cfg.CollectIntervalMinutes = minutes | ||
| } | ||
| if value := strings.TrimSpace(values["fleetPort"]); value != "" { | ||
| port, err := strconv.Atoi(value) | ||
| if err != nil || port <= 0 || port > 65535 { | ||
| return SettingsState{}, fmt.Errorf("fleet API port must be between 1 and 65535") | ||
| } | ||
| cfg.FleetPort = port | ||
| } | ||
| if err := a.cfg.Save(cfg); err != nil { | ||
| return SettingsState{}, err | ||
| } | ||
| return a.GetSettingsState() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- app.go around settings and fleet startup ---'
nl -ba app.go | sed -n '1,340p'
echo
echo '--- search fleetAPI / FleetBindAddress / FleetPort ---'
rg -n "fleetAPI|FleetBindAddress|FleetPort|SaveSettings|GetFleetState" -S .Repository: GeiserX/CashPilot-Desktop
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- app.go around settings and fleet startup ---'
sed -n '1,340p' app.go | cat -n
echo
echo '--- search fleetAPI / FleetBindAddress / FleetPort / SaveSettings / GetFleetState ---'
rg -n "fleetAPI|FleetBindAddress|FleetPort|SaveSettings|GetFleetState" app.go .Repository: GeiserX/CashPilot-Desktop
Length of output: 18595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- fleet_server.go ---'
sed -n '1,240p' fleet_server.go | cat -nRepository: GeiserX/CashPilot-Desktop
Length of output: 6643
Restart the fleet API after changing bind/port. SaveSettings only persists the new values; the running listener keeps the old address/port, while GetFleetState immediately builds URLs from the updated config. That leaves the UI advertising a port the server isn’t listening on until restart.
🤖 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 `@app.go` around lines 211 - 231, SaveSettings currently updates
fleetBindAddress and fleetPort in config without restarting the running fleet
API listener, so the active server can keep using the old address/port. Update
the SaveSettings flow to trigger a fleet API restart or rebind after a
successful cfg.Save, and make sure the restart uses the updated cfg values
before returning from a.GetSettingsState. Reference the SaveSettings method and
the FleetState/GetFleetState path when wiring the restart so the UI and the
actual listener stay in sync.
Comment on lines
168
to
+193
| const totalBalance = earnings.reduce((sum, record) => sum + (record.error ? 0 : record.balance), 0); | ||
| const trackedCount = earnings.filter((record) => !record.error).length; | ||
| const recentServices = [...services] | ||
| .filter((svc) => !deployedSlugs.has(svc.slug)) | ||
| .sort((a, b) => a.name.localeCompare(b.name)) | ||
| .slice(0, 6); | ||
|
|
||
| root.innerHTML = ` | ||
| ${titlebar()} | ||
| <div class="shell"> | ||
| <aside class="sidebar"> | ||
| <div> | ||
| <p class="eyebrow">CashPilot Desktop</p> | ||
| <h1>Dashboard</h1> | ||
| <p class="muted">Earnings, services, logs, and payouts for this machine.</p> | ||
| </div> | ||
| <div class="runtime-card compact ${current.runtime.available ? "ok" : "warn"}"> | ||
| <strong>${current.runtime.available ? "Runtime ready" : "Runtime offline"}</strong> | ||
| <span>${escapeHtml(current.runtime.message)}</span> | ||
| </div> | ||
| <nav class="service-list"> | ||
| ${services.map((svc) => serviceButton(svc, deployments)).join("")} | ||
| </nav> | ||
| </aside> | ||
| <main class="content"> | ||
| <section class="metric-grid"> | ||
| <div class="app-layout"> | ||
| ${appSidebar("dashboard")} | ||
| <div class="main-content"> | ||
| ${topbar("Dashboard", totalBalance, current)} | ||
| <main class="page-content"> | ||
| <section class="stats-grid"> | ||
| ${metricCard("Total Balance", formatBalance(totalBalance, "USD"), "Latest collected balance")} | ||
| ${metricCard("Today", "$0.00", "Awaiting daily history")} | ||
| ${metricCard("This Month", "$0.00", "Awaiting monthly history")} | ||
| ${metricCard("Active Services", `${runningCount}`, "Containers currently running")} | ||
| ${metricCard("Tracked", `${trackedCount}`, "Services with earnings data")} | ||
| ${metricCard("Catalog", `${services.length}`, "Available providers")} | ||
| </section> | ||
|
|
||
| <section class="panel earnings-panel"> | ||
| <div class="split"> | ||
| <section class="card earnings-panel"> | ||
| <div class="card-header"> | ||
| <div> | ||
| <p class="eyebrow">Earnings</p> | ||
| <h2>What your services are earning</h2> | ||
| <span class="card-title">Earnings</span> | ||
| <p class="muted compact-copy">What each service has earned so far.</p> | ||
| </div> | ||
| <div class="hero-actions"> | ||
| <button class="primary" id="open-wizard">+ Add Service</button> | ||
| <button class="secondary" id="refresh">Refresh</button> | ||
| <div class="tab-strip"> | ||
| <button class="tab-btn active">7 days</button> | ||
| <button class="tab-btn">30 days</button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Total Balance card ignores the selected display currency.
The topbar correctly formats with current.config.displayCurrency || "USD" (line 317), but the "Total Balance" metric card hardcodes "USD" regardless of the user's currency selection introduced in this PR. Same totalBalance value will show two different currency labels on the same screen.
🐛 Proposed fix
- ${metricCard("Total Balance", formatBalance(totalBalance, "USD"), "Latest collected balance")}
+ ${metricCard("Total Balance", formatBalance(totalBalance, current.config.displayCurrency || "USD"), "Latest collected balance")}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const totalBalance = earnings.reduce((sum, record) => sum + (record.error ? 0 : record.balance), 0); | |
| const trackedCount = earnings.filter((record) => !record.error).length; | |
| const recentServices = [...services] | |
| .filter((svc) => !deployedSlugs.has(svc.slug)) | |
| .sort((a, b) => a.name.localeCompare(b.name)) | |
| .slice(0, 6); | |
| root.innerHTML = ` | |
| ${titlebar()} | |
| <div class="shell"> | |
| <aside class="sidebar"> | |
| <div> | |
| <p class="eyebrow">CashPilot Desktop</p> | |
| <h1>Dashboard</h1> | |
| <p class="muted">Earnings, services, logs, and payouts for this machine.</p> | |
| </div> | |
| <div class="runtime-card compact ${current.runtime.available ? "ok" : "warn"}"> | |
| <strong>${current.runtime.available ? "Runtime ready" : "Runtime offline"}</strong> | |
| <span>${escapeHtml(current.runtime.message)}</span> | |
| </div> | |
| <nav class="service-list"> | |
| ${services.map((svc) => serviceButton(svc, deployments)).join("")} | |
| </nav> | |
| </aside> | |
| <main class="content"> | |
| <section class="metric-grid"> | |
| <div class="app-layout"> | |
| ${appSidebar("dashboard")} | |
| <div class="main-content"> | |
| ${topbar("Dashboard", totalBalance, current)} | |
| <main class="page-content"> | |
| <section class="stats-grid"> | |
| ${metricCard("Total Balance", formatBalance(totalBalance, "USD"), "Latest collected balance")} | |
| ${metricCard("Today", "$0.00", "Awaiting daily history")} | |
| ${metricCard("This Month", "$0.00", "Awaiting monthly history")} | |
| ${metricCard("Active Services", `${runningCount}`, "Containers currently running")} | |
| ${metricCard("Tracked", `${trackedCount}`, "Services with earnings data")} | |
| ${metricCard("Catalog", `${services.length}`, "Available providers")} | |
| </section> | |
| <section class="panel earnings-panel"> | |
| <div class="split"> | |
| <section class="card earnings-panel"> | |
| <div class="card-header"> | |
| <div> | |
| <p class="eyebrow">Earnings</p> | |
| <h2>What your services are earning</h2> | |
| <span class="card-title">Earnings</span> | |
| <p class="muted compact-copy">What each service has earned so far.</p> | |
| </div> | |
| <div class="hero-actions"> | |
| <button class="primary" id="open-wizard">+ Add Service</button> | |
| <button class="secondary" id="refresh">Refresh</button> | |
| <div class="tab-strip"> | |
| <button class="tab-btn active">7 days</button> | |
| <button class="tab-btn">30 days</button> | |
| const totalBalance = earnings.reduce((sum, record) => sum + (record.error ? 0 : record.balance), 0); | |
| const trackedCount = earnings.filter((record) => !record.error).length; | |
| root.innerHTML = ` | |
| ${titlebar()} | |
| <div class="app-layout"> | |
| ${appSidebar("dashboard")} | |
| <div class="main-content"> | |
| ${topbar("Dashboard", totalBalance, current)} | |
| <main class="page-content"> | |
| <section class="stats-grid"> | |
| ${metricCard("Total Balance", formatBalance(totalBalance, current.config.displayCurrency || "USD"), "Latest collected balance")} | |
| ${metricCard("Today", "$0.00", "Awaiting daily history")} | |
| ${metricCard("This Month", "$0.00", "Awaiting monthly history")} | |
| ${metricCard("Active Services", `${runningCount}`, "Containers currently running")} | |
| </section> | |
| <section class="card earnings-panel"> | |
| <div class="card-header"> | |
| <div> | |
| <span class="card-title">Earnings</span> | |
| <p class="muted compact-copy">What each service has earned so far.</p> | |
| </div> | |
| <div class="tab-strip"> | |
| <button class="tab-btn active">7 days</button> | |
| <button class="tab-btn">30 days</button> |
🤖 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 `@frontend/src/main.ts` around lines 168 - 193, The Total Balance metric card
is hardcoding USD instead of using the selected display currency. Update the
dashboard render logic in main.ts so the metricCard call for “Total Balance”
uses the same currency source as topbar, namely current.config.displayCurrency
|| "USD". Keep the shared totalBalance value unchanged and ensure the currency
label is consistent across the page.
Comment on lines
+303
to
+364
| function navButton(view: View, label: string, active: string) { | ||
| return `<button class="sidebar-link ${active === view ? "active" : ""}" data-view="${view}">${escapeHtml(label)}</button>`; | ||
| } | ||
|
|
||
| function topbar(title: string, totalBalance: number, current: AppState) { | ||
| const notifications = current.notifications || []; | ||
| return ` | ||
| <header class="topbar"> | ||
| <div class="topbar-left"> | ||
| <span class="topbar-title">${escapeHtml(title)}</span> | ||
| </div> | ||
| <div class="topbar-right"> | ||
| <span class="runtime-dot ${current.runtime.available ? "ok" : "warn"}"></span> | ||
| <span class="topbar-runtime">${current.runtime.available ? "Runtime ready" : "Runtime offline"}</span> | ||
| <span class="topbar-earnings">${formatBalance(totalBalance, current.config.displayCurrency || "USD")}</span> | ||
| <select class="currency-select" id="currency-select" title="Display currency"> | ||
| ${(current.currencies || ["USD", "EUR"]).map((currency) => `<option value="${currency}" ${currency === current.config.displayCurrency ? "selected" : ""}>${currency}</option>`).join("")} | ||
| </select> | ||
| <details class="notification-menu"> | ||
| <summary aria-label="Notifications">Alerts <span class="notify-badge">${notifications.length}</span></summary> | ||
| <div class="notification-popover"> | ||
| <strong>Notifications</strong> | ||
| ${notifications.length ? notifications.map((item) => ` | ||
| <div class="notification-item ${escapeHtml(item.level)}"> | ||
| <span>${escapeHtml(item.title)}</span> | ||
| <small>${escapeHtml(item.message)}</small> | ||
| </div> | ||
| `).join("") : `<p class="muted">No alerts right now.</p>`} | ||
| </div> | ||
| </details> | ||
| </div> | ||
| </header> | ||
| `; | ||
| } | ||
|
|
||
| function wireShellNav() { | ||
| document.querySelectorAll<HTMLButtonElement>("[data-view]").forEach((button) => { | ||
| button.addEventListener("click", () => { | ||
| const next = button.dataset.view; | ||
| if (next === "dashboard" || next === "wizard" || next === "catalog" || next === "settings" || next === "fleet") { | ||
| activeView = next; | ||
| if (next === "wizard") { | ||
| wizardStep = 1; | ||
| } | ||
| resetScrollAfterRender = true; | ||
| render(); | ||
| } | ||
| }); | ||
| }); | ||
| document.querySelectorAll<HTMLButtonElement>("[data-url]").forEach((button) => { | ||
| button.addEventListener("click", () => { | ||
| const url = button.dataset.url; | ||
| if (url) BrowserOpenURL(url); | ||
| }); | ||
| }); | ||
| document.querySelector<HTMLSelectElement>("#currency-select")?.addEventListener("change", async (event) => { | ||
| const currency = (event.target as HTMLSelectElement).value; | ||
| await SaveSettings({displayCurrency: currency}); | ||
| state = await GetAppState(); | ||
| render(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wails calls that can reject aren't error-handled — silent failures on invalid input.
SaveSettings (line 360) can reject (backend validates collectIntervalMinutes/fleetPort and returns an error, per app.go's SaveSettings). There's no try/catch here, so a rejection becomes an unhandled promise rejection: state/render() never run, and the user gets zero feedback that the currency change failed. The same gap exists in saveSettingsFromForm (line 521) and addFleetDevice/removeFleetDevice (lines 640, 646) later in this file — flagging once here since it's the same root cause repeated across the new settings/fleet call sites.
🛡️ Proposed fix pattern (apply similarly to the other call sites)
document.querySelector<HTMLSelectElement>("`#currency-select`")?.addEventListener("change", async (event) => {
const currency = (event.target as HTMLSelectElement).value;
- await SaveSettings({displayCurrency: currency});
- state = await GetAppState();
- render();
+ try {
+ await SaveSettings({displayCurrency: currency});
+ state = await GetAppState();
+ render();
+ } catch (error) {
+ alert(`Failed to save currency: ${String(error)}`);
+ }
});📝 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
| function navButton(view: View, label: string, active: string) { | |
| return `<button class="sidebar-link ${active === view ? "active" : ""}" data-view="${view}">${escapeHtml(label)}</button>`; | |
| } | |
| function topbar(title: string, totalBalance: number, current: AppState) { | |
| const notifications = current.notifications || []; | |
| return ` | |
| <header class="topbar"> | |
| <div class="topbar-left"> | |
| <span class="topbar-title">${escapeHtml(title)}</span> | |
| </div> | |
| <div class="topbar-right"> | |
| <span class="runtime-dot ${current.runtime.available ? "ok" : "warn"}"></span> | |
| <span class="topbar-runtime">${current.runtime.available ? "Runtime ready" : "Runtime offline"}</span> | |
| <span class="topbar-earnings">${formatBalance(totalBalance, current.config.displayCurrency || "USD")}</span> | |
| <select class="currency-select" id="currency-select" title="Display currency"> | |
| ${(current.currencies || ["USD", "EUR"]).map((currency) => `<option value="${currency}" ${currency === current.config.displayCurrency ? "selected" : ""}>${currency}</option>`).join("")} | |
| </select> | |
| <details class="notification-menu"> | |
| <summary aria-label="Notifications">Alerts <span class="notify-badge">${notifications.length}</span></summary> | |
| <div class="notification-popover"> | |
| <strong>Notifications</strong> | |
| ${notifications.length ? notifications.map((item) => ` | |
| <div class="notification-item ${escapeHtml(item.level)}"> | |
| <span>${escapeHtml(item.title)}</span> | |
| <small>${escapeHtml(item.message)}</small> | |
| </div> | |
| `).join("") : `<p class="muted">No alerts right now.</p>`} | |
| </div> | |
| </details> | |
| </div> | |
| </header> | |
| `; | |
| } | |
| function wireShellNav() { | |
| document.querySelectorAll<HTMLButtonElement>("[data-view]").forEach((button) => { | |
| button.addEventListener("click", () => { | |
| const next = button.dataset.view; | |
| if (next === "dashboard" || next === "wizard" || next === "catalog" || next === "settings" || next === "fleet") { | |
| activeView = next; | |
| if (next === "wizard") { | |
| wizardStep = 1; | |
| } | |
| resetScrollAfterRender = true; | |
| render(); | |
| } | |
| }); | |
| }); | |
| document.querySelectorAll<HTMLButtonElement>("[data-url]").forEach((button) => { | |
| button.addEventListener("click", () => { | |
| const url = button.dataset.url; | |
| if (url) BrowserOpenURL(url); | |
| }); | |
| }); | |
| document.querySelector<HTMLSelectElement>("#currency-select")?.addEventListener("change", async (event) => { | |
| const currency = (event.target as HTMLSelectElement).value; | |
| await SaveSettings({displayCurrency: currency}); | |
| state = await GetAppState(); | |
| render(); | |
| }); | |
| } | |
| function navButton(view: View, label: string, active: string) { | |
| return `<button class="sidebar-link ${active === view ? "active" : ""}" data-view="${view}">${escapeHtml(label)}</button>`; | |
| } | |
| function topbar(title: string, totalBalance: number, current: AppState) { | |
| const notifications = current.notifications || []; | |
| return ` | |
| <header class="topbar"> | |
| <div class="topbar-left"> | |
| <span class="topbar-title">${escapeHtml(title)}</span> | |
| </div> | |
| <div class="topbar-right"> | |
| <span class="runtime-dot ${current.runtime.available ? "ok" : "warn"}"></span> | |
| <span class="topbar-runtime">${current.runtime.available ? "Runtime ready" : "Runtime offline"}</span> | |
| <span class="topbar-earnings">${formatBalance(totalBalance, current.config.displayCurrency || "USD")}</span> | |
| <select class="currency-select" id="currency-select" title="Display currency"> | |
| ${(current.currencies || ["USD", "EUR"]).map((currency) => `<option value="${currency}" ${currency === current.config.displayCurrency ? "selected" : ""}>${currency}</option>`).join("")} | |
| </select> | |
| <details class="notification-menu"> | |
| <summary aria-label="Notifications">Alerts <span class="notify-badge">${notifications.length}</span></summary> | |
| <div class="notification-popover"> | |
| <strong>Notifications</strong> | |
| ${notifications.length ? notifications.map((item) => ` | |
| <div class="notification-item ${escapeHtml(item.level)}"> | |
| <span>${escapeHtml(item.title)}</span> | |
| <small>${escapeHtml(item.message)}</small> | |
| </div> | |
| `).join("") : `<p class="muted">No alerts right now.</p>`} | |
| </div> | |
| </details> | |
| </div> | |
| </header> | |
| `; | |
| } | |
| function wireShellNav() { | |
| document.querySelectorAll<HTMLButtonElement>("[data-view]").forEach((button) => { | |
| button.addEventListener("click", () => { | |
| const next = button.dataset.view; | |
| if (next === "dashboard" || next === "wizard" || next === "catalog" || next === "settings" || next === "fleet") { | |
| activeView = next; | |
| if (next === "wizard") { | |
| wizardStep = 1; | |
| } | |
| resetScrollAfterRender = true; | |
| render(); | |
| } | |
| }); | |
| }); | |
| document.querySelectorAll<HTMLButtonElement>("[data-url]").forEach((button) => { | |
| button.addEventListener("click", () => { | |
| const url = button.dataset.url; | |
| if (url) BrowserOpenURL(url); | |
| }); | |
| }); | |
| document.querySelector<HTMLSelectElement>("`#currency-select`")?.addEventListener("change", async (event) => { | |
| const currency = (event.target as HTMLSelectElement).value; | |
| try { | |
| await SaveSettings({displayCurrency: currency}); | |
| state = await GetAppState(); | |
| render(); | |
| } catch (error) { | |
| alert(`Failed to save currency: ${String(error)}`); | |
| } | |
| }); | |
| } |
🤖 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 `@frontend/src/main.ts` around lines 303 - 364, The currency change handler in
wireShellNav currently awaits SaveSettings without handling rejection, so
invalid settings can fail silently and skip state/render updates. Wrap the async
SaveSettings/GetAppState flow in try/catch, and surface an error to the user if
SaveSettings rejects; then apply the same error handling pattern to the other
Wails call sites called out in saveSettingsFromForm, addFleetDevice, and
removeFleetDevice.
Comment on lines
+445
to
+537
| async function renderSettings(current: AppState) { | ||
| const settings = await GetSettingsState(); | ||
| const total = totalBalance(current); | ||
| root.innerHTML = ` | ||
| ${titlebar()} | ||
| <div class="app-layout"> | ||
| ${appSidebar("settings")} | ||
| <div class="main-content"> | ||
| ${topbar("Settings", total, current)} | ||
| <main class="page-content"> | ||
| <section class="card"> | ||
| <div class="card-header"> | ||
| <div> | ||
| <span class="card-title">Environment Variables</span> | ||
| <p class="muted compact-copy">Variables that affect this desktop node. Locked values are controlled by the app or OS.</p> | ||
| </div> | ||
| <button class="primary compact-btn" id="save-settings">Save Variables</button> | ||
| </div> | ||
| <div class="settings-list"> | ||
| ${settings.environment.map(renderEnvSetting).join("")} | ||
| </div> | ||
| </section> | ||
|
|
||
| <section class="card"> | ||
| <div class="card-header"> | ||
| <div> | ||
| <span class="card-title">Earnings Collection</span> | ||
| <p class="muted compact-copy">Credentials for automated earnings tracking. Manual/mobile-first services can be tracked without deploying a container.</p> | ||
| </div> | ||
| </div> | ||
| <div class="collector-grid"> | ||
| ${settings.collectors.map(renderCollectorSetting).join("")} | ||
| </div> | ||
| </section> | ||
| </main> | ||
| </div> | ||
| </div> | ||
| `; | ||
| wireChrome(); | ||
| wireShellNav(); | ||
| maybeResetScroll(); | ||
| document.querySelector("#save-settings")?.addEventListener("click", () => void saveSettingsFromForm()); | ||
| document.querySelectorAll<HTMLButtonElement>("[data-service]").forEach((button) => { | ||
| button.addEventListener("click", () => openWizard(button.dataset.service)); | ||
| }); | ||
| } | ||
|
|
||
| function renderEnvSetting(item: SettingsState["environment"][number]) { | ||
| const editableKey = envInputName(item.key); | ||
| return ` | ||
| <label class="setting-row"> | ||
| <span> | ||
| <strong>${escapeHtml(item.label)}</strong> | ||
| <small>${escapeHtml(item.key)} · ${escapeHtml(item.source)}</small> | ||
| </span> | ||
| <input data-setting="${editableKey}" value="${escapeHtml(item.value)}" ${item.readOnly ? "readonly" : ""} /> | ||
| <small>${escapeHtml(item.help)}</small> | ||
| </label> | ||
| `; | ||
| } | ||
|
|
||
| function renderCollectorSetting(item: SettingsState["collectors"][number]) { | ||
| return ` | ||
| <button class="collector-row" data-service="${escapeHtml(item.slug)}"> | ||
| <span>${escapeHtml(item.name)}</span> | ||
| <small>${escapeHtml(item.collector || "manual")}</small> | ||
| <strong class="${item.configured ? "configured" : ""}">${item.configured ? "Configured" : "Not configured"}</strong> | ||
| </button> | ||
| `; | ||
| } | ||
|
|
||
| async function saveSettingsFromForm() { | ||
| const values: Record<string, string> = {}; | ||
| document.querySelectorAll<HTMLInputElement>("[data-setting]").forEach((input) => { | ||
| if (!input.readOnly) values[input.dataset.setting || ""] = input.value; | ||
| }); | ||
| await SaveSettings(values); | ||
| state = await GetAppState(); | ||
| render(); | ||
| } | ||
|
|
||
| function envInputName(key: string) { | ||
| const names: Record<string, string> = { | ||
| CASHPILOT_HOSTNAME_PREFIX: "hostnamePrefix", | ||
| CASHPILOT_COLLECT_INTERVAL: "collectIntervalMinutes", | ||
| CASHPILOT_DISPLAY_CURRENCY: "displayCurrency", | ||
| CASHPILOT_FLEET_BIND: "fleetBindAddress", | ||
| CASHPILOT_FLEET_PORT: "fleetPort", | ||
| TZ: "timezone", | ||
| }; | ||
| return names[key] || key; | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant symbols ==\n'
rg -n "GetSettingsState|SaveSettings|CASHPILOT_API_KEY|Secret: true|ReadOnly|envInputName|renderEnvSetting|saveSettingsFromForm" frontend src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== File map: frontend/src/main.ts ==\n'
ast-grep outline frontend/src/main.ts --view expanded || true
printf '\n== File map: app.go ==\n'
ast-grep outline app.go --view expanded || true
printf '\n== app.go excerpt around settings ==\n'
rg -n -A80 -B20 "func .*GetSettingsState|func .*SaveSettings|CASHPILOT_API_KEY|Secret: true|ReadOnly" app.go || true
printf '\n== frontend/src/main.ts excerpt around settings render ==\n'
sed -n '445,537p' frontend/src/main.tsRepository: GeiserX/CashPilot-Desktop
Length of output: 28256
Fleet API Key is editable but not saved — CASHPILOT_API_KEY renders as a normal input, but SaveSettings ignores it because envInputName() has no mapping and the backend doesn’t handle that key. Mark it read-only until key rotation is implemented, or wire up a real rotation path.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 447-481: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: root.innerHTML = `
${titlebar()}
${appSidebar("settings")}
${topbar("Settings", total, current)}
Environment Variables
Variables that affect this desktop node. Locked values are controlled by the app or OS.
Save Variables
${settings.environment.map(renderEnvSetting).join("")}
<section class="card">
<div class="card-header">
<div>
<span class="card-title">Earnings Collection</span>
<p class="muted compact-copy">Credentials for automated earnings tracking. Manual/mobile-first services can be tracked without deploying a container.</p>
</div>
</div>
<div class="collector-grid">
${settings.collectors.map(renderCollectorSetting).join("")}
</div>
</section>
</main>
</div>
</div>
`
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(dom-content-modification)
[warning] 447-481: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: root.innerHTML = `
${titlebar()}
${appSidebar("settings")}
${topbar("Settings", total, current)}
Environment Variables
Variables that affect this desktop node. Locked values are controlled by the app or OS.
Save Variables
${settings.environment.map(renderEnvSetting).join("")}
<section class="card">
<div class="card-header">
<div>
<span class="card-title">Earnings Collection</span>
<p class="muted compact-copy">Credentials for automated earnings tracking. Manual/mobile-first services can be tracked without deploying a container.</p>
</div>
</div>
<div class="collector-grid">
${settings.collectors.map(renderCollectorSetting).join("")}
</div>
</section>
</main>
</div>
</div>
`
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(unsafe-html-content-assignment)
🤖 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 `@frontend/src/main.ts` around lines 445 - 537, The Fleet API key field is
exposed as an editable environment input, but it cannot be persisted because
renderEnvSetting() uses envInputName() and SaveSettings() only handles mapped
keys. Update the settings UI in renderEnvSetting/renderSettings so
CASHPILOT_API_KEY is rendered read-only or hidden from editing until rotation
exists, and make sure envInputName() does not imply it is writable. If you
choose to support editing, wire CASHPILOT_API_KEY through the same save path and
backend handling used by the other settings keys.
Comment on lines
+741
to
+745
| .tab-btn.active, | ||
| .filter-tab.active { | ||
| color: #fff; | ||
| background: var(--accent); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
def lin(c):
c/=255
return c/12.92 if c<=0.03928 else ((c+0.055)/1.055)**2.4
def L(r,g,b):
return 0.2126*lin(r)+0.7152*lin(g)+0.0722*lin(b)
def ratio(f,b):
l1,l2=L(*f),L(*b)
hi,lo=max(l1,l2),min(l1,l2)
return (hi+0.05)/(lo+0.05)
print("white on `#fb7185`:", round(ratio((255,255,255),(251,113,133)),2))
PYRepository: GeiserX/CashPilot-Desktop
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="frontend/src/style.css"
# Show the relevant blocks with line numbers.
sed -n '450,500p;730,760p' "$file" | cat -n
# Find the accent variable definition.
rg -n --no-heading --line-number --fixed-strings "--accent" "$file"Repository: GeiserX/CashPilot-Desktop
Length of output: 2751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="frontend/src/style.css"
# Find the accent variable definition and nearby palette colors.
rg -n --no-heading --line-number "accent|text-primary|text-secondary|bg-secondary|bg-tertiary" "$file"
# Show the root/theme block around the accent definition.
python3 - <<'PY'
from pathlib import Path
p = Path("frontend/src/style.css")
lines = p.read_text().splitlines()
for i,l in enumerate(lines,1):
if "--accent" in l:
start = max(1, i-15)
end = min(len(lines), i+15)
for j in range(start, end+1):
print(f"{j:4d}: {lines[j-1]}")
break
PYRepository: GeiserX/CashPilot-Desktop
Length of output: 2727
White text on --accent needs a higher-contrast pairing. .notify-badge and the active tab styles both use #fff on --accent (#fb7185), which is too low contrast for AA; switch to a darker fill or darker text color.
🤖 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 `@frontend/src/style.css` around lines 741 - 745, The active tab styling in
.tab-btn.active and .filter-tab.active uses white text on --accent, which does
not meet the needed contrast. Update the .tab-btn.active and .filter-tab.active
rules in the stylesheet to use a higher-contrast pairing by either changing the
background from --accent to a darker fill or changing the text color to a darker
accessible color, and apply the same contrast fix to .notify-badge so both use
consistent accessible colors.
Comment on lines
+1385
to
+1388
| .collector-grid, | ||
| .connection-grid, | ||
| .fleet-snippets, | ||
| .fleet-form, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "\bcp-sidebar\b|class=.*\bsidebar\b" frontend/src/main.tsRepository: GeiserX/CashPilot-Desktop
Length of output: 441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== main.ts sidebar-related markup =="
sed -n '240,280p' frontend/src/main.ts
echo
echo "== style.css around sidebar rules =="
grep -nE '\.cp-sidebar|\.sidebar\b|max-width:\s*900px' -n frontend/src/style.css
echo
sed -n '320,360p' frontend/src/style.css
echo
sed -n '1368,1405p' frontend/src/style.cssRepository: GeiserX/CashPilot-Desktop
Length of output: 2985
Update the mobile sidebar selector to .cp-sidebar.
frontend/src/main.ts renders <aside class="cp-sidebar">, but the @media (max-width: 900px) rule still targets .sidebar, so the fixed sidebar won’t collapse on narrow screens.
🤖 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 `@frontend/src/style.css` around lines 1385 - 1388, The mobile responsive rule
is targeting the wrong sidebar selector, so the fixed sidebar in main.ts will
not collapse on narrow screens. Update the `@media` (max-width: 900px) styles in
the stylesheet to use the .cp-sidebar selector instead of .sidebar, and make
sure the existing sidebar collapse behavior is applied to the aside rendered by
the main.ts component.
Comment on lines
+22
to
+31
| FirstRunComplete bool `json:"firstRunComplete"` | ||
| DisplayCurrency string `json:"displayCurrency"` | ||
| RuntimeProvider string `json:"runtimeProvider"` | ||
| AutoUpdate bool `json:"autoUpdate"` | ||
| HostnamePrefix string `json:"hostnamePrefix"` | ||
| CollectIntervalMinutes int `json:"collectIntervalMinutes"` | ||
| Timezone string `json:"timezone"` | ||
| FleetAPIKey string `json:"fleetApiKey"` | ||
| FleetBindAddress string `json:"fleetBindAddress"` | ||
| FleetPort int `json:"fleetPort"` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
FleetAPIKey persisted in plaintext, unlike other secrets.
Every other secret in this codebase (internal/store credentials) is AES-GCM encrypted at rest using a keychain-derived master key. FleetAPIKey is just another field on AppConfig, serialized via json.MarshalIndent and written to config.json (0o600) in plaintext (see Save, lines 106-116). A local file-permission bypass or backup leak exposes the fleet bearer token directly, whereas credentials table entries would not. Given the coding guideline to never handle credentials outside the OS keychain path, this token should go through internal/store's encrypted credential storage (or the OS keychain) instead of the plain config file.
As per coding guidelines, **/*.go: "Never hardcode credentials — use OS keychain via internal/keyring" — while this key isn't hardcoded, storing it in plaintext config defeats the same protection the OS-keychain-backed encrypted store already provides for other secrets in this codebase.
🤖 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 `@internal/config/config.go` around lines 22 - 31, Move FleetAPIKey out of
AppConfig plaintext serialization and store it through the encrypted credential
path used by internal/store and internal/keyring. Update the config handling
around AppConfig.Save/Load so the token is retrieved from the secure store
instead of being marshaled into config.json, and keep the FleetAPIKey field out
of the JSON-persisted config shape. Ensure the new storage and lookup behavior
is wired through the existing config accessors so callers still read/write the
API key transparently.
Source: Coding guidelines
Comment on lines
+287
to
+319
| func (s *Store) UpsertFleetHeartbeat(device FleetDevice) (FleetDevice, error) { | ||
| if device.Name == "" { | ||
| return FleetDevice{}, fmt.Errorf("device name is required") | ||
| } | ||
| if device.Kind == "" { | ||
| device.Kind = "worker" | ||
| } | ||
| if device.Status == "" { | ||
| device.Status = "online" | ||
| } | ||
| if device.LastSeen == "" { | ||
| device.LastSeen = time.Now().UTC().Format(time.RFC3339Nano) | ||
| } | ||
| servicesRaw, err := json.Marshal(device.Services) | ||
| if err != nil { | ||
| return FleetDevice{}, err | ||
| } | ||
| var id int64 | ||
| err = s.db.QueryRow(`SELECT id FROM fleet_devices WHERE kind = ? AND name = ?`, device.Kind, device.Name).Scan(&id) | ||
| if err == nil { | ||
| device.ID = id | ||
| _, err = s.db.Exec(` | ||
| UPDATE fleet_devices | ||
| SET endpoint = ?, os = ?, arch = ?, status = ?, services = ?, last_seen = ?, updated_at = datetime('now') | ||
| WHERE id = ? | ||
| `, device.Endpoint, device.OS, device.Arch, device.Status, string(servicesRaw), device.LastSeen, id) | ||
| return device, err | ||
| } | ||
| if err != sql.ErrNoRows { | ||
| return FleetDevice{}, err | ||
| } | ||
| return s.UpsertFleetDevice(device) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- internal/store/store.go relevant slice ---'
nl -ba internal/store/store.go | sed -n '250,360p'
echo
echo '--- search fleet_devices schema/migrations ---'
rg -n "fleet_devices|UNIQUE\\s*\\(|CREATE UNIQUE INDEX|kind, name" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
echo
echo '--- list migration files if any ---'
fd -t f '.*' . | rg 'migrations|schema|sql$|store'Repository: GeiserX/CashPilot-Desktop
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- internal/store/store.go relevant slice ---'
sed -n '250,360p' internal/store/store.go | cat -n
echo
echo '--- search fleet_devices schema/migrations ---'
rg -n "fleet_devices|CREATE TABLE|CREATE UNIQUE INDEX|UNIQUE\\s*\\(" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
echo
echo '--- files mentioning fleet_devices ---'
rg -l "fleet_devices" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: GeiserX/CashPilot-Desktop
Length of output: 5445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- internal/store/store.go relevant slice ---'
sed -n '280,340p' internal/store/store.go | cat -n
printf '\n%s\n' '--- fleet_devices schema/migration matches ---'
rg -n "fleet_devices|CREATE TABLE|CREATE UNIQUE INDEX|UNIQUE\\s*\\(" -S . \
--glob '!**/node_modules/**' \
--glob '!**/dist/**' \
--glob '!**/build/**'
printf '\n%s\n' '--- candidate files ---'
rg -l "fleet_devices" -S . \
--glob '!**/node_modules/**' \
--glob '!**/dist/**' \
--glob '!**/build/**'Repository: GeiserX/CashPilot-Desktop
Length of output: 3459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '388,420p' internal/store/store.go | cat -nRepository: GeiserX/CashPilot-Desktop
Length of output: 1322
Make fleet_devices(kind, name) unique and upsert atomically
UpsertFleetHeartbeat does a read-then-write path, and fleet_devices has no UNIQUE(kind, name) constraint, so concurrent heartbeats can still create duplicate rows. Add the constraint and switch this to INSERT ... ON CONFLICT(kind, name) DO UPDATE.
🤖 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 `@internal/store/store.go` around lines 287 - 319, The UpsertFleetHeartbeat
path currently does a read-then-update flow on fleet_devices using kind and
name, which can race and still insert duplicates. Add a UNIQUE constraint on the
fleet_devices(kind, name) pair in the store schema/migration, then change
UpsertFleetHeartbeat to use a single atomic INSERT ... ON CONFLICT(kind, name)
DO UPDATE upsert instead of QueryRow plus UPDATE, keeping the existing
defaulting and service marshaling behavior.
Owner
Author
Review triage (CodeRabbit + internal review panel)Thanks for the thorough review. Here's how each finding was handled. Fixed in this PR
Deferred to Slice 7 (fleet finish) — tracked
Deferred to Slice 2 (earnings intelligence) — tracked
Minor / cosmetic — follow-up
This PR is scoped to "reconcile the WIP into a building, tested baseline"; the deferred items are real and map to already-planned slices. |
Address the review panel + CodeRabbit findings on the reconcile baseline: - Fleet API now defaults to loopback (127.0.0.1); LAN exposure is opt-in and warned (settings help + SECURITY.md). Empty FleetAPIKey rejects auth; heartbeat returns 400 on empty name/client_id and 500 (generic) on a store error instead of reflecting the raw error; /api/health is GET-only; the fleet HTTP server gets read/write/idle timeouts. - Add a sync.RWMutex to config.Manager — the fleet HTTP goroutine reads Config() concurrently with SaveSettings writes (go test -race). - Reconcile no longer deletes every deployment when the runtime returns an empty (but error-free) container list (e.g. a different Docker context). - config.load() now coerces on-disk values via applyDefaults (symmetry w/ Save). - Validate displayCurrency server-side against the supported set (closes the topbar self-XSS sink at its source). - CI: persist-credentials: false on checkout. Tests (go test -race ./... green on macOS/arm64): keychain file-fallback, load coercion, empty-list reconcile-keep, empty-key auth reject, name->client_id fallback, empty-name 400, health non-GET 405, config concurrency, ephemeral-port fleet bind + real request.
This was referenced Jul 5, 2026
This was referenced Aug 5, 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
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.
Slice 0 — reconcile the in-progress branch into a building, tested baseline
First slice of the desktop master plan (
docs/desktop-master-plan.md, #28). Lands the large uncommitted feature branch as a clean, compiling, tested baseline so every later slice builds on solid ground.What this lands
fleet_server.go): token-authenticated heartbeat receiver, compatible with the CashPilot server's worker protocol.Fixes to make it actually build
//go:embed build/appicon.pngicon the WIP had deleted (was a hard compile error).tscpasses andwails buildproduces a real.app.Tests + CI
ci.yml): build + vet + race test + coverage onubuntu-latest— the repo previously only ran on version tags.tray_*.go) ignored likeapp.go/main.go.Also
Verified live
Built and launched on an Apple-Silicon Mac mini; the app renders its onboarding and runs.
Code-signing is intentionally deferred for now.
Summary by CodeRabbit
New Features
Bug Fixes
Chores