feat(cli): add native QuickAdd Obsidian CLI handlers#1129
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new QuickAdd CLI (run/list/check), centralized preflight requirement collection, related tests and docs, CLI type declarations, and registers CLI handlers during plugin startup after workspace layout readiness. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Client
participant Handler as QuickAdd CLI Handler
participant Executor as ChoiceExecutor
participant Preflight as collectChoiceRequirements
participant Choice as Choice Engine
CLI->>Handler: quickadd:run (params + vars)
Handler->>Handler: parse params, parse vars JSON, apply value-* overrides
Handler->>Executor: create/set up executor and apply variables
Handler->>Preflight: request FieldRequirements (choice, executor, options)
Preflight-->>Handler: FieldRequirements list
Handler->>Preflight: request unresolved inputs (requirements, variables)
Preflight-->>Handler: unresolved inputs list
alt non-interactive & unresolved inputs
Handler-->>CLI: JSON payload with missingFields and missingFlags
else all inputs resolved
Handler->>Executor: execute choice
Executor->>Choice: run choice
Choice-->>Executor: result (duration, success)
Executor-->>Handler: execution result
Handler-->>CLI: JSON success response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Deploying quickadd with
|
| Latest commit: |
68a7127
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7ac8658c.quickadd.pages.dev |
| Branch Preview URL: | https://feat-quickadd-cli-handlers.quickadd.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/global.d.ts (1)
8-10: Redundant type union in CliData.The type
string | "true"is redundant since"true"is a subset ofstring. The literal"true"provides no additional type safety here.💡 Simplified type definition
interface CliData { - [key: string]: string | "true"; + [key: string]: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/global.d.ts` around lines 8 - 10, The CliData interface currently types values as `string | "true"`, which is redundant because the string literal "true" is already encompassed by string; update the CliData declaration (interface CliData) to use a single string type for values (e.g., change the union to just string) so the type is simplified and consistent.src/preflight/collectChoiceRequirements.ts (1)
242-244: Silent error swallowing may hide script configuration issues.Catching and ignoring all errors from
getUserScriptduring preflight may hide legitimate configuration problems. Consider logging at debug level to aid troubleshooting.💡 Optional: Add debug logging for script errors
} catch { - // Ignore script spec errors in preflight collection. + // Ignore script spec errors in preflight collection. + // Uncomment for debugging: log.logMessage(`Preflight: skipped script ${userScriptCommand.path}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/preflight/collectChoiceRequirements.ts` around lines 242 - 244, The catch block that silently swallows errors from getUserScript in collectChoiceRequirements.ts should at minimum log the caught error at debug level instead of ignoring it; update the catch to accept an error (e.g., err) and call the module’s logger/debug method (or processLogger.debug) with a concise message referencing getUserScript and the error details so configuration/script failures are visible during troubleshooting while still not failing preflight.src/cli/registerQuickAddCliHandlers.test.ts (1)
203-232: Consider adding a test forvarsJSON parsing.The test suite covers
value-<name>and extrakey=valuepatterns but doesn't exercise thevars=<json>parameter. Consider adding coverage for JSON variable ingestion.💡 Suggested test case for vars JSON parsing
it("parses vars JSON parameter correctly", async () => { const { plugin, handlers } = createPlugin([templateChoice]); registerQuickAddCliHandlers(plugin); const run = handlers.find((handler) => handler.command === "quickadd:run"); const output = await Promise.resolve( run!.handler({ choice: templateChoice.name, vars: '{"project":"QA","sprint":42}', }), ); const payload = JSON.parse(String(output)); expect(payload.ok).toBe(true); expect(executors[0].variables.get("project")).toBe("QA"); expect(executors[0].variables.get("sprint")).toBe(42); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/registerQuickAddCliHandlers.test.ts` around lines 203 - 232, Add a unit test that exercises JSON parsing of the vars CLI parameter: after calling registerQuickAddCliHandlers and finding the "quickadd:run" handler, invoke run!.handler with a vars string like '{"project":"QA","sprint":42}' and assert payload.ok is true and that the executor received those variables (check executors[0].variables.get("project") === "QA" and .get("sprint") === 42). This test should live alongside the existing non-interactive tests that use templateChoice and should verify that registerQuickAddCliHandlers correctly parses the vars JSON into executor variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/registerQuickAddCliHandlers.test.ts`:
- Around line 203-232: Add a unit test that exercises JSON parsing of the vars
CLI parameter: after calling registerQuickAddCliHandlers and finding the
"quickadd:run" handler, invoke run!.handler with a vars string like
'{"project":"QA","sprint":42}' and assert payload.ok is true and that the
executor received those variables (check executors[0].variables.get("project")
=== "QA" and .get("sprint") === 42). This test should live alongside the
existing non-interactive tests that use templateChoice and should verify that
registerQuickAddCliHandlers correctly parses the vars JSON into executor
variables.
In `@src/global.d.ts`:
- Around line 8-10: The CliData interface currently types values as `string |
"true"`, which is redundant because the string literal "true" is already
encompassed by string; update the CliData declaration (interface CliData) to use
a single string type for values (e.g., change the union to just string) so the
type is simplified and consistent.
In `@src/preflight/collectChoiceRequirements.ts`:
- Around line 242-244: The catch block that silently swallows errors from
getUserScript in collectChoiceRequirements.ts should at minimum log the caught
error at debug level instead of ignoring it; update the catch to accept an error
(e.g., err) and call the module’s logger/debug method (or processLogger.debug)
with a concise message referencing getUserScript and the error details so
configuration/script failures are visible during troubleshooting while still not
failing preflight.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/docs/Advanced/CLI.mddocs/docs/Advanced/ObsidianUri.mdsrc/cli/registerQuickAddCliHandlers.test.tssrc/cli/registerQuickAddCliHandlers.tssrc/global.d.tssrc/main.tssrc/preflight/collectChoiceRequirements.tssrc/preflight/runOnePagePreflight.ts
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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
1 participant
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.
Add first-class Obsidian CLI support for QuickAdd choices.
This introduces native plugin CLI handlers so shell workflows can run QuickAdd choices directly and safely in non-interactive mode.
What changed:
quickaddandquickadd:run(execute a choice bychoice=<name>orid=<id>)quickadd:list(list flattened choice inventory)quickadd:check(preflight unresolved required inputs)value-<name>=...(URI-compatible)key=valueparamsvars=<json-object>missingandmissingFlagssuggestionsNotes:
registerCliHandler) to preserve compatibility on older Obsidian versions.Summary by CodeRabbit
New Features
Documentation
Tests
Chores