feat(ui): add theme system and integration setup guides #10
Conversation
Major Features: - Add MIT LICENSE file - Implement dark/light theme system with persistent storage - Create comprehensive setup guides for Bolt, PuppetDB, and Puppetserver - Enhance integration status display and navigation Backend Changes: - Add theme configuration support (THEME_MODE, THEME_STORAGE) - Enhance Bolt plugin with priority-based integration selection - Improve Puppetserver certificate API error handling and retry logic - Update PuppetDB service with better error messages - Add theme endpoints to integration routes Frontend Changes: - Add BoltSetupGuide and PuppetdbSetupGuide components - Refactor PuppetserverSetupGuide with improved UX and step-by-step flow - Add theme switcher to navigation with system/light/dark modes - Improve integration status indicators with better visual feedback - Enhance certificate management UI Documentation: - Update all docs to reflect new theme feature - Enhance API endpoint documentation - Improve setup and authentication guides - Add todo items for circuit breaker and CA authorization fixes Code Quality: - Fix markdown linting issues in steering files - Remove duplicate H1 headings from documentation - Fix ordered list numbering in frontend README
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive theme system with dark/light mode support, adds detailed setup guides for all integrations (Bolt, PuppetDB, Puppetserver), enhances the Bolt integration with better configuration validation, implements PuppetDB circuit breaker configuration, adds certificate API fallback for PE 2025.3.0, updates the license from MIT to Apache-2.0, and improves documentation clarity around authentication methods (specifically noting that token authentication is Puppet Enterprise only).
Key Changes:
- Theme system with persistent storage (localStorage) and system preference detection
- New comprehensive setup guide components with interactive authentication method selection
- Bolt integration improvements with non-blocking validation and proper health checks
- PuppetDB circuit breaker configuration support
- Puppetserver certificate API fallback to PuppetDB when CA API unavailable
- Documentation updates clarifying PE-only vs open source authentication methods
Reviewed changes
Copilot reviewed 47 out of 49 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| LICENSE | New Apache-2.0 license file added |
| package.json, package-lock.json | License changed from MIT to Apache-2.0 |
| frontend/tailwind.config.js | Added dark mode class strategy |
| frontend/src/lib/theme.svelte.ts | New theme manager with system/light/dark modes |
| frontend/src/main.ts | Initialize theme manager on app startup |
| frontend/src/components/Navigation.svelte | Added theme toggle button |
| frontend/src/components/*SetupGuide.svelte | New/refactored comprehensive setup guides |
| frontend/src/App.svelte | Added footer with attribution |
| backend/src/server.ts | Non-blocking Bolt validation, conditional registration |
| backend/src/integrations/puppetserver/* | Certificate API fallback to PuppetDB |
| backend/src/integrations/puppetdb/* | Circuit breaker configuration support |
| backend/src/integrations/bolt/* | Enhanced health checks for Bolt availability |
| backend/src/config/* | PuppetDB circuit breaker schema and parsing |
| docs/* | Clarified PE-only token authentication across all docs |
| .kiro/steering/* | Removed duplicate H1 headings (markdown linting) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class="mt-8 text-center"> | ||
| <p class="text-gray-600 dark:text-gray-400"> | ||
| For detailed documentation, see <a | ||
| href="https://github.com/example42/pabawi/tree/main/docs/puppetdb-integration-setup.md" |
There was a problem hiding this comment.
The GitHub repository URL path is malformed. The URL "https://github.com/example42/pabawi/tree/main/docs/puppetdb-integration-setup.md" mixes the tree view path with a direct file reference. It should be "https://github.com/example42/pabawi/blob/main/docs/puppetdb-integration-setup.md" to link directly to the markdown file.
| ### Token-Based Authentication (PuppetDB) | ||
| PuppetDB supports token-based authentication using RBAC tokens from Puppet Enterprise or API tokens. | ||
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox require certificate-based authentication.** |
There was a problem hiding this comment.
The term "OpenVox" appears to be incorrect in the Puppet documentation context. This should be "Open Source Puppet" instead. OpenVox is a VoIP telecommunications company and is unrelated to Puppet infrastructure management.
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox require certificate-based authentication.** | |
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet requires certificate-based authentication.** |
| ## Authentication Setup | ||
|
|
||
| PuppetDB supports token-based authentication for API access. | ||
| **Important: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication.** |
There was a problem hiding this comment.
Same "OpenVox" typo - should be "Open Source Puppet".
| **Important: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication.** | |
| **Important: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet installations must use certificate-based authentication.** |
| ### Token-Based Authentication | ||
|
|
||
| Some integrations (PuppetDB, Puppetserver) support token-based authentication: | ||
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication.** |
There was a problem hiding this comment.
Same "OpenVox" typo - should be "Open Source Puppet".
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication.** | |
| **Note: Token-based authentication is only available with Puppet Enterprise. Open Source Puppet installations must use certificate-based authentication.** |
| this.puppetDBConfig.circuitBreaker?.resetTimeout ?? 60000, | ||
| this.puppetDBConfig.circuitBreaker?.timeout ?? this.puppetDBConfig.timeout, |
There was a problem hiding this comment.
The circuit breaker configuration uses nullish coalescing on optional nested properties, but the fallback values are not aligned with the schema defaults. According to the schema in lines 87-90 of backend/src/config/schema.ts, the defaults are threshold=5, timeout=60000, resetTimeout=30000. However, line 176 uses this.puppetDBConfig.timeout as the fallback for circuitBreaker.timeout instead of 60000, which could lead to inconsistent behavior if PUPPETDB_TIMEOUT is set but PUPPETDB_CIRCUIT_BREAKER_TIMEOUT is not.
| this.puppetDBConfig.circuitBreaker?.resetTimeout ?? 60000, | |
| this.puppetDBConfig.circuitBreaker?.timeout ?? this.puppetDBConfig.timeout, | |
| this.puppetDBConfig.circuitBreaker?.resetTimeout ?? 30000, | |
| this.puppetDBConfig.circuitBreaker?.timeout ?? 60000, |
| - `PUPPETDB_SSL_CA`: Path to CA certificate | ||
| - `PUPPETDB_CACHE_TTL`: Cache duration in ms (default: 300000) | ||
|
|
||
| **Important:** Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication. |
There was a problem hiding this comment.
Same "OpenVox" typo - should be "Open Source Puppet".
| **Important:** Token-based authentication is only available with Puppet Enterprise. Open Source Puppet and OpenVox installations must use certificate-based authentication. | |
| **Important:** Token-based authentication is only available with Puppet Enterprise. Open Source Puppet installations must use certificate-based authentication. |
frontend/src/main.ts
Outdated
| // Initialize theme manager | ||
| themeManager; |
There was a problem hiding this comment.
The theme manager is initialized but the result is unused. This statement has no effect and appears to be an attempt to ensure the singleton is created. Consider explicitly initializing the theme manager or documenting why this statement is necessary.
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="modal-title" | ||
| tabindex="-1" |
There was a problem hiding this comment.
The tabindex="-1" attribute on a dialog role element is incorrect for accessibility. Elements with dialog role should typically have tabindex="0" or be naturally focusable. A tabindex of -1 removes the element from the keyboard tab order, which can prevent proper keyboard navigation for screen reader users. Either remove the tabindex or set it to "0".
| function loadMetrics(): void { | ||
| if (showMetricsWarning) { | ||
| showMetricsWarning = false; | ||
| fetchMetrics(); | ||
| } else { | ||
| fetchMetrics(); | ||
| } |
There was a problem hiding this comment.
There's a duplicate call to fetchMetrics() in the loadMetrics function. Line 124 shows fetchMetrics() being called immediately after setting showMetricsWarning to false, then line 126 calls it again in the else block. The first call on line 124 should be removed as it's unnecessary - the warning flag is being set to false and then fetchMetrics is called again unconditionally.
| ssh: | ||
| user: admin | ||
| port: 22 | ||
| private-key: ~/.ssh/id_rsa | ||
| host-key-check: false`; |
There was a problem hiding this comment.
The SSH inventory example hard-codes host-key-check: false, which disables SSH host key verification and makes it trivial for a man-in-the-middle on the network to impersonate a target host and receive or modify commands and output. If users copy this example into production, any compromise of DNS, routing, or ARP on the path between Pabawi and the nodes can lead to command execution on attacker-controlled machines and data exfiltration. Use host-key-check: true by default and document that disabling host key checks should only be a last-resort troubleshooting step in tightly controlled environments, with a clear warning about the risk.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 71 out of 72 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Major Features:
Backend Changes:
Frontend Changes:
Documentation:
Code Quality: