fix: prevent VALUE/MVALUE self-recursion

[chhoumann]

Fixes #920

Prevents the formatter from hanging when user input contains {{value}} / {{mvalue}} by making VALUE/MVALUE replacement non-recursive and single-pass.

• Adds regression tests for self-referential VALUE/MVALUE input
• bun run test / bun run build pass locally

Summary by CodeRabbit

• Bug Fixes

  • Resolved issue [BUG] Using {{value}} or {{mvalue}} in the Capture input dialog causes the application to become unresponsive. #920: VALUE and MVALUE placeholders no longer cause recursive expansion or infinite loops.
  • Value prompting now occurs only once per formatter run, allowing empty strings as valid inputs.
  • Math value placeholder handling optimized to prevent infinite loop scenarios.

• Tests

  • Added regression tests for VALUE and MVALUE placeholder behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
quickadd	Ready	Preview	Dec 18, 2025 1:09pm

[coderabbitai]

Walkthrough

Adds regression tests for issue #920 and modifies formatter logic to prevent recursive expansion of VALUE and MVALUE placeholders. Changes replaceValueInString and replaceMathValueInString to use single-pass replacement instead of loops, eliminating infinite recursion when placeholders are entered as user input.

Changes

Cohort / File(s)	Summary
Formatter core logic src/formatters/formatter.ts	Modified replaceValueInString to add early exit guard and prompt once per run with single non-recursive replacement pass. Modified replaceMathValueInString to scan input in one pass instead of loop-based replacement, preventing infinite loops when input contains {{MVALUE}}.
Regression tests src/formatters/formatter-issue920.test.ts	Added test file with Issue920TestFormatter subclass and four test cases validating that VALUE and MVALUE placeholders do not cause recursive expansion when entered as user input.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• replaceValueInString logic change: Verify early guard condition is correct and single-pass replacement properly handles all regex matches without leaving unconverted placeholders
• replaceMathValueInString refactor: Ensure index advancement in scanning loop correctly skips over replaced matches to prevent re-processing
• Test coverage: Confirm test cases adequately simulate user input scenarios and that the custom formatter subclass correctly overrides behavior for testing

Poem

🐰 A loop that spiraled round and round,
Now halts before it's found—
VALUE and MVALUE tamed at last,
Single passes break the cast,
No more infinite requests! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: prevent VALUE/MVALUE self-recursion' directly summarizes the main change: addressing recursive expansion of VALUE/MVALUE placeholders that causes the application to hang.
Linked Issues check	✅ Passed	The PR implements all core requirements from issue #920: prevents recursive expansion of {{value}} and {{mvalue}} via single-pass replacements [#920], adds regression tests to guard against future regressions [#920], and ensures non-recursive replacement to avoid infinite loops [#920].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #920: formatter.ts modifies VALUE/MVALUE replacement logic, and formatter-issue920.test.ts adds regression tests; both directly address the self-recursion bug.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/920-value-mvalue-recursion

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/formatters/formatter-issue920.test.ts (1)

87-117: Comprehensive regression coverage for the core issue.

The four test cases correctly verify that:

1. Self-referential input (returning {{VALUE}} or {{MVALUE}} from prompts) is treated as literal.
2. Partial matches (e.g., prefix {{VALUE}}) do not trigger recursive expansion.

The test at lines 100-104 covers multiple {{VALUE}} occurrences, which is excellent.

Optional: Add a test case for multiple MVALUE occurrences

Consider adding a test to verify behavior when multiple {{MVALUE}} placeholders are present:

+	it("handles multiple {{MVALUE}} placeholders independently", async () => {
+		formatter.setMathResponse("5");
+		const result = await formatter.testFormat("A {{MVALUE}} B {{MVALUE}} C");
+		expect(result).toBe("A 5 B 5 C");
+	});

This would document the intended behavior (each occurrence prompts independently) and guard against regressions.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41744eb and a623dc4.

📒 Files selected for processing (2)

• src/formatters/formatter-issue920.test.ts (1 hunks)
• src/formatters/formatter.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Source code lives in src/: core logic under engine/, services/, and utils/; Svelte UI in src/gui; shared types in src/types; settings entry in src/quickAddSettingsTab.ts

Files:

• src/formatters/formatter-issue920.test.ts
• src/formatters/formatter.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through the logger utilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters or tests/obsidian-stub.ts

Files:

• src/formatters/formatter-issue920.test.ts
• src/formatters/formatter.ts

src/**/*.{ts,tsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for classes and Svelte components

Files:

• src/formatters/formatter-issue920.test.ts
• src/formatters/formatter.ts

🧠 Learnings (1)

📚 Learning: 2025-12-09T21:20:52.425Z

Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes

Applied to files:

• src/formatters/formatter-issue920.test.ts

🧬 Code graph analysis (1)

src/formatters/formatter.ts (1)

src/constants.ts (2)

• NAME_VALUE_REGEX (71-71)
• MATH_VALUE_REGEX (91-91)

🪛 ast-grep (0.40.0)

src/formatters/formatter.ts

[warning] 147-147: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(NAME_VALUE_REGEX.source, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

---

[warning] 370-370: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(MATH_VALUE_REGEX.source, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

🔇 Additional comments (4)

src/formatters/formatter-issue920.test.ts (1)

7-85: Well-designed test fixture.

The Issue920TestFormatter class provides clean test isolation by stubbing all abstract methods and exposing configurable prompt responses. This focused approach allows you to test the specific replacement logic without dependencies on the full Obsidian app context.

src/formatters/formatter.ts (3)

130-152: Excellent fix for VALUE self-recursion.

The implementation correctly prevents infinite loops by:

1. Fast-path optimization (line 133): skips work when no placeholders exist.
2. Single prompt (line 142): prompts once per formatter run, allowing empty string as valid input.
3. Single-pass replacement (line 148): the replacer function ensures all occurrences are replaced in one pass without re-scanning the output, and treats $ in user input literally (not as a special replacement character).

The logic preserves programmatic value injection via hasConcreteVariable("value") (line 137), maintaining backward compatibility.

---

368-385: Correct single-pass implementation for MVALUE.

The refactored logic prevents infinite loops by scanning the original input string once and building output incrementally:

1. The regex scans input (which never changes), not the accumulated output.
2. The local lastIndex variable tracks the copy position in input for building segments.
3. Each {{MVALUE}} triggers a prompt (line 379), which differs from VALUE behavior (prompt once, reuse for all). This appears intentional for supporting independent math calculations at each placeholder.

The approach ensures that if a user enters {{MVALUE}} as input to the math prompt, it won't trigger re-scanning because we're iterating over the original input, not the output being constructed.

---

148-148: Static analysis warnings are false positives.

The ast-grep tool flags lines 148 and 371 for potential ReDoS vulnerabilities. However, both NAME_VALUE_REGEX and MATH_VALUE_REGEX are constant patterns defined in src/constants.ts:

export const NAME_VALUE_REGEX = new RegExp(/{{NAME}}|{{VALUE}}/i);
export const MATH_VALUE_REGEX = new RegExp(/{{MVALUE}}/i);

These are simple literal alternations with no user input, complex quantifiers, or backtracking. There is no ReDoS risk here.

Also applies to: 371-371

[github-actions]

🎉 This PR is included in version 2.9.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🚀 Release has been published: v2.9.4