Skip to content

fix(macro): let a user script that exports nothing finish its macro - #2209

Merged
chhoumann merged 2 commits into
masterfrom
2208-bug-user-scripts-are-running-throwing-error
Oct 9, 2026
Merged

chhoumann merged 2 commits into
masterfrom
2208-bug-user-scripts-are-running-throwing-error

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Why

A user script that does all its work at the top level and exports nothing ran fine. QuickAdd then threw "is an empty object", showed an error notice, and stopped the macro, so later steps never ran. Fixes #2208.

What changed

  • A user script whose module exports nothing now finishes with no output, and the macro continues.
  • A selected member (Script::member) that is an empty object still raises the error.

Scope

  • Covers the undrilled module export only.
  • Leaves out settings-only exports (module.exports = { settings }), which still open the member picker. That behavior predates this PR.

Tradeoffs

A script that defines a function but forgets to export it now does nothing silently, where before it showed this error. QuickAdd cannot tell that case apart from an intentional top-level script. The old error also fired only after the script had run, so it stopped the macro without undoing anything.

Blast Radius

Only user-script macro steps whose module exports nothing change behavior. The empty member picker from #404 still cannot open.

Verification

  • Obsidian 1.14.3, macro with the reporter's script followed by a marker script. Before: the folder was created, step 2 did not run, and one error notice showed. After: the folder was created, step 2 ran, and no error or notice showed.
  • The regression test fails on master with user script in macro for 'Todoist' is an empty object. A second test fails if the exception also covers drilled members.
  • pnpm run test passed 6851 tests. pnpm run lint and pnpm run build passed.

Note

Fix: allow user scripts with no exports to finish their macro

A user script whose module loads as an empty record and has no selected member now returns with an undefined result instead of failing. The top-level code still runs first. Member-drilled selections that resolve to an empty object are still rejected by the existing validation.

Adds tests for both paths in userScriptExecution.test.ts, plus a createApp helper that accepts a custom script source.

Macroscope summarized 7fc1233.

Summary by CodeRabbit

  • Bug Fixes
    • User scripts with no exports can now run their top-level code successfully when no exported member is selected, returning an undefined result.
    • Selecting a member from an empty exported object continues to produce an error.

@chhoumann chhoumann linked an issue Oct 9, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 706b9997-52b1-4822-a47e-5808e7d86246
📥 Commits

Reviewing files that changed from the base of the PR and between 26eb73a and 7fc1233.

📒 Files selected for processing (2)
  • src/engine/userScriptExecution.test.ts
  • src/engine/userScriptExecution.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

executeUserScript now accepts an empty exported object when no member is selected. Tests cover top-level script execution, its undefined result, and rejection when selecting a member.

Changes

Empty User Script Handling

Layer / File(s) Summary
Handle scripts with no exports
src/engine/userScriptExecution.ts, src/engine/userScriptExecution.test.ts
Execution returns when a script has no exported values and the command does not select a member. Tests check that top-level code runs with an undefined result, and that selecting a member rejects with an “is an empty object” error.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 7fc12

User scripts that export nothing now finish and let the macro continue instead of raising an error. Selecting an empty member still errors. No merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: allowing user scripts that export nothing to complete macro execution.
Linked Issues check Passed Issue #2208 requires a top-level user script with no exports to run without an error. The change detects an undrilled empty module export in executeUserScript and returns without delegating to the e…
Out of Scope Changes check Passed The diff changes only user-script execution and its regression tests. The execution change directly implements issue #2208. The tests support both the new undrilled behavior and the retained drilled-m…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: package-manager metadata or lockfile failed a supply-chain integrity policy. Refresh the packageManager pin and lockfile locally.


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.

❤️ Share

A rabbit ran a script at night
No export came, yet all ran right
The empty bundle raised no fuss
A chosen member still rejects
Then carrots fill the test suite’s nest

Comment @coderabbitai help to get the list of available commands.

@chhoumann

Copy link
Copy Markdown
Owner Author

Independent verification: PASS

Head: 7fc12331f9ca41ade2bd22eb0ca88fe8a6a880b3
Base branch: origin/master, freshly fetched at 40dd5b4f9168b73ee5a37ef55e4b8500c4750cf5
Merge-base: 26eb73a54151056f0910f020ee52475fd5584344
Stable patch-id of merge-base-to-head diff: c1bb42b86cf21c952a768849316f28e2713e43e6

Surface: Obsidian app/installer 1.14.3 on macOS, isolated vault quickadd-verify-2209 under .claude/worktrees/verify-2209/.obsidian-e2e-vaults/. Base testing used the head tree with only src/engine/userScriptExecution.ts replaced by the base version. Both variants passed pnpm run build and were reloaded into Obsidian before testing.

  • Reporter macro, using the issue's exact top-level fs/path folder script followed by the marker script: base created the folder, then threw user script in macro for 'Verify2209 reporter' is an empty object, logged one console error, showed one matching error Notice, and left the marker null. Head created the folder and returned "marker":"ran","thrown":null,"consoleErrors":[],"notices":[].
  • Negative control lib::start with module.exports = { start: {} };: both base and head threw user script in macro for 'Verify2209 empty member' is an empty object, logged the error, showed the matching Notice, and skipped the marker step.
  • Function export and multi-export object with multi::start: both variants ran the intended functions. Actual output through quickAddApi.format and {{MACRO:...}} was function output 2209 and member output 2209, with no errors or new Notices.
  • pnpm exec vitest run src/engine/userScriptExecution.test.ts: head 5 passed (5); base 1 failed | 4 passed (5). The sole base failure was runs the top-level code and finishes without output, with user script in macro for 'Todoist' is an empty object.
  • Inspected the error Notice and clean head UI, captured console/errors and workspace state, and confirmed the layout stayed unchanged. Removed test files and the detached worktree, stopped its private Obsidian instance, and confirmed the main checkout's Git status matches the pre-test baseline.

@chhoumann
chhoumann merged commit 2abd73c into master Oct 9, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] User Scripts are Running & Throwing Error

1 participant