feat: add support for paths in resource commands#719
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| it("returns true for paths containing backslash", () => { | ||
| expect(isPathArg("workflows\\new-comment")).to.equal(true); | ||
| }); |
There was a problem hiding this comment.
Backslash path test fails on Linux and macOS
Medium Severity
The test asserts isPathArg("workflows\\new-comment") returns true, but on Linux/macOS where path.sep is /, the string (containing only \) doesn't match either check in isPathArg. The function correctly treats \ as a non-separator on Unix, but the test incorrectly assumes it's always recognized as a path separator. This test will fail on CI if it runs on Linux or macOS.
| const props = merge(this.props, { | ||
| flags: { annotate: true }, | ||
| args: { workflowKey: workflow.key }, | ||
| }); |
There was a problem hiding this comment.
Unnecessary args merge in push commands
Low Severity
The args: { workflowKey: workflow.key } merge (and equivalent for all other resource push commands) is unused. All upsert* methods in api-v1.ts destructure only { flags } from the props parameter and never read args. This dead code adds confusion about the API contract. The PR reviewers already flagged this concern.
Additional Locations (2)
2 participants
Description
Adds support for
knock workflow push /some/path/to/workflowinstead of justknock workflow push <key>.Adding this because agents seem to default to it, and it feels like we should support it given it's essentially how our system works.
Tasks
KNO-11716