Skip to content

Commit d2cb8ba

Browse files
committed
fix: Prevent API calls during discovery and correct arg parsing
- Python Library (`ObsidianPluginDevPythonToJS.py`): - Fixed `NameError: name 'args' is not defined` in `_handle_cli_args` by adding `args, unknown = parser.parse_known_args()`. - Ensured `_script_settings_definitions` defaults to an empty list if not defined by the user script before `_handle_cli_args` is called. - Introduced `OBSIDIAN_BRIDGE_MODE` environment variable handling: - Plugin sets `OBSIDIAN_BRIDGE_MODE="discovery"` when running scripts for settings discovery. - Library `__init__` reads this mode. - `_send_receive` now checks `self._execution_mode`. If "discovery", it raises an `ObsidianCommError` immediately, preventing API calls and providing a clear error message. This effectively sandboxes library API usage during discovery for scripts not properly handling `--get-settings-json`. - TypeScript Plugin (`src/python_executor.ts`): - Added `OBSIDIAN_BRIDGE_MODE: "discovery"` to the environment variables when spawning scripts for settings discovery. This resolves issues where scripts not handling `--get-settings-json` could execute unintended API calls during the plugin's startup/settings refresh phase. It also fixes a bug in the Python library's CLI argument handling. Auto-start logic appears to be functioning correctly with existing settings checks.
1 parent 900d036 commit d2cb8ba

File tree

2 files changed

+94
-33
lines changed

2 files changed

+94
-33
lines changed

ObsidianPluginDevPythonToJS.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def _handle_cli_args():
121121
)
122122
# Parse only known args defined above, ignore others that might be passed
123123
# This prevents errors if Obsidian or the user passes other args accidentally
124-
124+
args, unknown = parser.parse_known_args()
125125
# --- Check for Event Trigger Environment Variables ---
126126
# This check happens AFTER parsing CLI args but BEFORE checking --get-settings-json
127127
# because event triggers should take precedence over settings discovery.
@@ -142,15 +142,17 @@ def _handle_cli_args():
142142
# print("DEBUG: --get-settings-json flag detected.", file=sys.stderr) # Optional debug
143143
try:
144144
# Use the globally stored definitions populated by define_settings
145-
json_output = json.dumps(_script_settings_definitions) # No indent for production
145+
# Ensure _script_settings_definitions exists, default to empty list if not defined by user script
146+
definitions_to_output = _script_settings_definitions if '_script_settings_definitions' in globals() else []
147+
json_output = json.dumps(definitions_to_output) # No indent for production
146148
print(json_output) # Print JSON to stdout
147149
sys.exit(0) # Exit successfully, preventing rest of script execution
148150
except TypeError as e:
149151
# Error during JSON serialization (e.g., non-serializable default value)
150152
error_msg = {
151153
"status": "error",
152154
"error": f"Failed to serialize settings definitions to JSON: {e}. Check default values.",
153-
"definitions": _script_settings_definitions # Include definitions for debugging
155+
"definitions": definitions_to_output # Include definitions for debugging
154156
}
155157
print(json.dumps(error_msg), file=sys.stderr)
156158
sys.exit(1) # Exit with error
@@ -217,6 +219,12 @@ def __init__(self, http_port: int = HTTP_PORT, connect_timeout: float = 2.0, req
217219
self.session = requests.Session()
218220
# Store script path for event listener registration payload
219221
self._script_relative_path_for_api: Optional[str] = None
222+
223+
# --- Determine execution mode (normal or discovery) ---
224+
self._execution_mode = os.environ.get("OBSIDIAN_BRIDGE_MODE", "normal")
225+
if self._execution_mode == "discovery":
226+
# Use stderr for info messages during discovery to avoid polluting stdout (which is used for JSON)
227+
print("INFO: ObsidianPluginDevPythonToJS: Initialized in settings discovery mode. API calls will be disabled.", file=sys.stderr)
220228

221229
# --- Read script relative path from environment variable ---
222230
# This is crucial for the get_script_settings method.
@@ -309,6 +317,15 @@ def _send_receive(self, action: str, payload: Optional[Dict[str, Any]] = None, t
309317
with status 'error'. Includes the failing action name.
310318
requests.exceptions.RequestException: For underlying HTTP request issues.
311319
"""
320+
# --- Prevent API calls during settings discovery ---
321+
if self._execution_mode == "discovery":
322+
error_message = (
323+
"API calls are disabled during settings discovery mode. "
324+
"Ensure your script handles the --get-settings-json argument "
325+
"(e.g., using _handle_cli_args() before initializing ObsidianPluginDevPythonToJS)."
326+
)
327+
# Raise specific error to indicate the cause clearly
328+
raise ObsidianCommError(error_message, action=action)
312329
request_data = {"action": action, "payload": payload if payload is not None else {}}
313330
request_timeout = timeout if timeout is not None else self.request_timeout
314331
response_text = "" # Initialize for potential use in error reporting

src/python_executor.ts

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ export async function discoverScriptSettings(plugin: ObsidianPythonBridge, scrip
8181
discoveryPYTHONPATH = `${discoveryPYTHONPATH}${path.delimiter}${currentPYTHONPATH}`;
8282
}
8383
// --- End PYTHONPATH for discovery ---
84-
const env = { ...process.env, PYTHONPATH: discoveryPYTHONPATH };
84+
const env = {
85+
...process.env, // Inherit existing environment
86+
PYTHONPATH: discoveryPYTHONPATH,
87+
OBSIDIAN_BRIDGE_MODE: "discovery"
88+
};
8589
// --- End PYTHONPATH for discovery ---
8690
plugin.logDebug(`Executing discovery with PYTHONPATH: ${discoveryPYTHONPATH} and cwd: ${scriptDir}`);
8791
const pythonProcess = spawn(plugin.pythonExecutable!, args, { timeout: discoveryTimeoutMs, cwd: scriptDir, env: env }); // Set working directory to the script's folder // Pass modified environment
@@ -340,40 +344,80 @@ export async function runAllPythonScripts(plugin: ObsidianPythonBridge): Promise
340344
* Called after plugin load, server start, and initial settings sync.
341345
* @param plugin The ObsidianPythonBridge plugin instance.
342346
*/
343-
export function runAutoStartScripts(plugin: ObsidianPythonBridge): void {
347+
export async function runAutoStartScripts(plugin: ObsidianPythonBridge): Promise<void> { // Make async to await loadData
344348
plugin.logInfo("Checking for scripts to run on startup...");
345-
const scriptsFolder = getScriptsFolderPath(plugin);
346-
if (!scriptsFolder) { plugin.logWarn("Cannot run auto-start scripts: Scripts folder path is invalid."); return; }
349+
350+
// Reload settings just before checking to ensure the latest values are used.
351+
// This is a safeguard against potential async state issues.
352+
await plugin.loadSettings();
353+
const currentSettings = plugin.settings; // Use a local copy for the loop
354+
355+
const scriptsFolder = getScriptsFolderPath(plugin); // Uses plugin.settings which was just reloaded
356+
if (!scriptsFolder) {
357+
plugin.logWarn("Cannot run auto-start scripts: Scripts folder path is invalid.");
358+
return;
359+
}
360+
347361
let scriptsRunCount = 0;
348-
for (const relativePath in plugin.settings.scriptAutoStartStatus) {
349-
const shouldAutoStart = plugin.settings.scriptAutoStartStatus[relativePath];
350-
const isScriptActive = plugin.settings.scriptActivationStatus[relativePath] !== false;
351-
if (shouldAutoStart && isScriptActive) {
352-
const absolutePath = path.join(scriptsFolder, relativePath);
353-
try {
354-
if (fs.existsSync(absolutePath) && fs.statSync(absolutePath).isFile()) {
355-
plugin.logInfo(`Auto-starting script: ${relativePath}`);
356-
const delaySeconds = plugin.settings.scriptAutoStartDelay[relativePath] ?? 0;
357-
const delayMs = Math.max(0, delaySeconds) * 1000;
358-
if (delayMs > 0) {
359-
plugin.logInfo(` -> Delaying execution by ${delaySeconds} second(s).`);
360-
setTimeout(() => {
361-
if (plugin.settings.scriptActivationStatus[relativePath] !== false) { // Re-check activation status inside timeout
362-
plugin.logInfo(`Executing delayed auto-start script: ${relativePath}`);
363-
runPythonScript(plugin, absolutePath, "auto-start"); // No await
364-
} else plugin.logWarn(`Skipping delayed auto-start for ${relativePath}: Script was disabled during delay.`);
365-
}, delayMs);
366-
} else runPythonScript(plugin, absolutePath, "auto-start"); // No await
367-
scriptsRunCount++;
368-
} else {
369-
plugin.logWarn(`Skipping auto-start for ${relativePath}: Script file not found at ${absolutePath}.`);
370-
// Consider cleaning up stale auto-start/delay entries here if desired
362+
plugin.logDebug("Current AutoStart Status:", currentSettings.scriptAutoStartStatus);
363+
plugin.logDebug("Current Activation Status:", currentSettings.scriptActivationStatus);
364+
365+
// Iterate over scripts that have an entry in scriptAutoStartStatus
366+
for (const relativePath in currentSettings.scriptAutoStartStatus) {
367+
const shouldAutoStart = currentSettings.scriptAutoStartStatus[relativePath];
368+
// Explicitly check if shouldAutoStart is true
369+
if (shouldAutoStart === true) {
370+
// Then check if the script is active
371+
const isScriptActive = currentSettings.scriptActivationStatus[relativePath] !== false;
372+
plugin.logDebug(`Checking script ${relativePath}: shouldAutoStart=${shouldAutoStart}, isScriptActive=${isScriptActive}`);
373+
374+
if (isScriptActive) {
375+
const absolutePath = path.join(scriptsFolder, relativePath);
376+
try {
377+
if (fs.existsSync(absolutePath) && fs.statSync(absolutePath).isFile()) {
378+
plugin.logInfo(`Auto-starting script: ${relativePath}`);
379+
const delaySeconds = currentSettings.scriptAutoStartDelay[relativePath] ?? 0;
380+
const delayMs = Math.max(0, delaySeconds) * 1000;
381+
382+
if (delayMs > 0) {
383+
plugin.logInfo(` -> Delaying execution by ${delaySeconds} second(s).`);
384+
setTimeout(async () => { // Make callback async to reload settings
385+
// Re-check status just before delayed execution
386+
await plugin.loadSettings(); // Reload again
387+
const latestActivationStatus = plugin.settings.scriptActivationStatus[relativePath] !== false;
388+
const latestAutoStartStatus = plugin.settings.scriptAutoStartStatus[relativePath] === true;
389+
390+
if (latestActivationStatus && latestAutoStartStatus) {
391+
plugin.logInfo(`Executing delayed auto-start script: ${relativePath}`);
392+
runPythonScript(plugin, absolutePath, "auto-start"); // No await needed here
393+
} else {
394+
plugin.logWarn(`Skipping delayed auto-start for ${relativePath}: Script status changed during delay (Active: ${latestActivationStatus}, AutoStart: ${latestAutoStartStatus}).`);
395+
}
396+
}, delayMs);
397+
} else {
398+
// Immediate execution (no need to re-check here as we just loaded settings)
399+
runPythonScript(plugin, absolutePath, "auto-start"); // No await needed here
400+
}
401+
scriptsRunCount++;
402+
} else {
403+
plugin.logWarn(`Skipping auto-start for ${relativePath}: Script file not found at ${absolutePath}.`);
404+
}
405+
} catch (error) {
406+
plugin.logError(`Error checking file status for auto-start script ${absolutePath}:`, error);
371407
}
372-
} catch (error) { plugin.logError(`Error checking file status for auto-start script ${absolutePath}:`, error); }
408+
} else {
409+
plugin.logDebug(`Skipping auto-start for ${relativePath}: Script is not active.`);
410+
}
411+
} else {
412+
plugin.logDebug(`Skipping auto-start for ${relativePath}: Auto-start setting is false.`);
373413
}
414+
} // End for loop
415+
416+
if (scriptsRunCount > 0) {
417+
plugin.logInfo(`Finished launching ${scriptsRunCount} auto-start script(s).`);
418+
} else {
419+
plugin.logInfo("No active scripts configured for auto-start based on current settings.");
374420
}
375-
if (scriptsRunCount > 0) plugin.logInfo(`Finished launching ${scriptsRunCount} auto-start script(s).`);
376-
else plugin.logInfo("No active scripts configured for auto-start.");
377421
}
378422

379423
// --- Dynamic Command Management ---

0 commit comments

Comments
 (0)