Repository navigation
PowerNight 2.0.0 - Mobile UI & Task preset feature - #2
Conversation
Bumps React to 19, react-router-dom to v7, date-fns to v4, and all associated type definitions and dev tooling to current releases.
Introduces the TaskPreset table for storing reusable task templates. Adds TaskPresetService with CRUD operations and a preset_exists_by_name guard used during seeding. run_migration now seeds seven built-in presets (reserve levels, grid charging, operating modes, grid export) on startup if they are not already present.
Adds GET /tasks/presets, POST /tasks/presets, and DELETE /tasks/presets/<id> under the tasks blueprint. Built-in presets are protected from deletion (403). Input is validated against CommandType and CronCommand before persisting.
Planner page gains a preset selector (grouped by built-in vs user), "Save as Preset" modal, and delete button for user-defined presets. TaskPreset type and three api client methods (getTaskPresets, createTaskPreset, deleteTaskPreset) are added. Responsive fixes across the app: mobile hamburger nav in Header, horizontal scroll on LogsTable, touch-dismiss support in Tooltip, flex-wrap on Dashboard status bar and Settings form rows, and stacked action buttons in the Planner task table on small screens.
|
Important Review skippedToo many files! This PR contains 148 files, which is 48 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters: 1
📒 Files selected for processing: 148
You can disable this status message by setting the WalkthroughThis pull request introduces task preset management to PowerNight: a new database model, service layer, and API endpoints enable users to create, list, and delete reusable task templates. Frontend updates add responsive design improvements to multiple components and pages, along with dependency upgrades. Documentation plans for security hardening and UX improvements are also included. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Planner as Planner Page
participant API as API Routes
participant Service as TaskPresetService
participant DB as Database
User->>Planner: Load page / Select preset / Save as preset / Delete preset
Planner->>API: GET /api/v1/tasks/presets
API->>Service: list_presets()
Service->>DB: Query TaskPreset records
DB-->>Service: Return presets (ordered by sort_order, name)
Service-->>API: Return presets array + total
API-->>Planner: Return { success, presets, total }
User->>Planner: Fill form & click "Save as Preset"
Planner->>API: POST /api/v1/tasks/presets (name, command, command_params, default_time)
API->>API: Validate command, command_params, default_time
API->>Service: create_preset(...)
Service->>DB: Insert TaskPreset with is_builtin=false
DB-->>Service: Return created preset
Service-->>API: Return created preset dict
API-->>Planner: Return { success, preset }
Planner->>Planner: Refresh preset list, clear form
User->>Planner: Select preset to delete
Planner->>API: DELETE /api/v1/tasks/presets/<preset_id>
API->>Service: delete_preset(preset_id)
Service->>DB: Check is_builtin flag
alt is_builtin == true
Service-->>API: Raise DatabaseError (blocked)
API-->>Planner: Return { success: false, error: "...", status: 403 }
else is_builtin == false
DB->>DB: Delete preset record
Service-->>API: Return true
API-->>Planner: Return { success: true, status: 200 }
end
Planner->>Planner: Refresh presets, clear selection if needed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/powernight/core/database/services.py (1)
779-780: Consider preserving exception chain for debugging.The exception re-raising pattern loses the original traceback. Using
raise ... from ewould help with debugging.♻️ Optional improvement
except Exception as e: - raise DatabaseError(f"Failed to create preset: {e}") + raise DatabaseError(f"Failed to create preset: {e}") from eApply the same pattern to the other
except Exception as eblocks in this service.Also applies to: 790-791, 805-806, 824-825
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powernight/core/database/services.py` around lines 779 - 780, The DatabaseError raised in the except blocks currently discards the original traceback; update the re-raises to preserve the exception chain by using "raise DatabaseError(f'Failed to create preset: {e}') from e" (replace the message as appropriate) in the except block shown and apply the same pattern to the other handlers in this service (the other except Exception as e blocks that raise DatabaseError around the create/preset logic) so the original exception is chained to DatabaseError for proper debugging.src/powernight/web/api/tasks_api.py (1)
802-809: Fragile string matching to detect builtin preset deletion.The check
if "Built-in" in str(e)couples the API layer to the exact error message text fromTaskPresetService.delete_preset(). If the message changes, this will return 500 instead of 403.Consider introducing a specific exception type (e.g.,
BuiltinPresetError) for this case.♻️ Suggested approach
In
exceptions.py:class BuiltinPresetError(DatabaseError): """Raised when attempting to modify a built-in preset.""" passIn
services.py:if preset.is_builtin: - raise DatabaseError("Built-in presets cannot be deleted") + raise BuiltinPresetError("Built-in presets cannot be deleted")In
tasks_api.py:+ except BuiltinPresetError as e: + return jsonify({ + 'success': False, + 'error': 'Forbidden', + 'message': str(e), + 'timestamp': datetime.now(timezone.utc).isoformat() + }), 403 + except DatabaseError as e: - if "Built-in" in str(e): - return jsonify({ - 'success': False, - 'error': 'Forbidden', - 'message': str(e), - 'timestamp': datetime.now(timezone.utc).isoformat() - }), 403 return jsonify({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powernight/web/api/tasks_api.py` around lines 802 - 809, Replace the fragile string-match by introducing a dedicated exception and using it end-to-end: add BuiltinPresetError(DatabaseError) in exceptions.py, update TaskPresetService.delete_preset (or the method that raises DatabaseError on built-in deletion) to raise BuiltinPresetError instead of raising DatabaseError with the "Built-in" text, then in tasks_api.py import BuiltinPresetError and change the handler to except BuiltinPresetError as e: return the 403 JSON response (and keep a general except DatabaseError fallback for other DB errors); ensure imports are updated accordingly.src/powernight/web/src/pages/Planner.tsx (1)
69-76: Consider surfacing preset loading errors to users.Unlike
handleSavePresetandhandleDeletePresetwhich set the error state,loadPresetssilently logs errors to the console. If presets fail to load, users won't see any indication.♻️ Optional: Add error feedback
const loadPresets = async () => { try { const data = await api.getTaskPresets(); setPresets(data.presets || []); } catch (err) { console.error('Failed to load presets:', err); + // Optionally notify user, though this is non-critical + // setError('Failed to load presets'); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powernight/web/src/pages/Planner.tsx` around lines 69 - 76, loadPresets currently swallows errors by only logging to console; update its error handling to mirror handleSavePreset/handleDeletePreset by calling the component error setter (e.g., setError) inside the catch with a user-facing message (and optionally the err.message), and clear that error on successful load (e.g., call setError(null/undefined) before or after setPresets). Specifically modify the loadPresets function that calls api.getTaskPresets to setPresets(data.presets || []) on success and setError(...) on failure so users receive visible feedback when presets fail to load.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/powernight/web/src/components/Header.tsx`:
- Around line 55-60: The mobile nav toggle removed the browser focus ring making
keyboard use impossible; in the Header component update the toggle button
(onClick={() => setMenuOpen(!menuOpen)}, aria-expanded={menuOpen}) and the
mobile menu items to restore a visible keyboard focus state instead of using
focus:outline-none — either remove that utility or replace it with an accessible
focus style (e.g. focus-visible:ring / focus-visible:outline or a clear
background/border change) and apply the same visible focus treatment to the
corresponding menu item elements rendered in the collapse (also present around
the elements referenced near the other block at lines showing the same pattern).
Ensure the aria-expanded logic and setMenuOpen remain unchanged while only
altering classes/styles for keyboard focus visibility.
In `@src/powernight/web/src/components/Tooltip.tsx`:
- Around line 112-124: The touch handler handleTouchStart currently calls
e.preventDefault(), which blocks follow-up clicks on touch devices and breaks
interactive children; remove the e.preventDefault() call (or conditionally skip
it for interactive elements) and simply toggle setIsVisible((prev) => !prev) so
the tooltip still opens on touch without cancelling the native click for
links/buttons wrapped by triggerRef.
In `@src/powernight/web/src/utils/api.ts`:
- Around line 345-356: The createTaskPreset method references the CommandType
type but it isn't imported, causing a TypeScript error; add an import for
CommandType from the module that exports your types (e.g., import { CommandType
} from '../types') at the top of the file so the type annotation on
createTaskPreset is resolved; ensure the import is included alongside other
imports in src/powernight/web/src/utils/api.ts and that the symbol name exactly
matches CommandType used in the function signature.
---
Nitpick comments:
In `@src/powernight/core/database/services.py`:
- Around line 779-780: The DatabaseError raised in the except blocks currently
discards the original traceback; update the re-raises to preserve the exception
chain by using "raise DatabaseError(f'Failed to create preset: {e}') from e"
(replace the message as appropriate) in the except block shown and apply the
same pattern to the other handlers in this service (the other except Exception
as e blocks that raise DatabaseError around the create/preset logic) so the
original exception is chained to DatabaseError for proper debugging.
In `@src/powernight/web/api/tasks_api.py`:
- Around line 802-809: Replace the fragile string-match by introducing a
dedicated exception and using it end-to-end: add
BuiltinPresetError(DatabaseError) in exceptions.py, update
TaskPresetService.delete_preset (or the method that raises DatabaseError on
built-in deletion) to raise BuiltinPresetError instead of raising DatabaseError
with the "Built-in" text, then in tasks_api.py import BuiltinPresetError and
change the handler to except BuiltinPresetError as e: return the 403 JSON
response (and keep a general except DatabaseError fallback for other DB errors);
ensure imports are updated accordingly.
In `@src/powernight/web/src/pages/Planner.tsx`:
- Around line 69-76: loadPresets currently swallows errors by only logging to
console; update its error handling to mirror handleSavePreset/handleDeletePreset
by calling the component error setter (e.g., setError) inside the catch with a
user-facing message (and optionally the err.message), and clear that error on
successful load (e.g., call setError(null/undefined) before or after
setPresets). Specifically modify the loadPresets function that calls
api.getTaskPresets to setPresets(data.presets || []) on success and
setError(...) on failure so users receive visible feedback when presets fail to
load.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9dc854b8-866c-4b0e-a5cc-09913467a6e8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
docs/plans/2026-04-25-security-remediation.mddocs/plans/ux-improvement-roadmap.mdpackage.jsonsrc/powernight/core/database/__init__.pysrc/powernight/core/database/exceptions.pysrc/powernight/core/database/migration.pysrc/powernight/core/database/models.pysrc/powernight/core/database/services.pysrc/powernight/web/api/tasks_api.pysrc/powernight/web/src/components/Header.tsxsrc/powernight/web/src/components/LogsTable.tsxsrc/powernight/web/src/components/Tooltip.tsxsrc/powernight/web/src/pages/Dashboard.tsxsrc/powernight/web/src/pages/Planner.tsxsrc/powernight/web/src/pages/Settings.tsxsrc/powernight/web/src/types/index.tssrc/powernight/web/src/utils/api.ts
There was a problem hiding this comment.
Pull request overview
Adds a “Task Preset” feature spanning database, backend API, and Planner UI to support reusable predefined task configurations, alongside several responsive UX refinements and planning docs updates.
Changes:
- Introduces
TaskPresetpersistence (model/service) and seeds built-in presets during migration. - Adds REST endpoints for listing/creating/deleting presets and wires them into the React Planner UI.
- Improves mobile/responsive UX across Header/Planner/Dashboard/LogsTable and updates Tooltip touch behavior; updates frontend dependencies.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/powernight/web/src/utils/api.ts | Adds typed client methods for task preset endpoints. |
| src/powernight/web/src/types/index.ts | Introduces TaskPreset TypeScript type. |
| src/powernight/web/src/pages/Settings.tsx | Improves responsive form layout for inputs/buttons. |
| src/powernight/web/src/pages/Planner.tsx | Adds preset loading/selection, “save as preset” modal, and responsive table tweaks. |
| src/powernight/web/src/pages/Dashboard.tsx | Makes section header layout wrap better on smaller screens. |
| src/powernight/web/src/components/Tooltip.tsx | Adds touch toggle + outside-tap dismissal behavior. |
| src/powernight/web/src/components/LogsTable.tsx | Adds horizontal scrolling container + min column widths for mobile. |
| src/powernight/web/src/components/Header.tsx | Adds mobile hamburger menu and hides desktop nav on small screens. |
| src/powernight/web/api/tasks_api.py | Adds /tasks/presets GET/POST and /tasks/presets/<id> DELETE endpoints. |
| src/powernight/core/database/services.py | Adds TaskPresetService for CRUD operations. |
| src/powernight/core/database/models.py | Adds TaskPreset SQLAlchemy model + serialization. |
| src/powernight/core/database/migration.py | Seeds built-in presets during migration run. |
| src/powernight/core/database/exceptions.py | Adds PresetNotFoundError. |
| src/powernight/core/database/init.py | Exports TaskPreset and PresetNotFoundError. |
| package.json | Updates frontend dependencies/tooling versions. |
| docs/plans/ux-improvement-roadmap.md | Adds UX roadmap planning document. |
| docs/plans/2026-04-25-security-remediation.md | Adds security remediation planning document. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {selectedPresetId && (() => { | ||
| const selected = presets.find(p => p.id === selectedPresetId); | ||
| return selected && !selected.is_builtin ? ( | ||
| <button | ||
| type="button" | ||
| onClick={() => handleDeletePreset(selectedPresetId)} | ||
| className="px-3 py-2 text-red-600 border border-red-300 rounded-md hover:bg-red-50" | ||
| title="Delete this preset" | ||
| > | ||
| Delete | ||
| </button> | ||
| ) : null; | ||
| })()} |
There was a problem hiding this comment.
Using an IIFE inside JSX ({selectedPresetId && (() => { ... })()}) makes this render path harder to read/debug and is easy to accidentally break with hooks/state. Consider computing selectedPreset once (e.g., via a local const or useMemo) and rendering conditionally without an inline function call.
| {selectedPresetId && (() => { | |
| const selected = presets.find(p => p.id === selectedPresetId); | |
| return selected && !selected.is_builtin ? ( | |
| <button | |
| type="button" | |
| onClick={() => handleDeletePreset(selectedPresetId)} | |
| className="px-3 py-2 text-red-600 border border-red-300 rounded-md hover:bg-red-50" | |
| title="Delete this preset" | |
| > | |
| Delete | |
| </button> | |
| ) : null; | |
| })()} | |
| {selectedPresetId && presets.find(p => p.id === selectedPresetId)?.is_builtin === false && ( | |
| <button | |
| type="button" | |
| onClick={() => handleDeletePreset(selectedPresetId)} | |
| className="px-3 py-2 text-red-600 border border-red-300 rounded-md hover:bg-red-50" | |
| title="Delete this preset" | |
| > | |
| Delete | |
| </button> | |
| )} |
| if not request.is_json: | ||
| raise ValidationError("Request must contain JSON data") | ||
|
|
||
| data = request.get_json() | ||
|
|
||
| # Validate required fields | ||
| if not data.get('name', '').strip(): | ||
| raise ValidationError("Preset name is required") |
There was a problem hiding this comment.
request.get_json() can return None (or raise on invalid JSON) even when request.is_json is true. Consider using request.get_json(silent=True) and explicitly validating data is a dict before accessing .get(...) to avoid returning a 500 for malformed bodies.
| <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"> | ||
| <div className="bg-white p-6 rounded-lg shadow-xl max-w-md w-full mx-4"> | ||
| <h3 className="text-lg font-semibold mb-4">Save as Preset</h3> | ||
| <div className="space-y-4"> | ||
| <div> | ||
| <label className="block text-sm font-medium text-gray-700 mb-1"> | ||
| Preset Name | ||
| </label> | ||
| <input |
There was a problem hiding this comment.
The new "Save as Preset" modal is implemented inline here, duplicating modal markup/behavior (overlay, close handling, keyboard behavior) and lacks basic dialog semantics (role="dialog", aria-modal, labelled-by). Consider reusing a shared modal component (or introducing one) to ensure consistent accessibility and interaction patterns.
| <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"> | |
| <div className="bg-white p-6 rounded-lg shadow-xl max-w-md w-full mx-4"> | |
| <h3 className="text-lg font-semibold mb-4">Save as Preset</h3> | |
| <div className="space-y-4"> | |
| <div> | |
| <label className="block text-sm font-medium text-gray-700 mb-1"> | |
| Preset Name | |
| </label> | |
| <input | |
| <div | |
| className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" | |
| onClick={(e) => { | |
| if (e.target === e.currentTarget) { | |
| setShowSavePresetModal(false); | |
| } | |
| }} | |
| > | |
| <div | |
| className="bg-white p-6 rounded-lg shadow-xl max-w-md w-full mx-4" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-labelledby="save-preset-modal-title" | |
| onKeyDown={(e) => { | |
| if (e.key === 'Escape') { | |
| setShowSavePresetModal(false); | |
| } | |
| }} | |
| > | |
| <h3 id="save-preset-modal-title" className="text-lg font-semibold mb-4">Save as Preset</h3> | |
| <div className="space-y-4"> | |
| <div> | |
| <label htmlFor="preset-name-input" className="block text-sm font-medium text-gray-700 mb-1"> | |
| Preset Name | |
| </label> | |
| <input | |
| id="preset-name-input" |
| # Validate command parameters if provided | ||
| command_params = data.get('command_params', {}) | ||
| if command_params: | ||
| command = CronCommand(data['command'], command_params) | ||
| validation = command.validate() | ||
| if not validation.valid: | ||
| raise ValidationError(f"Invalid command parameters: {', '.join(validation.errors)}") |
There was a problem hiding this comment.
command_params is assumed to be an object/dict, but a non-dict JSON value (string/array) will cause CronCommand(...).validate() to throw and be reported as a 500. Add a type check (e.g., ensure isinstance(command_params, dict)) and return a 422 Validation Error when it's not a mapping.
| except Exception as e: | ||
| current_app.logger.error(f"Failed to list presets: {e}") | ||
| return jsonify({ | ||
| 'success': False, | ||
| 'error': 'Internal Server Error', | ||
| 'message': f'Failed to list presets: {e}', | ||
| 'timestamp': datetime.now(timezone.utc).isoformat() | ||
| }), 500 |
There was a problem hiding this comment.
These error responses include the raw exception text ({e}) in the JSON payload. This can leak internal details to clients; log the exception server-side, but return a generic public message in the response body.
| except Exception as e: | ||
| current_app.logger.error(f"Failed to create preset: {e}") | ||
| return jsonify({ | ||
| 'success': False, | ||
| 'error': 'Internal Server Error', | ||
| 'message': f'Failed to create preset: {e}', | ||
| 'timestamp': datetime.now(timezone.utc).isoformat() | ||
| }), 500 |
There was a problem hiding this comment.
The 500 handler returns message: f'Failed to create preset: {e}', exposing internal exception details to the client. Prefer returning a generic message (and optionally a request ID) while logging the full exception server-side.
| except DatabaseError as e: | ||
| if "Built-in" in str(e): | ||
| return jsonify({ | ||
| 'success': False, | ||
| 'error': 'Forbidden', | ||
| 'message': str(e), | ||
| 'timestamp': datetime.now(timezone.utc).isoformat() | ||
| }), 403 |
There was a problem hiding this comment.
Detecting the "built-in preset" case via substring matching on str(e) is brittle (message changes will break behavior). Prefer raising/catching a dedicated exception for forbidden built-in deletion (or an explicit error code) and mapping that to a 403 here.
| const handleTouchStart = (e: React.TouchEvent) => { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
Calling preventDefault() on touchstart can block native scrolling and other default touch interactions on mobile when the user starts a scroll gesture on a tooltip trigger. Consider removing preventDefault (or only preventing default for specific non-scroll cases) and using a click/pointer handler to toggle visibility.
| const handleTouchStart = (e: React.TouchEvent) => { | |
| e.preventDefault(); | |
| const handleTouchStart = () => { |
| <div className="flex items-center md:hidden"> | ||
| <button | ||
| onClick={() => setMenuOpen(!menuOpen)} | ||
| className="text-white p-2 rounded-md hover:bg-white/10 focus:outline-none" |
There was a problem hiding this comment.
The mobile menu button removes the default focus outline (focus:outline-none) but doesn't add a replacement focus style. Add a visible focus indicator (e.g., focus-visible:ring-*) so keyboard users can see focus location.
| className="text-white p-2 rounded-md hover:bg-white/10 focus:outline-none" | |
| className="text-white p-2 rounded-md hover:bg-white/10 focus:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 focus-visible:ring-offset-blue-600" |
Raise Python floors (Flask 3, Flask-CORS 6, cryptography 49, mypy 2, pytest 9, etc.) in pyproject.toml and requirements.txt, and bump all frontend packages (ESLint 10, Vite 8, Vitest 4, TypeScript 6) with a regenerated lockfile. Migrate ESLint to flat config (eslint.config.js) since v9+ requires it, and migrate the Vitest harness from the jest global to vi so the suite can run. Bump the Docker base image to python:3.13-slim and update all GitHub Actions pins and node/python versions. Also bump the app version to 2.0.0. Verified via type-check, production build, Docker build, and a container /health boot smoke test.
Part of the 2.0.0 release. The /health, /version, status, and OpenAPI docs endpoints hardcoded the version string, causing it to drift from pyproject.toml. They now import powernight.__version__ so future bumps only touch pyproject.toml and __init__.py. Updates the illustrative JSON in CLAUDE.md and docs/FAQ.md to match. Note: the canonical version declarations (pyproject.toml, package.json, package-lock.json, docker-compose.yml, __init__.py) were already committed to 2.0.0 in aa1083c by a parallel dependency-update commit.
…urrency, secure API by default, green + CI-enforced tests A full audit found the instability rooted in ~11,600 lines of dead or fictional code and a handful of verified runtime defects. This change: - Deletes ~15 permanently-500 API endpoints (undefined helpers), demo stubs, and seven dead modules; frontend mirrors removed with them - Fixes the broken config-write path (single consolidated config API) - Makes config recovery non-destructive: restore verified backups only, never overwrite the user's file with fabricated defaults; fail fast - Runs scheduled task execution in a background thread so a slow task no longer blocks the scheduler; locks the DB-manager singleton; makes preset seeding idempotent with a unique constraint - Rewrites the connector fallback paths to serve stale cache instead of crashing with AttributeError when the Powerwall is unreachable - Secures the API by default: auth_enabled=true with fail-closed boot, require_auth on sensitive endpoints, security headers, rate limiting, persisted SECRET_KEY, 0o600 token permissions, compose requires POWERNIGHT_API_KEY - Rehabilitates the test suites (265 backend + 31 frontend tests green) and adds ci.yml enforcing pytest, a ruff undefined-name gate, lint, type-check, vitest, and build on every PR - Adds frontend stability fixes and UX quick wins (poll cleanup, param validation, ConfirmDialog, toasts, auto-refresh, health indicator, History filters) - Reconciles CLAUDE.md/AGENTS.md/docs with the actual code and prunes ten unused dependencies
| self.storage_path.mkdir(parents=True, exist_ok=True) | ||
| try: | ||
| os.chmod(self.storage_path, 0o700) | ||
| except OSError: |
|
|
||
| from powernight.web import create_app | ||
| from powernight.core.config import PowerNightConfig, PowerwallSettings, AutomationSettings, WebInterfaceSettings, LoggingSettings, MonitoringSettings | ||
| import pytest |
| # Get profile information from headers | ||
| profile_id = request.headers.get('X-Powerwall-Profile', 'demo') | ||
| # Check if we're in demo mode (cloud mode only) | ||
| config = get_config() |
| """Log a single warning when requests are served with auth disabled.""" | ||
| global _auth_disabled_warned | ||
| if not _auth_disabled_warned: | ||
| _auth_disabled_warned = True |
Allow PowerNight to start without credentials on trusted networks while automatically enabling protection when an API key or complete Basic credentials are configured. Keep explicit authentication fail-closed, repair frontend API-key handling, and update Compose, documentation, and test coverage.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Move project setup, run, and archive commands into the repository Conductor settings so workspaces use consistent scripts without relying on local-only configuration. Co-authored-by: Cursor <cursoragent@cursor.com>
Upgrade npm and Python dependencies, CI actions, and runtime images to stable releases. Raise minimum Python support to 3.11 and retain TypeScript 6.0.3 for supported ESLint compatibility. Adapt browser test setup for jsdom 30 and generate build dependency metadata from manifests. Validate unit tests, clean installations, production builds, and isolated Docker/browser smoke checks; document existing dashboard E2E and informational typing failures in the verification handoff.
Return 503 with TESLA_AUTH_REQUIRED when Tesla authorization is unavailable, while preserving app-level 401 handling and access to Settings. Restore authentication on configuration reads and cover missing tokens, failed refresh, key retention, and desktop/mobile onboarding with regression tests.
Track file-owned values separately from effective runtime configuration. Persist only user edits, preserve environment precedence, and omit override values from logs. Cover repeated saves, backups, and both config endpoints.
Resolve include paths relative to the frontend tsconfig so source and tests are actually checked. Fix the exposed type errors and Vitest matcher types, and preserve zero-valued command parameters in controlled inputs.
Translate duplicate names into a typed conflict and HTTP 409 response. Keep unexpected database details out of API responses and let concurrent built-in seeding tolerate the same conflict. Add service and API coverage.
Protect cached connection state with a short-lived lock that is never held during network calls. Use cached state for health polling and verify status reads remain responsive while mocked Tesla connection requests are blocked.
Keep connection checks, rate limiting, and command dispatch under the same reentrant operation lock so disconnect waits for in-flight commands. Cover reserve, mode, grid charging, export, and state-of-charge operations.
There was a problem hiding this comment.
9 issues found across 134 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/unit/test_web_app_paths_simple.py">
<violation number="1" location="tests/unit/test_web_app_paths_simple.py:42">
P2: Four of the five tests now run the real `init_auth_api`, so each `create_app` call creates the auth storage directory (`data/` by default, or `POWERNIGHT_DATA_PATH`) on disk. The host environment is also no longer cleared, so an inherited `POWERNIGHT_DATA_PATH` (e.g. `/data` or `/app/data`) can make these tests fail with PermissionError or write into real data. Restore the `init_auth_api` patch (for example in a fixture) and clear the relevant env vars in every test, not just `POWERNIGHT_STATIC_PATH`.</violation>
</file>
<file name="docs/SECURITY.md">
<violation number="1" location="docs/SECURITY.md:8">
P2: This sentence is inaccurate. Several API routes are unauthenticated even when auth is enabled, including `/api/v1/version-info.json`, `GET /api/v1/config/timezone(s)`, and the `/api/auth/setup/*` routes. Reword it to list the actual public endpoints, or restate it as the intended public set after confirming the code matches.</violation>
</file>
<file name="src/powernight/core/database/migration.py">
<violation number="1" location="src/powernight/core/database/migration.py:229">
P2: One transient failure while seeding aborts seeding for all remaining built-in presets. The inner `except` catches only `PresetNameConflictError`, so a `DatabaseError` from `create_preset` exits the loop. Catch `DatabaseError` per preset, log it, and continue. Import `DatabaseError` from `.exceptions` to do that.</violation>
</file>
<file name="src/powernight/web/middleware.py">
<violation number="1" location="src/powernight/web/middleware.py:34">
P2: `_SlidingWindowRateLimiter._hits` never drops keys for clients whose timestamps have all aged out, so memory grows with every distinct `remote_addr` that ever hits a limited path. Sweep stale entries before the per-client check.</violation>
</file>
<file name="src/powernight/web/api/config_api.py">
<violation number="1" location="src/powernight/web/api/config_api.py:41">
P2: This fallback never runs for `GET /api/v1/config`. `@require_auth` calls `get_config()` first and turns the same "no configuration loaded" error into a 401, so the request fails before `get_app_config()` is reached. Either load config before auth runs, or drop the fallback and the comment that describes it.</violation>
<violation number="2" location="src/powernight/web/api/config_api.py:103">
P2: Client-side validation errors from `save_config()` come back as 500s. Bad `automation.timezone` or `monitoring` values pass `validate_config_data()` and fail later in `ConfigurationError`, which this handler reports as a server error with the details dropped. Catch `ConfigurationError` and return 400 with its message, or add timezone and monitoring checks to `validate_config_data()`.</violation>
</file>
<file name="src/powernight/web/src/components/Tooltip.tsx">
<violation number="1" location="src/powernight/web/src/components/Tooltip.tsx:126">
P2: Toggling on `touchstart` also fires when a touch starts a scroll, so scrolling the logs table on mobile will open tooltips on every cell the finger touches. Open on tap instead (for example via `onClick`), and check that desktop hover still behaves correctly.</violation>
</file>
<file name="src/powernight/web/api/api.py">
<violation number="1" location="src/powernight/web/api/api.py:502">
P2: This decorator makes `/api/v1/health` return 401 to unauthenticated monitoring probes and the Header health poll. Remove the decorator or whitelist the full API path.</violation>
</file>
<file name="src/powernight/web/eslint.config.js">
<violation number="1" location="src/powernight/web/eslint.config.js:16">
P2: The new lint config does not declare Vitest globals, so `npm run lint` reports `describe`, `it`, `expect`, and `vi` as undefined in the test suite. Add the Vitest globals to the test-file ESLint configuration.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
|
||
| # Status and monitoring endpoints | ||
| @api_blueprint.route('/status', methods=['GET']) | ||
| @require_auth |
There was a problem hiding this comment.
P2: This decorator makes /api/v1/health return 401 to unauthenticated monitoring probes and the Header health poll. Remove the decorator or whitelist the full API path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/powernight/web/api/api.py, line 502:
<comment>This decorator makes `/api/v1/health` return 401 to unauthenticated monitoring probes and the Header health poll. Remove the decorator or whitelist the full API path.</comment>
<file context>
@@ -930,6 +499,7 @@ def set_backup_reserve():
# Status and monitoring endpoints
@api_blueprint.route('/status', methods=['GET'])
+@require_auth
def get_system_status():
"""
</file context>
Publish stable version-only image tags and validate packaged versions. Add a separate manual promotion workflow that verifies both registries and required platforms before promoting the tested digest without rebuilding.
Exercise open and protected API modes with disposable Compose projects and synthetic legacy data. Verify migrations, preset conflicts, environment-secret isolation, and persistence across container recreation; reuse these checks in Docker Health Test.
Record local validation, migration and config-persistence caveats, staged publication, live canary gates, digest promotion, and backup-based rollback. Keep publication and production rollout outside the approved local-only validation scope.
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
3 existing issues remain and 2 new issues found across 76 files (changes from recent commits).
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/powernight/web/test/pages/Planner.test.tsx">
<violation number="1" location="src/powernight/web/test/pages/Planner.test.tsx:117">
P3: The test title says it verifies retry, but nothing is retried. It only checks that Save is re-enabled after the failure. Either rename it to reflect what is asserted, or add a second `createTaskPreset` resolve, click Save again, and assert the dialog closes and the call happened twice.</violation>
</file>
<file name="src/powernight/core/database/migration.py">
<violation number="1" location="src/powernight/core/database/migration.py:240">
P3: The new `return not failed` is never checked in production. `upgrade()` drops the `run_migration()` result, so a failed preset seed only appears as a log line and startup still reports "Database initialization complete". If partial seeding should be visible, check the return value in `upgrade()` and log a warning or raise `DatabaseMigrationError`.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
| logger.exception("Failed to seed preset %s", preset_data['name']) | ||
|
|
||
| logger.info("Built-in preset seeding completed") | ||
| return not failed |
There was a problem hiding this comment.
P3: The new return not failed is never checked in production. upgrade() drops the run_migration() result, so a failed preset seed only appears as a log line and startup still reports "Database initialization complete". If partial seeding should be visible, check the return value in upgrade() and log a warning or raise DatabaseMigrationError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/powernight/core/database/migration.py, line 240:
<comment>The new `return not failed` is never checked in production. `upgrade()` drops the `run_migration()` result, so a failed preset seed only appears as a log line and startup still reports "Database initialization complete". If partial seeding should be visible, check the return value in `upgrade()` and log a warning or raise `DatabaseMigrationError`.</comment>
<file context>
@@ -231,8 +232,12 @@ def seed_builtin_presets() -> bool:
+
logger.info("Built-in preset seeding completed")
- return True
+ return not failed
except Exception as e:
logger.error(f"Failed to seed built-in presets: {e}")
</file context>
Summary
Test plan
/api/v1/tasks/presetsreturns seeded preset datapytest tests/to confirm no backend regressionsnpm run testandnpm run type-checkfor frontend checksSummary by CodeRabbit
New Features
Documentation
Chores