feat(ui): Add system path information#1443
Conversation
WalkthroughThe PR relocates the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15–20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ui/src/pages/system-status/index.tsx (1)
7-8: Consider adding accessible label to icon-only Refresh buttonThe Refresh button is now icon-only, which can be unclear for screen readers and keyboard users. Suggest adding an
aria-label(and optionaltitle) while keeping the compact style.Example diff for Lines 75-86:
- <Button - variant="outline" - size="sm" - onClick={handleRefresh} - disabled={isRefreshing} - className="h-7 px-2" - > - <RefreshCw className={cn( - "h-3 w-3", - isRefreshing && "animate-spin" - )} /> - </Button> + <Button + variant="outline" + size="sm" + onClick={handleRefresh} + disabled={isRefreshing} + className="h-7 px-2" + aria-label="Refresh status" + title="Refresh status" + > + <RefreshCw + className={cn("h-3 w-3", isRefreshing && "animate-spin")} + /> + </Button>Also applies to: 57-87
ui/src/features/system-status/components/PathsCard.tsx (1)
7-52: Solid Paths UI; consider small a11y improvements for dialog and copy buttonThe overall structure (PathItem, PathsDialog, PathsCard) is clear and matches the new
pathsconfig. A few tweaks would improve accessibility and keyboard use without changing behavior:
- Make the dialog a proper ARIA dialog and support Escape key to close
Add dialog roles/labels and an Escape handler:function PathsDialog({ open, onClose }: { open: boolean; onClose: () => void }) { const config = useConfig(); const paths: PathsConfig = config.paths; + + React.useEffect(() => { + if (!open) return; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + onClose(); + } + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [open, onClose]); @@ - return ( - <div className="fixed inset-0 z-50 flex items-center justify-center"> + return ( + <div + className="fixed inset-0 z-50 flex items-center justify-center" + role="dialog" + aria-modal="true" + aria-labelledby="paths-dialog-title" + > @@ - <span className="text-xs font-medium">System Paths</span> + <span id="paths-dialog-title" className="text-xs font-medium"> + System Paths + </span>
- Ensure the copy button is visible when focused and labeled for screen readers
Right now it appears only on hover; keyboard focus still leaves it invisible.- <button - onClick={handleCopy} - className={cn( - 'p-0.5 rounded transition-all shrink-0', - 'opacity-0 group-hover:opacity-100', - 'hover:bg-slate-200 dark:hover:bg-slate-700', - copied && 'opacity-100 text-green-500' - )} - title="Copy path" - > + <button + type="button" + onClick={handleCopy} + aria-label="Copy path" + className={cn( + 'p-0.5 rounded transition-all shrink-0', + 'opacity-0 group-hover:opacity-100 focus-visible:opacity-100', + 'hover:bg-slate-200 dark:hover:bg-slate-700', + copied && 'opacity-100 text-green-500' + )} + title="Copy path" + >These changes keep the compact design while better matching the keyboard navigation and a11y guidelines.
Also applies to: 53-107, 109-126
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
internal/cmd/context.go(1 hunks)internal/common/config/config.go(1 hunks)internal/common/config/context.go(1 hunks)internal/common/config/context_test.go(1 hunks)internal/common/config/loader.go(1 hunks)internal/common/config/loader_test.go(3 hunks)internal/runtime/subcmd.go(1 hunks)internal/runtime/subcmd_test.go(9 hunks)internal/service/frontend/api/v2/dags.go(0 hunks)internal/service/frontend/server.go(1 hunks)internal/service/frontend/templates.go(2 hunks)internal/service/frontend/templates/base.gohtml(1 hunks)internal/test/command.go(2 hunks)internal/test/helper.go(1 hunks)ui/index.html(1 hunks)ui/src/contexts/ConfigContext.tsx(2 hunks)ui/src/features/system-status/components/PathsCard.tsx(1 hunks)ui/src/pages/system-status/index.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- internal/service/frontend/api/v2/dags.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/common/config/config.gointernal/test/helper.gointernal/common/config/context.gointernal/test/command.gointernal/service/frontend/server.gointernal/common/config/loader_test.gointernal/runtime/subcmd.gointernal/cmd/context.gointernal/common/config/loader.gointernal/service/frontend/templates.gointernal/common/config/context_test.gointernal/runtime/subcmd_test.go
ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{ts,tsx,jsx,js}: Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Support both light and dark modes for all UI components using Tailwind CSS class pairs likedark:bg-slate-700
NEVER use full-page loading overlays or LoadingIndicator components that hide content - show stale data while fetching updates instead
Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text withwhitespace-normal break-words
Use consistent metadata styling withbg-slate-200 dark:bg-slate-700backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, account for fixed elements when setting heights
Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels
Files:
ui/src/features/system-status/components/PathsCard.tsxui/src/contexts/ConfigContext.tsxui/src/pages/system-status/index.tsx
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/features/system-status/components/PathsCard.tsxui/src/contexts/ConfigContext.tsxui/src/pages/system-status/index.tsx
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Co-locate Go tests as*_test.go; favour table-driven cases and cover failure paths
Usestretchr/testify/requireand shared fixtures frominternal/testinstead of duplicating mocks
Files:
internal/common/config/loader_test.gointernal/common/config/context_test.gointernal/runtime/subcmd_test.go
🧠 Learnings (4)
📚 Learning: 2025-12-04T10:34:01.023Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.023Z
Learning: Applies to ui/~/.config/dagu/config.yaml : Dagu configuration file supports environment variables with DAGU_* prefix, per-DAG overrides, and command-line arguments
Applied to files:
internal/test/helper.goui/index.htmlinternal/runtime/subcmd_test.go
📚 Learning: 2025-12-04T10:34:01.023Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.023Z
Learning: Applies to ui/**/*.{yaml,yml} : Dagu YAML configurations should support minimal setup with sensible defaults and support for environment variable expansion and command substitution
Applied to files:
ui/index.html
📚 Learning: 2025-12-04T10:34:17.051Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.051Z
Learning: Applies to **/*_test.go : Co-locate Go tests as `*_test.go`; favour table-driven cases and cover failure paths
Applied to files:
internal/common/config/loader_test.gointernal/common/config/context_test.go
📚 Learning: 2025-12-04T10:34:17.051Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.051Z
Learning: Applies to **/*_test.go : Use `stretchr/testify/require` and shared fixtures from `internal/test` instead of duplicating mocks
Applied to files:
internal/common/config/loader_test.gointernal/common/config/context_test.go
🧬 Code graph analysis (14)
internal/common/config/config.go (1)
internal/common/config/context.go (1)
ConfigFileUsed(26-31)
ui/src/features/system-status/components/PathsCard.tsx (3)
ui/src/lib/utils.ts (1)
cn(4-6)ui/src/contexts/ConfigContext.tsx (2)
useConfig(35-37)PathsConfig(3-14)ui/src/components/ui/button.tsx (1)
Button(59-59)
internal/test/helper.go (2)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
ui/src/contexts/ConfigContext.tsx (1)
internal/common/config/config.go (1)
PathsConfig(144-157)
internal/common/config/context.go (1)
internal/common/config/path.go (1)
Paths(13-30)
internal/test/command.go (2)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
internal/service/frontend/server.go (1)
internal/common/config/path.go (1)
Paths(13-30)
internal/common/config/loader_test.go (2)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
internal/runtime/subcmd.go (2)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
internal/cmd/context.go (4)
internal/common/config/config.go (1)
Config(9-36)internal/common/logger/tag/tag.go (1)
Config(354-356)internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
internal/common/config/loader.go (2)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
internal/service/frontend/templates.go (4)
internal/common/config/path.go (1)
Paths(13-30)internal/common/config/config.go (1)
PathsConfig(144-157)ui/src/contexts/ConfigContext.tsx (1)
PathsConfig(3-14)internal/common/config/context.go (1)
ConfigFileUsed(26-31)
ui/src/pages/system-status/index.tsx (2)
ui/src/components/ui/button.tsx (1)
Button(59-59)ui/src/lib/utils.ts (1)
cn(4-6)
internal/runtime/subcmd_test.go (3)
internal/common/config/context.go (1)
ConfigFileUsed(26-31)internal/common/config/config.go (1)
Global(38-66)internal/common/config/env.go (2)
BaseEnv(9-11)NewBaseEnv(15-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Test on ubuntu-latest
🔇 Additional comments (16)
internal/common/config/config.go (1)
156-156: LGTM! Logical placement for config file path.Moving
ConfigFileUsedtoPathsConfigimproves code organization by grouping all path-related configuration together. The field is well-documented and maintains consistent visibility.internal/common/config/loader_test.go (1)
132-133: LGTM! Test updates align with the refactoring.All test assertions and setup code correctly updated to reference
cfg.Paths.ConfigFileUsedinstead ofcfg.Global.ConfigFileUsed. Test coverage for configuration file path tracking is maintained.Also applies to: 714-714, 734-734
internal/service/frontend/server.go (1)
65-65: LGTM! Clean extension of frontend configuration.Adding
PathstofuncsConfigprovides the frontend with access to path configuration, enabling the UI to display system path information. The change is consistent with how other config sections are passed to the frontend.internal/common/config/context.go (1)
28-28: LGTM! Accessor function correctly updated.The
ConfigFileUsedfunction now returns the value from the new location (cfg.Paths.ConfigFileUsed) while maintaining the same API signature. This abstraction shields callers from the internal refactoring.internal/cmd/context.go (1)
151-151: LGTM! Logging statement correctly updated.The debug log now references
cfg.Paths.ConfigFileUsedinstead of the old location. The structured logging output remains unchanged for consumers.internal/common/config/loader.go (1)
116-116: LGTM! Config loader correctly updated.The assignment of
ConfigFileUsednow targets the new location (cfg.Paths.ConfigFileUsed). This is the source where the configuration file path is initially captured from Viper during config loading.internal/service/frontend/templates/base.gohtml (1)
26-37: LGTM! Comprehensive path information exposed to UI.The new
pathsobject in the JavaScript config provides the frontend with access to all system path configurations, including the relocatedconfigFileUsed. This aligns with the PR's goal to add system path information and enables features like the PathsCard component mentioned in the AI summary.internal/test/helper.go (1)
166-166: LGTM! Test helper correctly updated.The test setup now assigns the config file path to the new location (
cfg.Paths.ConfigFileUsed), ensuring test fixtures match the production configuration structure.internal/runtime/subcmd.go (1)
24-30: ConfigFileUsed correctly sourced from PathsConfig in SubCmdBuilderUsing
cfg.Paths.ConfigFileUsedforconfigFilekeeps subcommands aligned with the new config shape while preserving existing behavior whereverb.configFile != ""is checked. Looks good.internal/test/command.go (1)
79-98: withConfigFlag correctly updated to use Paths.ConfigFileUsedThe helper now reads
cfg.Paths.ConfigFileUsed, preserving existing semantics for detecting existing config flags and inserting before--passthrough. This keeps tests in sync with the new config layout.internal/common/config/context_test.go (1)
54-63: TestConfigFileUsed aligned with PathsConfigInitializing
Config.Paths.ConfigFileUsedinTestConfigFileUsednow matches the production accessor, so the test remains meaningful after the field move.ui/index.html (1)
39-50: paths defaults match frontend PathsConfigThe new
pathsobject provides sensible defaults and matches thePathsConfigkeys used in the TS context/components, which should keep the dev index bootstrap consistent.ui/src/contexts/ConfigContext.tsx (1)
3-14: Typed PathsConfig integration into Config looks consistentDefining
PathsConfigand threading it throughConfig.pathscleanly models the new path data exposed from the backend and matches usage in PathsCard and index.html.Also applies to: 16-31
internal/runtime/subcmd_test.go (1)
19-27: Tests correctly updated to use PathsConfig for Executable and ConfigFileUsedThe subcommand tests now initialize
config.ConfigviaPathsConfig.ExecutableandPathsConfig.ConfigFileUsed, and the “without config” cases rely on empty/absent values. This keeps all--configexpectations in sync with the refactored runtime builder.Also applies to: 35-40, 124-129, 140-145, 233-237, 279-283, 332-336, 399-403, 510-514
internal/service/frontend/templates.go (2)
62-62: LGTM! Path configuration field added correctly.The addition of the
Pathsfield tofuncsConfigaligns with the relocation of path-related configuration. The type matchesconfig.PathsConfigand the AI summary confirms proper initialization inserver.go.
107-136: Template functions correctly expose path configuration.All 10 new functions match the fields in
config.PathsConfigand follow a consistent naming pattern. The implementation is straightforward and correct.Security note: These functions expose filesystem paths to templates. Verify that the templates rendering this data are only accessible to authorized administrators, as path disclosure could aid reconnaissance in a security context.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1443 +/- ##
=======================================
Coverage 59.94% 59.94%
=======================================
Files 187 187
Lines 21022 21022
=======================================
Hits 12602 12602
Misses 7106 7106
Partials 1314 1314
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Changes
✏️ Tip: You can customize this high-level summary in your review settings.