chore: add gosec linter#1185
Merged
Merged
Conversation
Contributor
WalkthroughCentralized permission constants were added in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (21)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (13)
🧰 Additional context used🧠 Learnings (14)📓 Common learnings📚 Learning: 2025-06-27T02:36:23.492ZApplied to files:
📚 Learning: 2025-06-24T03:17:11.949ZApplied to files:
📚 Learning: 2025-06-05T12:42:45.139ZApplied to files:
📚 Learning: 2025-08-29T13:56:33.955ZApplied to files:
📚 Learning: 2025-08-02T11:47:07.713ZApplied to files:
📚 Learning: 2025-04-28T03:48:46.327ZApplied to files:
📚 Learning: 2025-08-03T14:49:31.221ZApplied to files:
📚 Learning: 2025-08-24T03:25:10.117ZApplied to files:
📚 Learning: 2025-09-20T01:40:50.076ZApplied to files:
📚 Learning: 2025-09-09T13:29:11.771ZApplied to files:
📚 Learning: 2025-07-27T08:49:09.687ZApplied to files:
📚 Learning: 2025-04-28T04:27:33.074ZApplied to files:
📚 Learning: 2025-08-29T14:11:21.380ZApplied to files:
🧬 Code graph analysis (4)src/internal/model.go (2)
src/internal/common/load_config.go (2)
src/internal/utils/test_utils.go (1)
src/internal/handle_modal.go (1)
⏰ 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)
🔇 Additional comments (6)
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 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/internal/handle_panel_movement.go (1)
69-101: exec.Command hardening looks good; consider clarifying cursor invariantsThe new cursor upper‑bound check and use of a single
filePathvariable (plus the//nolint:gosecjustification) make the open path safer and easier to reason about, and theexec.Commandusage is safe from command injection since the program name is fixed and the path is passed as a single argument, not through a shell.One minor follow‑up: if there is any chance
panel.cursorcan go negative, this function would still panic on the slice index. Either adding a symmetricpanel.cursor < 0guard, or documenting thatpanel.cursoris an invariantly non‑negative value maintained elsewhere, would make this more obviously robust. Given the PR scope, treating this as a low‑priority follow‑up is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
plan.md(1 hunks)src/cmd/help_printer.go(5 hunks)src/cmd/main.go(3 hunks)src/internal/common/load_config.go(2 hunks)src/internal/config_function.go(1 hunks)src/internal/handle_file_operations.go(3 hunks)src/internal/handle_modal.go(2 hunks)src/internal/handle_panel_movement.go(3 hunks)src/internal/model.go(2 hunks)src/internal/ui/metadata/utils.go(2 hunks)src/internal/ui/sidebar/pinned.go(1 hunks)src/internal/utils/bool_file_store.go(1 hunks)src/internal/utils/bool_file_store_test.go(1 hunks)src/internal/utils/file_utils.go(5 hunks)src/internal/utils/test_utils.go(1 hunks)src/pkg/file_preview/image_preview.go(1 hunks)src/pkg/file_preview/kitty.go(2 hunks)src/pkg/file_preview/thumbnail_generator.go(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/ui/sidebar/pinned.gosrc/internal/utils/test_utils.gosrc/internal/handle_file_operations.gosrc/internal/handle_modal.go
📚 Learning: 2025-04-12T14:00:49.244Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:10-13
Timestamp: 2025-04-12T14:00:49.244Z
Learning: In the sidebar package, variables like `pinnedDividerDir` and `diskDividerDir` should remain unexported as they're only used within the package, even though their struct fields are exported with PascalCase (Name, Location) for JSON serialization purposes.
Applied to files:
src/internal/ui/sidebar/pinned.go
📚 Learning: 2025-09-14T05:34:28.184Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 841
File: src/pkg/file_preview/kitty.go:78-79
Timestamp: 2025-09-14T05:34:28.184Z
Learning: lazysegtree correctly identified that `\x1b[0m` is an ANSI reset sequence that resets text formatting to default, not a delay command as incorrectly commented in the Kitty graphics protocol clearing code in src/pkg/file_preview/kitty.go.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR #1021 for yorukot/superfile, the preview panel refactoring created duplicate ImagePreviewer instances (one in defaultModelConfig and another in preview.New()), which caused image preview crashes. The refactoring also renamed SetContextWithRenderText to SetContentWithRenderText, and moved global batCmd to per-instance batCmd initialization, all contributing to crashes during image preview and test failures.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-03-29T10:02:04.260Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 730
File: src/internal/model_render.go:653-654
Timestamp: 2025-03-29T10:02:04.260Z
Learning: A panel validation function should be implemented to ensure panel.cursor doesn't become negative or go out of bounds, but this should be addressed in a separate PR - not as part of the PR fixing crashes when opening files with the editor on an empty panel.
Applied to files:
src/internal/handle_panel_movement.go
📚 Learning: 2025-03-29T13:20:46.467Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 730
File: src/internal/function.go:211-211
Timestamp: 2025-03-29T13:20:46.467Z
Learning: Empty panel.element checks should be added at the beginning of functions that access panel.element to prevent crashes when a panel has no elements, similar to using `if len(panel.element) == 0 { return }`.
Applied to files:
src/internal/handle_panel_movement.go
📚 Learning: 2025-06-05T12:42:45.139Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.139Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Applied to files:
src/pkg/file_preview/thumbnail_generator.gosrc/internal/utils/file_utils.gosrc/internal/model.go
📚 Learning: 2025-06-27T02:36:23.492Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:0-0
Timestamp: 2025-06-27T02:36:23.492Z
Learning: In superfile's LoadTomlFile function refactoring, the preferred approach is to return structured errors (TomlLoadError) rather than using PrintfAndExit for all error conditions. This allows callers to decide appropriate error handling: fatal errors cause immediate exit, while non-fatal errors (like missing fields when not fixing) only print warnings and continue.
Applied to files:
src/internal/utils/file_utils.gosrc/internal/model.go
📚 Learning: 2025-06-24T03:17:11.949Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:104-104
Timestamp: 2025-06-24T03:17:11.949Z
Learning: In superfile's LoadTomlFile function, error handling should follow this pattern: use PrintfAndExit for critical errors that prevent application functionality (config corruption, file system failures during fixing), and use fmt.Println with errorPrefix for non-critical errors that users should see but can continue from (missing fields when not fixing, cleanup warnings).
Applied to files:
src/internal/utils/file_utils.gosrc/internal/model.go
📚 Learning: 2025-09-06T13:42:44.590Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1039
File: src/internal/ui/zoxide/model.go:53-54
Timestamp: 2025-09-06T13:42:44.590Z
Learning: The zoxide modal in src/internal/ui/zoxide/model.go is missing handling for common.Hotkeys.Quit when zClient is nil (lines 52-54), only handling ConfirmTyping and CancelTyping. This creates inconsistency with other modals like sort options menu, help menu, and notify model which all properly handle the Quit hotkey. The prompt modal has the same inconsistency.
Applied to files:
src/internal/model.go
📚 Learning: 2025-09-04T07:24:30.872Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1041
File: src/internal/default_config.go:38-38
Timestamp: 2025-09-04T07:24:30.872Z
Learning: In the superfile codebase, the main model struct has a zClient field for zoxide directory tracking, and the trackDirectoryWithZoxide function in model.go checks if m.zClient is nil before proceeding. When reviewing model initialization functions like defaultModelConfig, ensure all struct fields are properly assigned, especially external service clients like zClient that enable core functionality.
Applied to files:
src/internal/model.go
📚 Learning: 2025-07-27T15:35:25.617Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:35:25.617Z
Learning: In yorukot/superfile, deleteItemWarn() only displays a warning modal via the channelMessage system and does not perform actual file deletion. The actual deletion happens when the user confirms the modal, which triggers getDeleteCmd() that returns a tea.Cmd. The architecture intentionally separates UI modal operations (using channelMessage/goroutines) from file I/O operations (using tea.Cmd pattern), creating clean separation of concerns between UI state management and file operations.
Applied to files:
src/internal/handle_modal.go
🧬 Code graph analysis (5)
src/cmd/main.go (1)
src/config/fixed_variable.go (1)
LastCheckVersion(43-43)
src/internal/config_function.go (1)
src/config/fixed_variable.go (1)
LogFile(51-51)
src/internal/handle_file_operations.go (1)
src/config/fixed_variable.go (1)
ChooserFile(71-71)
src/internal/model.go (1)
src/config/fixed_variable.go (1)
LastDirFile(52-52)
src/internal/common/load_config.go (1)
src/config/fixed_variable.go (3)
ThemeFileVersion(44-44)CurrentVersion(15-15)ThemeFolder(40-40)
🪛 GitHub Actions: Go CI
src/internal/handle_file_operations.go
[error] 374-374: golangci-lint: File is not properly formatted (golines).
🪛 GitHub Check: Build and Test (ubuntu-latest)
src/pkg/file_preview/kitty.go
[failure] 92-92:
File is not properly formatted (golines)
src/internal/handle_file_operations.go
[failure] 374-374:
File is not properly formatted (golines)
src/pkg/file_preview/image_preview.go
[failure] 337-337:
File is not properly formatted (golines)
🪛 LanguageTool
plan.md
[style] ~268-~268: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...with PNG, JPG, GIF files - Test with very large images (>4K resolution) - Test with ...
(EN_WEAK_ADJECTIVE)
[style] ~272-~272: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...e edge cases**: - Resize terminal to very small (10x10) - Resize to very large - ...
(EN_WEAK_ADJECTIVE)
[style] ~273-~273: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...al to very small (10x10) - Resize to very large - Image preview should adapt without...
(EN_WEAK_ADJECTIVE)
⏰ 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: Analyze (go)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (18)
src/pkg/file_preview/kitty.go (1)
128-129: LGTM! Appropriate use of nolint suppressions.The gosec suppressions are justified here. The terminal dimensions are bounded by maxWidth and maxHeight parameters, which are validated at line 100-102, ensuring the int-to-uint32 conversions are safe.
Also applies to: 133-134
src/pkg/file_preview/thumbnail_generator.go (1)
77-77: LGTM! Explicit error handling improvement.The explicit error discard is appropriate here. The file is created solely to obtain a unique temporary path, then immediately closed before ffmpeg writes to it. Any Close() error at this point is non-critical and would be superseded by ffmpeg's subsequent operations.
src/internal/config_function.go (1)
31-31: LGTM! Security improvement for log file permissions.Tightening the log file permissions from 0666 to 0600 is a good security practice, restricting access to the owner only. This prevents other users from reading potentially sensitive debug information.
src/internal/ui/sidebar/pinned.go (1)
50-50: LGTM! Security improvement for pinned directories file.Reducing permissions from 0644 to 0600 appropriately restricts access to the user's pinned directories configuration to the owner only.
src/internal/handle_modal.go (1)
37-37: LGTM! Justified nolint suppressions for user-created directories.The 0755 permissions and nolint:gosec suppressions are appropriate here. User-created directories typically need standard permissions (owner rwx, group rx, other rx) to allow proper filesystem traversal, unlike application config files which should be restricted to 0600.
Also applies to: 48-48
src/internal/utils/test_utils.go (1)
42-42: LGTM! Appropriate nolint suppressions for test utilities.The gosec suppressions are justified for test setup functions. Test directories and files need standard readable permissions (0755/0644) to function properly in the test environment.
Also applies to: 50-50
src/internal/utils/bool_file_store.go (1)
34-34: LGTM! Security improvement for boolean state files.Tightening permissions to 0600 appropriately restricts access to user toggle state files to the owner only.
src/internal/utils/bool_file_store_test.go (1)
142-142: LGTM! Test expectation correctly updated.The test expectation has been properly updated to match the implementation change in
bool_file_store.go, ensuring the test validates the new 0600 permission requirement.src/internal/model.go (2)
780-780: LGTM! Explicit error discard improves code clarity.The explicit
_ = et.Close()makes it clear that the error is intentionally ignored during cleanup, which is appropriate in the quit sequence.
791-791: Permission change to 0o600 is correct and improves security.The
LastDirFileis sourced by the shell wrapper in the same user's shell context (via. "$SPF_LAST_DIR"in cd_on_quit.sh). Since both the superfile process (writing) and shell wrapper (sourcing) execute as the same user, the 0o600 permission is fully compatible and actually provides a security benefit by preventing other users from accessing directory history.src/internal/handle_file_operations.go (2)
291-293: LGTM! Improved error visibility for cut operation cleanup.Logging the error when removing the source after a cut operation is a good improvement. This helps diagnose issues without halting the operation, which is appropriate since the paste has already succeeded.
425-425: LGTM! Appropriate permission tightening for chooser file.Restricting the chooser file to 0o600 (owner read/write only) is appropriate since it contains the selected file path and only needs to be accessed by the invoking process.
src/internal/ui/metadata/utils.go (1)
4-4: LGTM! Well-justified nolint pragmas for MD5 usage.The
nolint:gosecdirectives are appropriately applied with clear justification comments. MD5 is being used for file integrity display purposes (showing checksums to users), not for cryptographic security, which is a valid use case.Also applies to: 63-63
src/internal/common/load_config.go (1)
202-202: LGTM! Appropriate permission tightening for theme files.The permission changes to 0o700 for theme directories and 0o600 for theme version files are appropriate. Theme files are user-specific configuration data that don't need to be accessible by other users.
Also applies to: 207-207, 238-238
src/internal/utils/file_utils.go (2)
30-30: LGTM! Consistent permission tightening across utility functions.The permission changes to 0o600 for files and 0o700 for directories are consistently applied across all utility functions. This is appropriate for configuration, TOML data, and backup files which are user-specific and don't need broader access.
Also applies to: 153-153, 235-235, 246-246
138-142: LGTM! Improved error handling for backup file cleanup.The enhanced error handling for backup file close operations improves robustness. Capturing and reporting close errors during the deferred cleanup ensures that file descriptor issues don't go unnoticed.
src/cmd/help_printer.go (1)
34-34: LGTM! Explicit error handling improves code clarity.Explicitly discarding the return values from
Fprintfcalls makes it clear that errors are intentionally ignored. This is appropriate for UI rendering where errors are unlikely and non-critical.Also applies to: 39-39, 49-51, 72-72, 80-80, 90-90, 134-134
src/cmd/main.go (1)
206-206: LGTM! Appropriate permission tightening for user state files.The permission changes to 0o600 are consistently applied across all user-specific state and configuration files (first use check, config files, JSON initialization, and version check files). This is appropriate as these files contain user-specific data that doesn't need to be accessible by other users.
Also applies to: 216-216, 225-225, 233-233
Deploying superfile with
|
| Latest commit: |
4bc2c70
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c6ef66c3.superfile.pages.dev |
| Branch Preview URL: | https://add-gosec.superfile.pages.dev |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lazysegtree
commented
Dec 12, 2025
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/internal/permissions/permissions.go (1)
1-50: Preferfs.FileMode-typed constants (safer than untyped ints).Right now these are untyped numeric consts; consider typing them to
fs.FileModeto avoid accidental arithmetic/misuse and to align withos.WriteFile/os.MkdirAll/os.OpenFileexpectations.package permissions +import "io/fs" + // File permissions const ( @@ - ConfigFilePerm = 0600 + ConfigFilePerm fs.FileMode = 0o600 @@ - UserFilePerm = 0644 + UserFilePerm fs.FileMode = 0o644 ) @@ - ConfigDirPerm = 0700 + ConfigDirPerm fs.FileMode = 0o700 @@ - UserDirPerm = 0755 + UserDirPerm fs.FileMode = 0o755 @@ - LogFilePerm = 0600 + LogFilePerm fs.FileMode = 0o600 @@ - ExtractedFilePerm = 0644 + ExtractedFilePerm fs.FileMode = 0o644 @@ - ExtractedDirPerm = 0755 + ExtractedDirPerm fs.FileMode = 0o755 )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
report.md(1 hunks)src/cmd/main.go(4 hunks)src/internal/common/ui_consts.go(1 hunks)src/internal/config_function.go(2 hunks)src/internal/handle_modal.go(3 hunks)src/internal/permissions/permissions.go(1 hunks)src/internal/ui/sidebar/pinned.go(2 hunks)src/internal/utils/bool_file_store.go(2 hunks)src/internal/utils/test_utils.go(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/internal/common/ui_consts.go
🚧 Files skipped from review as they are similar to previous changes (2)
- src/internal/utils/test_utils.go
- src/internal/ui/sidebar/pinned.go
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-08-24T03:24:50.857Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:18-21
Timestamp: 2025-08-24T03:24:50.857Z
Learning: The superfile project uses github.com/charmbracelet/x/ansi as the standardized ANSI package and deprecates github.com/charmbracelet/x/exp/term/ansi to avoid binary bloat and potential incompatibilities.
Applied to files:
src/internal/config_function.gosrc/cmd/main.go
📚 Learning: 2025-08-24T03:25:10.117Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-08-06T10:54:31.444Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When lazysegtree says a review is "inaccurate and pre-existing issues. Not much useful," it means I should focus specifically on bugs or problems introduced by the current PR changes, not architectural concerns or code quality issues that were already present. The refactoring work in superfile PRs is generally well-implemented and should be evaluated on whether the specific changes work correctly, not on broader codebase architecture.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-04-28T03:48:46.327Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-08-02T11:47:07.713Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-08-06T10:27:20.420Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-04-28T04:27:33.074Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-04-12T12:00:32.688Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-07-24T03:46:29.516Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Applied to files:
src/cmd/main.go
📚 Learning: 2025-08-29T13:56:33.955Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1011
File: src/internal/handle_modal.go:145-203
Timestamp: 2025-08-29T13:56:33.955Z
Learning: lazysegtree identified that help menu navigation functions (helpMenuListUp and helpMenuListDown) in src/internal/handle_modal.go are too complicated, can be simplified, are similar to sidebar navigation but inconsistent, and lack unit tests. He prefers to track such technical debt in separate GitHub issues rather than addressing it in the current PR scope.
Applied to files:
src/cmd/main.go
🧬 Code graph analysis (4)
src/internal/config_function.go (2)
src/config/fixed_variable.go (1)
LogFile(51-51)src/internal/permissions/permissions.go (1)
LogFilePerm(32-32)
src/internal/utils/bool_file_store.go (1)
src/internal/permissions/permissions.go (1)
ConfigFilePerm(10-10)
src/cmd/main.go (2)
src/internal/permissions/permissions.go (1)
ConfigFilePerm(10-10)src/config/fixed_variable.go (1)
LastCheckVersion(43-43)
src/internal/handle_modal.go (1)
src/internal/permissions/permissions.go (1)
UserDirPerm(25-25)
🪛 LanguageTool
report.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...fication Report Date: December 12, 2024 Branch: add-gosec **Purpose:*...
(MISSING_COMMA_AFTER_YEAR)
[style] ~139-~139: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...add-gosec - Test Date: December 12, 2024 - Tester: Automated verification su...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.18.1)
report.md
124-124: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (3)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
src/internal/permissions/permissions.go (1)
46-50: Backward-compat aliases look OK.src/cmd/main.go (2)
14-15: Import of centralized permissions: LGTM.
203-213: 0600 for config/state marker files is appropriate; behavior unchanged.Minor gotcha to be aware of:
os.WriteFile(..., mode)won’t tighten permissions for pre-existing files (it only applies on create). If the intent is to enforce stricter perms on upgrade, it would need an explicitos.Chmodpath (but that’s likely out-of-scope for this PR).Also applies to: 215-232, 234-239
src/internal/config_function.go (1)
10-11: Log file mode tightened to 0600: good for gosec/least-privilege.Also applies to: 33-34
src/internal/handle_modal.go (1)
9-10: Usingpermissions.UserDirPerm+ justified//nolint:gosecis consistent with “user-created dirs” exception.Please just confirm gosec is satisfied by these annotations in CI (rule IDs can vary by gosec version/config).
Also applies to: 36-55
src/internal/utils/bool_file_store.go (1)
7-9: WriteBoolFile now creates state files as 0600: good.Also applies to: 35-37
0f1e18e to
f320f52
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/pkg/file_preview/kitty.go (1)
92-95: This may still violategolines; split the trailing comment/expr.
The long inline//nolint:gosec // ...on the same line as+ kittyNonZeroOffsetis likely to re-trigger the formatter complaint. (This is the same issue previously raised.)func generatePlacementID(path string) uint32 { @@ - return uint32( - hash&kittyMaxID, - ) + kittyNonZeroOffset //nolint:gosec // Hash is bounded by kittyMaxID mask before conversion + // Hash is bounded by kittyMaxID mask before conversion. + masked := hash & kittyMaxID + return uint32(masked) + kittyNonZeroOffset //nolint:gosec }plan.md (1)
15-23: Fix internal plan count/status inconsistencies (trustworthiness).
This matches the earlier review feedback: the G104 count math, thesrc/cmd/main.gooccurrence count vs listed lines, and Phase 2 completion vs “All phases completed / 0 remaining” should align. The “very large/very small” wording is optional style polish.Also applies to: 57-60, 158-162, 266-274
report.md (2)
3-5: Fix report dates to match this PR’s Dec 2025 timeline (currently says 2024).Also applies to: 139-140
124-126: Use a Markdown heading for “Verdict” (markdownlint MD036).
🧹 Nitpick comments (5)
src/internal/utils/permissions.go (1)
1-19: Nice centralization of permission constants.
Optional: consider typing these asos.FileModeto prevent accidental non-mode usage.-package utils +package utils + +import "os" @@ - ConfigFilePerm = 0600 // configuration files (owner read/write only) + ConfigFilePerm os.FileMode = 0o600 // configuration files (owner read/write only)src/internal/handle_file_operations.go (1)
374-378: Prefer a centralized extracted-dir mode constant instead of0o755literal (if available).
Keeps permissions uniform and reduces future gosec churn.- err = os.MkdirAll( - outputDir, - 0o755, - ) //nolint:gosec // extracted directories need standard permissions for user access + err = os.MkdirAll(outputDir, utils.ExtractedDirMode) //nolint:gosec // extracted directories need standard permissions for user accesssrc/internal/utils/file_utils.go (3)
24-35: UseConfigFilePermconstant inWriteTomlDatainstead of0o600literal.
Keeps the permission policy in one place (and matches other PR edits like sidebar pinned).- err = os.WriteFile(filePath, tomlData, 0o600) + err = os.WriteFile(filePath, tomlData, ConfigFilePerm)
151-159:os.OpenFile(..., os.O_RDWR, 0o600)perm is ignored withoutos.O_CREATE.
This doesn’t hurt, but it’s misleading (and doesn’t actually “tighten” anything). Consider passing0or switching toos.OpenFile(filePath, os.O_RDWR, 0)to reflect reality.
230-240: Consider replacing remaining permission literals with centralized constants (ConfigDirPerm/ConfigFilePerm).
This improves consistency with the rest of the PR and reduces future drift.- if err := os.MkdirAll(dir, 0o700); err != nil { + if err := os.MkdirAll(dir, ConfigDirPerm); err != nil { return fmt.Errorf("failed to create directory %s: %w", dir, err) } @@ - if err = os.WriteFile(file, nil, 0o600); err != nil { + if err = os.WriteFile(file, nil, ConfigFilePerm); err != nil { return fmt.Errorf("failed to create file %s: %w", file, err) }Also applies to: 242-252
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
plan.md(1 hunks)report.md(1 hunks)src/cmd/help_printer.go(5 hunks)src/cmd/main.go(3 hunks)src/internal/common/load_config.go(2 hunks)src/internal/common/ui_consts.go(1 hunks)src/internal/config_function.go(1 hunks)src/internal/file_operations_extract.go(2 hunks)src/internal/handle_file_operations.go(3 hunks)src/internal/handle_modal.go(2 hunks)src/internal/handle_panel_movement.go(3 hunks)src/internal/model.go(2 hunks)src/internal/ui/metadata/utils.go(2 hunks)src/internal/ui/sidebar/pinned.go(2 hunks)src/internal/utils/bool_file_store.go(1 hunks)src/internal/utils/bool_file_store_test.go(1 hunks)src/internal/utils/file_utils.go(5 hunks)src/internal/utils/permissions.go(1 hunks)src/internal/utils/test_utils.go(1 hunks)src/pkg/file_preview/image_preview.go(1 hunks)src/pkg/file_preview/kitty.go(2 hunks)src/pkg/file_preview/thumbnail_generator.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/cmd/main.go
- src/internal/handle_panel_movement.go
- src/pkg/file_preview/image_preview.go
- src/internal/common/load_config.go
- src/internal/utils/bool_file_store.go
- src/internal/common/ui_consts.go
- src/internal/utils/bool_file_store_test.go
- src/pkg/file_preview/thumbnail_generator.go
- src/internal/ui/metadata/utils.go
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/handle_file_operations.gosrc/internal/handle_modal.gosrc/internal/ui/sidebar/pinned.gosrc/internal/utils/file_utils.go
📚 Learning: 2025-04-12T14:00:49.244Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 770
File: src/internal/ui/sidebar/consts.go:10-13
Timestamp: 2025-04-12T14:00:49.244Z
Learning: In the sidebar package, variables like `pinnedDividerDir` and `diskDividerDir` should remain unexported as they're only used within the package, even though their struct fields are exported with PascalCase (Name, Location) for JSON serialization purposes.
Applied to files:
src/internal/ui/sidebar/pinned.go
📚 Learning: 2025-09-14T05:34:28.184Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 841
File: src/pkg/file_preview/kitty.go:78-79
Timestamp: 2025-09-14T05:34:28.184Z
Learning: lazysegtree correctly identified that `\x1b[0m` is an ANSI reset sequence that resets text formatting to default, not a delay command as incorrectly commented in the Kitty graphics protocol clearing code in src/pkg/file_preview/kitty.go.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-08-27T11:32:17.326Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1021
File: .gitignore:37-37
Timestamp: 2025-08-27T11:32:17.326Z
Learning: In PR #1021 for yorukot/superfile, the preview panel refactoring created duplicate ImagePreviewer instances (one in defaultModelConfig and another in preview.New()), which caused image preview crashes. The refactoring also renamed SetContextWithRenderText to SetContentWithRenderText, and moved global batCmd to per-instance batCmd initialization, all contributing to crashes during image preview and test failures.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-06-27T02:36:23.492Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:0-0
Timestamp: 2025-06-27T02:36:23.492Z
Learning: In superfile's LoadTomlFile function refactoring, the preferred approach is to return structured errors (TomlLoadError) rather than using PrintfAndExit for all error conditions. This allows callers to decide appropriate error handling: fatal errors cause immediate exit, while non-fatal errors (like missing fields when not fixing) only print warnings and continue.
Applied to files:
src/internal/model.gosrc/internal/utils/file_utils.go
📚 Learning: 2025-06-05T12:42:45.139Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 821
File: src/internal/file_operations_compress.go:0-0
Timestamp: 2025-06-05T12:42:45.139Z
Learning: Pattern to watch for: defer file.Close() inside loops (for, range, filepath.Walk callbacks) causes resource leaks by accumulating file descriptors until function exits. Should be replaced with explicit Close() calls immediately after file operations with proper error handling.
Applied to files:
src/internal/model.gosrc/internal/utils/file_utils.go
📚 Learning: 2025-06-24T03:17:11.949Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 881
File: src/internal/utils/file_utils.go:104-104
Timestamp: 2025-06-24T03:17:11.949Z
Learning: In superfile's LoadTomlFile function, error handling should follow this pattern: use PrintfAndExit for critical errors that prevent application functionality (config corruption, file system failures during fixing), and use fmt.Println with errorPrefix for non-critical errors that users should see but can continue from (missing fields when not fixing, cleanup warnings).
Applied to files:
src/internal/model.gosrc/internal/utils/file_utils.go
📚 Learning: 2025-08-29T13:56:33.955Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1011
File: src/internal/handle_modal.go:145-203
Timestamp: 2025-08-29T13:56:33.955Z
Learning: lazysegtree identified that help menu navigation functions (helpMenuListUp and helpMenuListDown) in src/internal/handle_modal.go are too complicated, can be simplified, are similar to sidebar navigation but inconsistent, and lack unit tests. He prefers to track such technical debt in separate GitHub issues rather than addressing it in the current PR scope.
Applied to files:
src/internal/utils/file_utils.goplan.md
📚 Learning: 2025-07-27T15:32:06.922Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:32:06.922Z
Learning: When reviewing large refactoring PRs that change async patterns (like moving from goroutines to tea.Cmd), always check for incomplete refactoring where some call sites still use the old pattern while others use the new pattern, as this often leads to compilation errors and architectural inconsistencies.
Applied to files:
src/internal/utils/file_utils.goplan.md
📚 Learning: 2025-07-27T07:54:51.679Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-07-27T07:54:51.679Z
Learning: When reviewing PRs, be precise about distinguishing between newly introduced issues versus pre-existing issues. Use language like "pre-existing issues remain" or "architectural issues are present" rather than "issues were introduced" when the problems existed before the current PR changes.
Applied to files:
plan.md
🧬 Code graph analysis (7)
src/internal/config_function.go (2)
src/config/fixed_variable.go (1)
LogFile(51-51)src/internal/utils/permissions.go (1)
LogFilePerm(10-10)
src/internal/handle_file_operations.go (1)
src/config/fixed_variable.go (1)
ChooserFile(71-71)
src/internal/handle_modal.go (1)
src/internal/utils/permissions.go (1)
UserDirPerm(14-14)
src/internal/ui/sidebar/pinned.go (1)
src/internal/utils/permissions.go (1)
ConfigFilePerm(8-8)
src/internal/file_operations_extract.go (1)
src/internal/utils/permissions.go (2)
ExtractedFileMode(17-17)ExtractedDirMode(18-18)
src/internal/utils/test_utils.go (1)
src/internal/utils/permissions.go (2)
UserDirPerm(14-14)UserFilePerm(9-9)
src/internal/model.go (1)
src/config/fixed_variable.go (1)
LastDirFile(52-52)
🪛 LanguageTool
report.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...fication Report Date: December 12, 2024 Branch: add-gosec **Purpose:*...
(MISSING_COMMA_AFTER_YEAR)
[style] ~139-~139: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...add-gosec - Test Date: December 12, 2024 - Tester: Automated verification su...
(MISSING_COMMA_AFTER_YEAR)
plan.md
[style] ~268-~268: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...with PNG, JPG, GIF files - Test with very large images (>4K resolution) - Test with ...
(EN_WEAK_ADJECTIVE)
[style] ~272-~272: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...e edge cases**: - Resize terminal to very small (10x10) - Resize to very large - ...
(EN_WEAK_ADJECTIVE)
[style] ~273-~273: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...al to very small (10x10) - Resize to very large - Image preview should adapt without...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
report.md
124-124: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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 and Test (windows-latest)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
src/internal/model.go (1)
788-795: Tightening lastdir permissions to0600is the right call.
This file effectively stores shell commands / user navigation history; keeping it owner-only makes sense.src/pkg/file_preview/kitty.go (1)
127-137: G115 suppressions look fine here; just ensuredstRows/dstColscan’t go negative.
Given the arithmetic, they should be ≥0 for positive inputs, but it’s worth sanity-checking the computedint(...)results for tiny terminal sizes.src/internal/config_function.go (1)
30-38: Switching log file creation mode toutils.LogFilePermis correct.
This should address the “open perms too permissive” class of findings for new installs.src/internal/file_operations_extract.go (1)
10-26: Good: extraction file/dir modes now come from centralized utils constants.
Keeps security posture consistent across modules.src/internal/handle_modal.go (1)
34-40: LGTM: use ofutils.UserDirPerm+ explicit gosec rationale is consistent.
Just verify CI’s gosec integration honors//nolint:gosecfor theseMkdirAllcalls.Also applies to: 48-52
src/internal/ui/sidebar/pinned.go (2)
9-11: Check for accidental import cycle after addinginternal/utilsimport.
This should be fine, but please ensuregithub.com/yorukot/superfile/src/internal/utilsdoesn’t (directly or indirectly) importsrc/internal/ui/sidebar.
52-54: Permission hardening viautils.ConfigFilePermlooks correct.
Good alignment with centralizing config permissions (0600).src/internal/utils/test_utils.go (2)
42-44:UserDirPerm+ gosec suppression is reasonable for test fixtures.
50-52:UserFilePerm+ gosec suppression is reasonable for test fixtures.src/internal/handle_file_operations.go (1)
428-434: Chooser file permission tightened to0o600is sensible.src/internal/utils/file_utils.go (1)
137-150: Defer-close error handling improvement is good.
Only overridingresultErrwhen it’s otherwise empty avoids hiding the primary failure.report.md (1)
136-138: Verify the reported Go version matches the actual test environment.
If this was generated fromgo version, consider pasting the exact output or aligning the version string accordingly.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bfe0172 to
05603b8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
05603b8 to
6fbfff3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lazysegtree
commented
Dec 12, 2025
| errMessage = "paste item error" | ||
| } else if cut { | ||
| // TODO: Fix unhandled error | ||
| os.RemoveAll(filePath) |
Collaborator
Author
There was a problem hiding this comment.
This was redundant. Above pasteDir already does it.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/file_preview/kitty.go (1)
114-136: Add a guard forPixelsPerColumn/Row == 0to avoid divide-by-zero producing bad sizes.
Today this relies onp.terminalCap.GetTerminalCellSize()always returning non-zero pixel dimensions.
♻️ Duplicate comments (1)
src/pkg/file_preview/kitty.go (1)
82-94: golines may still fail on thegeneratePlacementIDreturn line; split into a temp var.
The current inline//nolint:gosecline was previously reported as too long; this still looks close to/over the 120 limit.func generatePlacementID(path string) uint32 { @@ hash := 0 for _, c := range path { hash = hash*kittyHashPrime + int(c) } - return uint32(hash&kittyMaxID) + //nolint:gosec // Hash is bounded by kittyMaxID mask before conversion - kittyNonZeroOffset + masked := hash & kittyMaxID + //nolint:gosec // Hash is bounded by kittyMaxID mask before conversion + return uint32(masked) + kittyNonZeroOffset }
🧹 Nitpick comments (3)
src/internal/handle_file_operations.go (2)
362-385: Preferutils.ExtractedDirModeinstead of hard-coded0o755(keep the nolint).
This keeps “extracted dir perms” consistent everywhere if it ever changes.- err = os.MkdirAll( //nolint:gosec // extracted directories need standard permissions for user access + err = os.MkdirAll( //nolint:gosec // extracted directories need standard permissions for user access outputDir, - 0o755, + utils.ExtractedDirMode, )
422-430: Use the newutils.ConfigFilePermconstant for chooser file writes (and keep 0600).- err := os.WriteFile(variable.ChooserFile, []byte(path), 0o600) + err := os.WriteFile(variable.ChooserFile, []byte(path), utils.ConfigFilePerm)src/internal/utils/consts.go (1)
3-25: Consider typing these permission constants asfs.FileMode(stronger intent/type-safety).package utils + +import "io/fs" const ( @@ // File permissions - ConfigFilePerm = 0600 // configuration files (owner read/write only) - UserFilePerm = 0644 // user-created files (owner rw, others r) - LogFilePerm = 0600 // log files (owner read/write only) + ConfigFilePerm fs.FileMode = 0o600 // configuration files (owner read/write only) + UserFilePerm fs.FileMode = 0o644 // user-created files (owner rw, others r) + LogFilePerm fs.FileMode = 0o600 // log files (owner read/write only) @@ // Directory permissions - ConfigDirPerm = 0700 // configuration directories (owner only) - UserDirPerm = 0755 // user-created directories (owner rwx, others rx) + ConfigDirPerm fs.FileMode = 0o700 // configuration directories (owner only) + UserDirPerm fs.FileMode = 0o755 // user-created directories (owner rwx, others rx) @@ // Extracted file permissions (from archives) - ExtractedFileMode = 0644 // extracted files - ExtractedDirMode = 0755 // extracted directories + ExtractedFileMode fs.FileMode = 0o644 // extracted files + ExtractedDirMode fs.FileMode = 0o755 // extracted directories )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.golangci.yaml(2 hunks)src/internal/common/ui_consts.go(0 hunks)src/internal/handle_file_operations.go(2 hunks)src/internal/handle_modal.go(2 hunks)src/internal/handle_panel_movement.go(3 hunks)src/internal/utils/consts.go(1 hunks)src/internal/utils/test_utils.go(1 hunks)src/pkg/file_preview/image_preview.go(1 hunks)src/pkg/file_preview/kitty.go(2 hunks)
💤 Files with no reviewable changes (1)
- src/internal/common/ui_consts.go
🚧 Files skipped from review as they are similar to previous changes (3)
- src/internal/handle_modal.go
- src/pkg/file_preview/image_preview.go
- src/internal/handle_panel_movement.go
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When lazysegtree says a review is "inaccurate and pre-existing issues. Not much useful," it means I should focus specifically on bugs or problems introduced by the current PR changes, not architectural concerns or code quality issues that were already present. The refactoring work in superfile PRs is generally well-implemented and should be evaluated on whether the specific changes work correctly, not on broader codebase architecture.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:27:20.420Z
Learning: lazysegtree specifically asked for a "strict review to find bad stuff" in PR reviews, indicating he wants comprehensive analysis that identifies all potential issues regardless of scope, not just issues within the current PR's scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-09-14T05:34:28.184Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 841
File: src/pkg/file_preview/kitty.go:78-79
Timestamp: 2025-09-14T05:34:28.184Z
Learning: lazysegtree correctly identified that `\x1b[0m` is an ANSI reset sequence that resets text formatting to default, not a delay command as incorrectly commented in the Kitty graphics protocol clearing code in src/pkg/file_preview/kitty.go.
Applied to files:
src/pkg/file_preview/kitty.go
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-06-21T15:17:49.464Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 887
File: src/internal/handle_file_operations.go:53-53
Timestamp: 2025-06-21T15:17:49.464Z
Learning: In Go, filepath.Abs() internally calls filepath.Clean() on the result, so calling filepath.Clean() after filepath.Abs() is redundant and should be avoided.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-08-24T03:25:10.117Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-04-12T12:00:32.688Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-08-06T10:54:31.444Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 979
File: src/internal/common/predefined_variable.go:47-47
Timestamp: 2025-08-06T10:54:31.444Z
Learning: When lazysegtree says a review is "inaccurate and pre-existing issues. Not much useful," it means I should focus specifically on bugs or problems introduced by the current PR changes, not architectural concerns or code quality issues that were already present. The refactoring work in superfile PRs is generally well-implemented and should be evaluated on whether the specific changes work correctly, not on broader codebase architecture.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-04-28T03:48:46.327Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/ui/sidebar/render.go:46-48
Timestamp: 2025-04-28T03:48:46.327Z
Learning: The user (lazysegtree) prefers to keep PRs focused and manageable in size, sometimes intentionally leaving TODO comments to track minor issues for later PRs rather than addressing everything at once.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-07-27T15:32:06.922Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T15:32:06.922Z
Learning: When reviewing large refactoring PRs that change async patterns (like moving from goroutines to tea.Cmd), always check for incomplete refactoring where some call sites still use the old pattern while others use the new pattern, as this often leads to compilation errors and architectural inconsistencies.
Applied to files:
src/internal/handle_file_operations.go
📚 Learning: 2025-04-28T04:27:33.074Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 781
File: src/internal/model_render.go:254-256
Timestamp: 2025-04-28T04:27:33.074Z
Learning: When race conditions or other issues that are unrelated to the current PR's focus are identified during review, lazysegtree prefers creating separate GitHub issues to track them rather than addressing them in the current PR. This helps maintain PR focus and scope.
Applied to files:
src/internal/handle_file_operations.go
🧬 Code graph analysis (2)
src/internal/handle_file_operations.go (1)
src/config/fixed_variable.go (1)
ChooserFile(71-71)
src/internal/utils/test_utils.go (1)
src/internal/utils/consts.go (2)
UserDirPerm(20-20)UserFilePerm(15-15)
⏰ 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). (3)
- GitHub Check: Build and Test (windows-latest)
- GitHub Check: Analyze (go)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
src/internal/utils/test_utils.go (1)
36-53: Nice cleanup: usingUserDirPerm/UserFilePermimproves consistency without changing behavior..golangci.yaml (1)
56-93: The commit history shows "fix: resolve gosec linter issues after enabling," indicating the 45 gosec findings are being addressed in this PR. Enabling gosec alongside test-file exclusions (standard linting practice) is not merge-blocking when the underlying issues are fixed.Likely an incorrect or invalid review comment.
92a606b to
39f635b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/internal/handle_modal.go (1)
34-47: HandleClose()error (likely gosec G104) while touching this path
defer f.Close()drops the error; if the PR is aiming to reduce gosec noise, this is a low-effort fix close to the touched lines.@@ f, err := os.Create(path) if err != nil { slog.Error("Error while createItem during file creation", "error", err) return } - defer f.Close() + defer func() { + if cerr := f.Close(); cerr != nil { + slog.Error("Error while createItem during file close", "error", cerr) + } + }() } else {
🧹 Nitpick comments (1)
src/internal/handle_modal.go (1)
37-40: Sanity-checkutils.UserDirPerm(=0755) is intentional for created dirs
Centralizing the mode is good; just confirm0755is appropriate here (user content dirs) vs0700for any app-owned/sensitive directories.Also applies to: 48-52
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/internal/handle_file_operations.go(2 hunks)src/internal/handle_modal.go(2 hunks)src/internal/handle_panel_movement.go(3 hunks)src/internal/utils/test_utils.go(1 hunks)src/pkg/file_preview/image_preview.go(1 hunks)src/pkg/file_preview/kitty.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/internal/handle_file_operations.go
- src/internal/handle_panel_movement.go
- src/pkg/file_preview/kitty.go
- src/pkg/file_preview/image_preview.go
- src/internal/utils/test_utils.go
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/utils/file_utils.go:252-275
Timestamp: 2025-08-24T03:25:10.117Z
Learning: In PR #1013 for yorukot/superfile, when reviewing the ReadFileContent utility function, lazysegtree chose to implement only the parameter renaming fix (filepath → filePath) to avoid shadowing and declined buffer size increases and optional truncation enhancements, preferring to keep the utility function scope focused and avoid over-engineering when the additional features aren't immediately needed.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/default_config.go:16-16
Timestamp: 2025-07-27T07:40:51.938Z
Learning: lazysegtree prefers simpler implementation approaches when the alternatives are significantly more complex, even if the alternatives might be architecturally cleaner, prioritizing maintainability and avoiding over-engineering.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 969
File: src/internal/key_function.go:40-40
Timestamp: 2025-08-03T09:34:55.721Z
Learning: lazysegtree emphasizes proper dependency direction in software architecture, preferring that low-level components (like modal handlers) should not depend on high-level components (like the main model object). He also prioritizes performance considerations, noting that creating objects on every keypress in hot code paths like key handling is inefficient and should be avoided.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1013
File: src/internal/model_render.go:329-341
Timestamp: 2025-08-29T14:11:21.380Z
Learning: lazysegtree prefers to defer help menu rendering optimizations and other technical debt improvements to separate GitHub issues when the current PR scope has grown too large, maintaining focus on the primary refactoring objectives while tracking performance improvements for future work.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 985
File: src/internal/model.go:0-0
Timestamp: 2025-08-11T01:49:30.040Z
Learning: lazysegtree prefers maintaining code correctness through proper design and invariants rather than adding defensive bounds checks at every slice access point, viewing such defensive programming as "duct taping" that can mask actual bugs instead of fixing them at their source.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 973
File: src/internal/ui/processbar/model_update.go:7-27
Timestamp: 2025-08-03T14:49:31.221Z
Learning: lazysegtree prefers to keep test-only code simple without adding production-level concerns like goroutine synchronization, cancellation contexts, or complex lifecycle management, even when such patterns might prevent potential issues, since the complexity isn't justified for test utilities.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1044
File: src/internal/utils/file_utils.go:149-149
Timestamp: 2025-09-09T13:29:11.771Z
Learning: lazysegtree prefers to keep PR scope focused on the primary objectives and considers pre-existing technical debt issues as out of scope for migration/refactoring PRs, preferring to defer such issues to separate GitHub issues rather than expanding the current PR scope.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 950
File: src/internal/ui/metadata/README.md:7-7
Timestamp: 2025-07-24T03:46:29.516Z
Learning: lazysegtree prefers to defer comprehensive unit testing to separate PRs when the current PR has grown too large, maintaining focus on the primary refactoring objectives while tracking testing requirements in dedicated GitHub issues.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 967
File: src/internal/key_function.go:45-47
Timestamp: 2025-08-02T11:47:07.713Z
Learning: lazysegtree prefers to track technical debt and architectural improvements in dedicated GitHub issues when they are identified during PR reviews but are beyond the scope of the current PR, particularly for complex refactoring needs like input handling architecture that would require significant changes.
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 963
File: src/internal/handle_file_operations.go:567-570
Timestamp: 2025-07-27T08:49:09.687Z
Learning: lazysegtree prefers to defer technical debt issues like model mutation concerns to later PRs when the current PR has already grown too large, maintaining focus on the primary objectives while acknowledging the need to track such issues for future work.
📚 Learning: 2025-09-20T01:40:50.076Z
Learnt from: lazysegtree
Repo: yorukot/superfile PR: 1081
File: src/internal/ui/sidebar/directory_utils.go:103-112
Timestamp: 2025-09-20T01:40:50.076Z
Learning: lazysegtree identified code duplication between removeNotExistingDirectories and TogglePinnedDirectory functions in src/internal/ui/sidebar/directory_utils.go, specifically 6 lines of JSON marshaling and file writing logic. He prefers to track such duplication fixes in separate GitHub issues and suggests either extracting common util functions or creating a PinnedDir manager for centralized Read/Write operations to PinnedFile.
Applied to files:
src/internal/handle_modal.go
🧬 Code graph analysis (1)
src/internal/handle_modal.go (1)
src/internal/utils/consts.go (1)
UserDirPerm(20-20)
⏰ 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 and Test (windows-latest)
- GitHub Check: Cloudflare Pages
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
220b671 to
4bc2c70
Compare
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| file_utils.go | 1 critical rule | 8.48 → 8.30 | Suppress |
Quality Gate Profile: The Bare Minimum
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
Collaborator
Author
|
codescene failure is too strict. |
yorukot
approved these changes
Dec 13, 2025
Collaborator
Author
|
Some more checks to ensure it works |
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jan 17, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | minor | `v1.4.0` → `v1.5.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>yorukot/superfile (yorukot/superfile)</summary> ### [`v1.5.0`](https://github.com/yorukot/superfile/releases/tag/v1.5.0) [Compare Source](yorukot/superfile@v1.4.0...v1.5.0) This release brings major new features, including video and PDF preview support, multi-column file panels, and configurable navigation, alongside significant code refactoring and comprehensive bug fixes. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Added video and PDF preview support via loading the first frame/page as an image. Thanks [@​Yassen-Higazi](https://github.com/Yassen-Higazi) for the implementation <details><summary>Screenshots</summary> <p> <img width="951" height="596" alt="image" src="https://github.com/user-attachments/assets/6edfa9c2-ebcd-4622-a115-f71fa533b3e1" /> <img width="949" height="594" alt="image" src="https://github.com/user-attachments/assets/8d15fa46-5178-422d-8eea-455cac31fdd0" /> </p> </details> - Multi-column file panel view with date/size/permission columns. Thanks [@​xelavopelk](https://github.com/xelavopelk) for the implementation <details><summary>Screenshots</summary> <p> <img width="420" height="264" alt="image" src="https://github.com/user-attachments/assets/e172f1e8-c2a5-42d2-8eeb-62e721f61a4f" /> </p> </details> - Configurable fast navigation in the Filepanel. See this MR for more details: [#​1220](yorukot/superfile#1220) - You can now configure `spf` to open files with specific extensions with your choice of editor application. Thanks [@​litvinov-git](https://github.com/litvinov-git) for the implementation See this MR for more details: [#​1197](yorukot/superfile#1197) - Terminal stdout support for shell commands - Allow launching with the filename. `spf /a/b/c.txt` will launch in `/a/b` with `c.txt` as the selected file. - Various bug fixes, including modal confirmations, layout issues, and race conditions. See 'Detailed Change Summary' ##### Internal Updates - Separated FilePanel and FileModel into a dedicated package for better code organization - Comprehensive end-to-end testing with layout fixes - Enhanced dimension validation and sidebar fixes - Updated multiple dependencies including Astro, Go toolchain, and linters - Added gosec linter and MND linter with magic number cleanup #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - allow hover to file [`#1177`](yorukot/superfile#1177) by @​lazysegtree - show count selected items in select mode [`#1187`](yorukot/superfile#1187) by @​xelavopelk - Add icon alias for kts to kt [`#1153`](yorukot/superfile#1153) by @​nicolaic - link icon and metadata [`#1171`](yorukot/superfile#1171) by @​xelavopelk - user configuration of editors by file extension [`#1197`](yorukot/superfile#1197) by @​litvinov-git - add video preview support [`#1178`](yorukot/superfile#1178) by @​Yassen-Higazi - Add pdf preview support [`#1198`](yorukot/superfile#1198) by @​Yassen-Higazi - Add icons in pinned directories [`#1215`](yorukot/superfile#1215) by @​lazysegtree - Enable fast configurable navigation [`#1220`](yorukot/superfile#1220) by @​lazysegtree - add Trash bin to default directories for Linux [`#1236`](yorukot/superfile#1236) by @​lazysegtree - add terminal stdout support for shell commands [`#1250`](yorukot/superfile#1250) by @​majiayu000 - More columns in file panel (MVP) [`#1268`](yorukot/superfile#1268) by @​xelavopelk ##### Bug Fix - only calculate checksum on files [`#1119`](yorukot/superfile#1119) by @​nikero41 - Linter issue with PrintfAndExit [`#1133`](yorukot/superfile#1133) by @​xelavopelk - Remove repeated os.ReadDir calls [`#1155`](yorukot/superfile#1155) by @​lazysegtree - Disable COPYFILE in macOS [`#1194`](yorukot/superfile#1194) by @​lazysegtree - add missing hotkeys to help menu [`#1192`](yorukot/superfile#1192) by @​lazysegtree - Fetch latest version automatically [`#1127`](yorukot/superfile#1127) by @​lazysegtree - Use async methods to prevent test race conditions [`#1201`](yorukot/superfile#1201) by @​lazysegtree - update metadata and process bar sizes when toggling footer [`#1218`](yorukot/superfile#1218) by @​lazysegtree - File panel dimension management [`#1222`](yorukot/superfile#1222) by @​lazysegtree - Layout fixes with full end-to-end tests [`#1227`](yorukot/superfile#1227) by @​lazysegtree - Fix flaky tests [`#1233`](yorukot/superfile#1233) by @​lazysegtree - modal confirmation bug with arrow keys [`#1243`](yorukot/superfile#1243) by @​lazysegtree - small file panel optimization [`#1241`](yorukot/superfile#1241) by @​xelavopelk - use ExtractOperationMsg for extraction [`#1248`](yorukot/superfile#1248) by @​lazysegtree - skip open_with from missing field validation [`#1251`](yorukot/superfile#1251) by @​lazysegtree - border height validation fixes [`#1267`](yorukot/superfile#1267) by @​lazysegtree - fix case with two active panes [`#1271`](yorukot/superfile#1271) by @​xelavopelk - help model formatting [`#1277`](yorukot/superfile#1277) by @​booth-w ##### Optimization - simplify renameIfDuplicate logic [`#1100`](yorukot/superfile#1100) by @​sarff - separate FilePanel into dedicated package [`#1195`](yorukot/superfile#1195) by @​lazysegtree - File model separation [`#1223`](yorukot/superfile#1223) by @​lazysegtree - Dimension validations [`#1224`](yorukot/superfile#1224) by @​lazysegtree - layout validation and sidebar dimension fixes [`#1228`](yorukot/superfile#1228) by @​lazysegtree - user rendering package and removal of unused preview code [`#1245`](yorukot/superfile#1245) by @​lazysegtree - user rendering package for file preview [`#1249`](yorukot/superfile#1249) by @​lazysegtree ##### Documentation - update Fish shell setup docs [`#1142`](yorukot/superfile#1142) by @​wleoncio - fix macOS typo [`#1212`](yorukot/superfile#1212) by @​wcbing - stylistic and linguistic cleanup of config documentation [`#1184`](yorukot/superfile#1184) by @​ninetailedtori ##### Dependencies - update astro monorepo [`#1010`](yorukot/superfile#1010) by @​renovate[bot] - update starlight-giscus [`#1020`](yorukot/superfile#1020) by @​renovate[bot] - bump astro versions [`#1138`](yorukot/superfile#1138), [`#1157`](yorukot/superfile#1157), [`#1158`](yorukot/superfile#1158) by @​dependabot[bot], @​renovate[bot] - bump vite [`#1134`](yorukot/superfile#1134) by @​dependabot[bot] - update setup-go action [`#1038`](yorukot/superfile#1038) by @​renovate[bot] - update expressive-code plugins [`#1189`](yorukot/superfile#1189), [`#1246`](yorukot/superfile#1246) by @​renovate[bot] - update sharp [`#1256`](yorukot/superfile#1256) by @​renovate[bot] - update fontsource monorepo [`#1257`](yorukot/superfile#1257) by @​renovate[bot] - update urfave/cli [`#1136`](yorukot/superfile#1136), [`#1190`](yorukot/superfile#1190) by @​renovate[bot] - update astro / starlight / ansi / toolchain deps [`#1275`](yorukot/superfile#1275), [`#1278`](yorukot/superfile#1278), [`#1280`](yorukot/superfile#1280) by @​renovate[bot] - update python and go versions [`#1276`](yorukot/superfile#1276), [`#1191`](yorukot/superfile#1191) by @​renovate[bot] - update golangci-lint action [`#1286`](yorukot/superfile#1286) by @​renovate[bot] ##### Misc - update CI input names [`#1120`](yorukot/superfile#1120) by @​nikero41 - Everforest Dark Hard theme [`#1114`](yorukot/superfile#1114) by @​fzahner - migrate tutorial demo assets to local [`#1140`](yorukot/superfile#1140) by @​yorukot - new logo asset [`#1145`](yorukot/superfile#1145) by @​nonepork - mirror repository to codeberg [`#1141`](yorukot/superfile#1141) by @​yorukot - sync package lock [`#1143`](yorukot/superfile#1143) by @​yorukot - bump golangci-lint version [`#1135`](yorukot/superfile#1135) by @​lazysegtree - add gosec linter [`#1185`](yorukot/superfile#1185) by @​lazysegtree - enable MND linter and clean magic numbers [`#1180`](yorukot/superfile#1180) by @​lazysegtree - skip permission tests when running as root [`#1186`](yorukot/superfile#1186) by @​lazysegtree - release v1.4.1-rc [`#1203`](yorukot/superfile#1203) by @​lazysegtree - 1.5.0-rc1 housekeeping changes [`#1264`](yorukot/superfile#1264) by @​lazysegtree </p> </details> #### New Contributors * @​fzahner made their first contribution in yorukot/superfile#1114 * @​sarff made their first contribution in yorukot/superfile#1100 * @​nicolaic made their first contribution in yorukot/superfile#1153 * @​Yassen-Higazi made their first contribution in yorukot/superfile#1178 * @​ninetailedtori made their first contribution in yorukot/superfile#1184 * @​litvinov-git made their first contribution in yorukot/superfile#1197 * @​wcbing made their first contribution in yorukot/superfile#1212 * @​majiayu000 made their first contribution in yorukot/superfile#1250 **Full Changelog**: <yorukot/superfile@v1.4.0...v1.5.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi44MC4xIiwidXBkYXRlZEluVmVyIjoiNDIuODAuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
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
2 participants
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.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.