read task info from list format instead of CSV#547
Conversation
WalkthroughThe changes introduce a new Windows YAML configuration example and refactor the Windows scheduled task parsing logic from CSV-based to LIST-based output. New parsing and test logic for LIST-formatted output are added, and related functions and tests are updated. Logging and error handling are also adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskScheduler
participant schtasks.exe
participant Parser
User->>TaskScheduler: Request registered tasks
TaskScheduler->>schtasks.exe: Run schtasks /query /fo list /v /tn "task"
schtasks.exe-->>TaskScheduler: Returns LIST-formatted output
TaskScheduler->>Parser: Parse output with getTaskInfoFromList
Parser-->>TaskScheduler: Returns parsed task info (map)
TaskScheduler-->>User: Returns registered task details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were identified. Suggested labels
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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). (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/other windows.yaml (1)
1-1: Fix line ending format.The static analysis tool flagged an incorrect line ending character. Please ensure the file uses LF (\n) line endings as expected.
-# yaml-language-server: $schema=https://creativeprojects.github.io/resticprofile/jsonschema/config.json +# yaml-language-server: $schema=https://creativeprojects.github.io/resticprofile/jsonschema/config.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/other windows.yaml(1 hunks)schtasks/list.go(1 hunks)schtasks/list_test.go(1 hunks)schtasks/schtasks.go(3 hunks)schtasks/schtasks_test.go(2 hunks)schtasks/taskscheduler.go(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: schtasks/schtasks.go:29-30
Timestamp: 2025-02-14T22:53:42.689Z
Learning: In the `schtasks` package, `tasksPath` is defined as a constant with value `\resticprofile backup\` in `taskscheduler.go`. It's used as a prefix for managing task paths in the Windows Task Scheduler.
Learnt from: zumm
PR: creativeprojects/resticprofile#541
File: schtasks/taskscheduler.go:60-67
Timestamp: 2025-07-29T16:14:02.636Z
Learning: In the schtasks package, double quotes cannot be used for command wrapping because `getTaskInfo` can't process them and unschedule commands fail when double quotes are used.
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: examples/windows.yaml:41-41
Timestamp: 2025-02-14T22:53:45.818Z
Learning: Changes to example configurations in the `examples/` directory do not require changelog entries as they are meant for testing and demonstration purposes only.
📚 Learning: in the `schtasks` package, `taskspath` is defined as a constant with value `\resticprofile backup\` ...
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: schtasks/schtasks.go:29-30
Timestamp: 2025-02-14T22:53:42.689Z
Learning: In the `schtasks` package, `tasksPath` is defined as a constant with value `\resticprofile backup\` in `taskscheduler.go`. It's used as a prefix for managing task paths in the Windows Task Scheduler.
Applied to files:
schtasks/taskscheduler.goschtasks/list_test.goschtasks/schtasks.goschtasks/list.goexamples/other windows.yaml
📚 Learning: the shell.splitarguments function in the resticprofile project returns only []string and does not re...
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#425
File: schedule/handler_windows.go:97-118
Timestamp: 2025-02-04T14:38:07.701Z
Learning: The shell.SplitArguments function in the resticprofile project returns only []string and does not return any error.
Applied to files:
schtasks/list_test.goschtasks/list.go
📚 Learning: in the `schtasks` package, `taskspath` is defined as a constant and is used for filtering registered...
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: schtasks/schtasks.go:29-30
Timestamp: 2025-02-14T22:53:42.689Z
Learning: In the `schtasks` package, `tasksPath` is defined as a constant and is used for filtering registered tasks by their path prefix.
Applied to files:
schtasks/schtasks.go
📚 Learning: in the schtasks package, double quotes cannot be used for command wrapping because `gettaskinfo` can...
Learnt from: zumm
PR: creativeprojects/resticprofile#541
File: schtasks/taskscheduler.go:60-67
Timestamp: 2025-07-29T16:14:02.636Z
Learning: In the schtasks package, double quotes cannot be used for command wrapping because `getTaskInfo` can't process them and unschedule commands fail when double quotes are used.
Applied to files:
schtasks/list.go
📚 Learning: in the schtasks package, `config.command` is guaranteed to be just a path to executable, making sing...
Learnt from: zumm
PR: creativeprojects/resticprofile#541
File: schtasks/taskscheduler.go:60-67
Timestamp: 2025-07-29T16:14:02.636Z
Learning: In the schtasks package, `config.Command` is guaranteed to be just a path to executable, making single quotes sufficient for wrapping in command construction.
Applied to files:
schtasks/list.go
📚 Learning: changes to example configurations in the `examples/` directory do not require changelog entries as t...
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: examples/windows.yaml:41-41
Timestamp: 2025-02-14T22:53:45.818Z
Learning: Changes to example configurations in the `examples/` directory do not require changelog entries as they are meant for testing and demonstration purposes only.
Applied to files:
examples/other windows.yaml
🪛 YAMLlint (1.37.1)
examples/other windows.yaml
[error] 1-1: wrong new line character: expected \n
(new-lines)
🔇 Additional comments (14)
examples/other windows.yaml (1)
3-34: LGTM! Well-structured Windows configuration example.The configuration demonstrates proper usage of resticprofile features including global settings, profile inheritance, backup exclusions, and Windows task scheduling. This example complements the improved Windows task parsing capabilities introduced in this PR.
schtasks/taskscheduler.go (2)
134-134: LGTM! Helpful debug logging addition.Adding debug logging before loading each task will improve troubleshooting capabilities when diagnosing task parsing issues.
137-137: LGTM! Appropriate error formatting change.Changing from %w to %s is correct for logging context where error wrapping isn't needed.
schtasks/schtasks_test.go (2)
21-21: LGTM! Test updated to match refactored function name.The test correctly calls the renamed
getTaskInfoFromCSVfunction, maintaining coverage of CSV parsing functionality.
27-47: LGTM! Test properly updated for new function signature.The test function rename and call to
getTaskInfocorrectly reflect the refactored API while maintaining proper error testing coverage.schtasks/list_test.go (1)
74-146: LGTM! Comprehensive test coverage for list parsing.The test provides excellent coverage of the new list format parsing functionality with realistic test data that includes quoted arguments in command lines (addressing the original issue #545). The expected output verification ensures correctness of the key-value pair parsing logic.
schtasks/list.go (2)
18-63: LGTM! Robust implementation of list format parsing.The parser effectively addresses the original issue by handling key-value pairs instead of CSV format, which eliminates problems with quoted arguments. Good error handling for invalid formats and duplicate keys ensures reliable parsing.
Key strengths:
- Handles both separator formats (
:and:) found in Windows schtasks output- Proper folder context tracking for task hierarchy
- Efficient line-by-line processing with bufio.Scanner
- Clear error messages for debugging
65-72: LGTM! Clean helper function for field access.The
getFirstFieldfunction provides a clean API for accessing parsed task data, replacing the previous version that worked with CSV data structures.schtasks/schtasks.go (6)
21-21: Function rename improves clarity.The renaming from
getCSVtogetTaskInfoFromCSVbetter reflects the specific purpose of parsing task information from CSV format, improving code readability.
36-36: Return type change enhances data access.Changing the return type from
[][]stringto[]map[string]stringprovides better structured access to task properties by name rather than requiring positional indexing, which aligns well with the switch to LIST format parsing.
49-52: Function rename maintains CSV parsing capability.The renamed
getTaskInfoFromCSVfunction preserves the existing CSV parsing logic, which is still needed for thegetRegisteredTasksfunction. The implementation remains unchanged, ensuring no regression in CSV parsing functionality.
127-128: Comment accurately reflects the change.The updated comment correctly indicates that the function returns raw output rather than specifically CSV format, which aligns with the switch to LIST format.
134-134: Command format change addresses the core issue.The switch from
/fo csvto/fo listdirectly addresses the parsing issue described in PR #545. The LIST format should handle quoted arguments more reliably than CSV format, which struggles with unescaped quotes within fields.This change is critical for fixing the bug where tasks with quoted command arguments (such as configuration file paths with spaces) couldn't be parsed correctly.
42-42: VerifiedgetTaskInfoFromListimplementationThe
getTaskInfoFromListfunction in schtasks/list.go correctly parses the LIST output and maintains robust error handling:
- Splits each line first on
": "then on": "to accommodate spacing variations, trimming both key and value.- Tracks the current folder via the
Folderkey and appends it to each record.- Returns errors for invalid line formats, duplicate keys and any scanner errors.
No further changes are required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/profile_test.go (1)
828-828: Consider removing debug logging.The
t.Log(profile)statement appears to be for debugging purposes. Consider removing it unless it provides valuable test output for ongoing debugging.- t.Log(profile)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config/profile_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: schtasks/schtasks.go:29-30
Timestamp: 2025-02-14T22:53:42.689Z
Learning: In the `schtasks` package, `tasksPath` is defined as a constant with value `\resticprofile backup\` in `taskscheduler.go`. It's used as a prefix for managing task paths in the Windows Task Scheduler.
Learnt from: zumm
PR: creativeprojects/resticprofile#541
File: schtasks/taskscheduler.go:60-67
Timestamp: 2025-07-29T16:14:02.636Z
Learning: In the schtasks package, double quotes cannot be used for command wrapping because `getTaskInfo` can't process them and unschedule commands fail when double quotes are used.
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: examples/windows.yaml:41-41
Timestamp: 2025-02-14T22:53:45.818Z
Learning: Changes to example configurations in the `examples/` directory do not require changelog entries as they are meant for testing and demonstration purposes only.
📚 Learning: the shell.splitarguments function in the resticprofile project returns only []string and does not re...
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#425
File: schedule/handler_windows.go:97-118
Timestamp: 2025-02-04T14:38:07.701Z
Learning: The shell.SplitArguments function in the resticprofile project returns only []string and does not return any error.
Applied to files:
config/profile_test.go
⏰ 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). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Build and test (1.24, ubuntu-latest)
- GitHub Check: Build and test (1.24, windows-latest)
- GitHub Check: Build and test (1.24, macos-latest)
🔇 Additional comments (2)
config/profile_test.go (2)
711-714: LGTM! Proper shell argument escaping test setup.The introduction of
expectedBackupSourcewithshell.NewArg(value, shell.ArgConfigEscape).String()correctly tests that backup source paths are properly escaped for shell usage, which aligns with the Windows task scheduling improvements in this PR.
829-829: LGTM! Updated assertions for escaped backup sources.The test assertions correctly expect the escaped format (
expectedBackupSource) rather than the raw format, ensuring that the shell argument escaping functionality works as intended for both implicit and explicit path copying scenarios.Also applies to: 837-837
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #547 +/- ##
==========================================
+ Coverage 79.45% 79.52% +0.07%
==========================================
Files 136 137 +1
Lines 13386 13425 +39
==========================================
+ Hits 10635 10675 +40
Misses 2331 2331
+ Partials 420 419 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
PR is ready to merge, |
|
I have tested it with spaces in path to config file and it seems like unschedule/status commands work fine. |
2 participants
Refactor Windows task scheduler to use list format instead of CSV
This PR refactors the Windows task scheduler implementation to read task information from the schtasks list format instead of CSV format.
Key Changes:
getTaskInfoFromList()function to parse key-value pairs from list format output/fo listinstead of/fo csv[][]string(CSV rows) to[]map[string]string(key-value records):and:)Benefits:
The change maintains backward compatibility while providing a more reliable way to extract Windows scheduled task information.
Fixes #545