Skip to content

Fix obsidian shell-out to use obsidian-cli instead of obsidian - #1474

Open
ramansrivastava wants to merge 1 commit into
akashgit:mainfrom
ramansrivastava:fix/obsidian-cli-binary-name
Open

Fix obsidian shell-out to use obsidian-cli instead of obsidian#1474
ramansrivastava wants to merge 1 commit into
akashgit:mainfrom
ramansrivastava:fix/obsidian-cli-binary-name

Conversation

@ramansrivastava

Copy link
Copy Markdown
Contributor

Summary

  • _obsidian_create() and _obsidian_search() in factory/obsidian/notes.py invoked a binary named obsidian, but the actual obsidian-cli binary is named obsidian-cli. On systems with the real Obsidian desktop app installed (also named obsidian), this launched the GUI app instead, waited out the 10s subprocess timeout, then force-killed it — causing visible window flicker and potential stale SingletonLock files.
  • Changed both invocations to use obsidian-cli. If it's not installed, subprocess.run raises FileNotFoundError, which is already caught and falls back to the existing direct file-write path.
  • Updated tests/test_obsidian.py::test_write_experiment_tries_cli_first, which had hardcoded the incorrect "obsidian" binary name in its mock/assertion.

Fixes #1460

Test plan

  • pytest tests/test_obsidian.py -v — 25 passed
  • ruff check factory/obsidian/notes.py tests/test_obsidian.py — clean
  • mypy factory/obsidian/notes.py — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_017LiyU9TiqgNaGVu3gWaqE4

The obsidian real desktop app binary is also named `obsidian` on
systems where it's installed, so _obsidian_create()/_obsidian_search()
were launching the GUI app instead of obsidian-cli, waiting out the
10s timeout, then force-killing it. Repeated SIGKILLs can also leave
a stale Electron SingletonLock behind.

Fixes akashgit#1460

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017LiyU9TiqgNaGVu3gWaqE4
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.81%. Comparing base (781194f) to head (c9d0c2d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1474      +/-   ##
==========================================
+ Coverage   82.79%   82.81%   +0.01%     
==========================================
  Files         225      225              
  Lines       24994    24994              
  Branches     4021     4021              
==========================================
+ Hits        20694    20698       +4     
+ Misses       3342     3341       -1     
+ Partials      958      955       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

factory archive shells out to obsidian and launches/kills the real desktop app instead of obsidian-cli

1 participant