Skip to content

Commit 6648383

Browse files
committed
fix(tasks): use OmniJS for task status changes to support inbox tasks
AppleScript's `set completed/dropped` fails on inbox tasks with error -10006. Delegate all task status changes to OmniJS (markComplete, markIncomplete, drop) which works uniformly across all task types. Affects edit_item (complete/drop/incomplete) and remove_item (drop). Project status changes remain in AppleScript (unaffected).
1 parent b101d99 commit 6648383

6 files changed

Lines changed: 497 additions & 65 deletions

File tree

src/omnifocus_mcp/mcp_tools/tasks/edit_item.py

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
)
1010
from ...utils import escape_applescript_string
1111
from ..reorder.move_helper import move_task_to_parent, move_task_to_position
12+
from .status_helper import change_task_status
1213

1314

1415
async def edit_item(
@@ -123,21 +124,15 @@ async def edit_item(
123124
changes.append("planned date")
124125

125126
# Handle task-specific properties
127+
effective_status = None # Will be set if task status change is needed
126128
if item_type == "task":
127-
# Handle status (new_status takes precedence over mark_complete)
129+
# Determine effective status change (new_status takes precedence over mark_complete)
130+
# Status is handled via OmniJS post-edit (works for all task types including inbox)
128131
if new_status is not None:
129-
if new_status == "completed":
130-
modifications.append(f"set completed of {item_var} to true")
131-
changes.append("status (completed)")
132-
elif new_status == "dropped":
133-
modifications.append(f"set dropped of {item_var} to true")
134-
changes.append("status (dropped)")
135-
elif new_status == "incomplete":
136-
modifications.append(f"set completed of {item_var} to false")
137-
modifications.append(f"set dropped of {item_var} to false")
138-
changes.append("status (incomplete)")
132+
effective_status = new_status
133+
changes.append(f"status ({new_status})")
139134
elif mark_complete:
140-
modifications.append(f"set completed of {item_var} to true")
135+
effective_status = "completed"
141136
changes.append("completed")
142137

143138
# Handle tags
@@ -194,8 +189,10 @@ async def edit_item(
194189
# Build result message
195190
changes_str = ", ".join(changes) if changes else "no changes"
196191

197-
# Check if we need post-edit operations (parent change or position change)
198-
needs_task_id = item_type == "task" and (new_parent_id is not None or new_position)
192+
# Check if we need post-edit operations (status, parent, or position change)
193+
needs_task_id = item_type == "task" and (
194+
effective_status is not None or new_parent_id is not None or new_position
195+
)
199196

200197
# For tasks with parent/position change, we need the task ID
201198
if needs_task_id:
@@ -241,7 +238,13 @@ async def edit_item(
241238
task_id = output # The script returned the task ID
242239
result_msg = f"Task updated successfully. Changed: {changes_str}"
243240

244-
# Handle parent change first (if specified)
241+
# Handle status change via OmniJS (works for all task types including inbox)
242+
if effective_status is not None:
243+
success, status_msg = await change_task_status(task_id, effective_status)
244+
if not success:
245+
result_msg += f" (status change failed: {status_msg})"
246+
247+
# Handle parent change (if specified)
245248
if new_parent_id is not None:
246249
success, parent_msg = await move_task_to_parent(task_id, new_parent_id)
247250
if success:

src/omnifocus_mcp/mcp_tools/tasks/remove_item.py

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import asyncio
88

99
from ...applescript_builder import generate_find_clause
10+
from .status_helper import change_task_status
1011

1112

1213
async def remove_item(
@@ -33,43 +34,70 @@ async def remove_item(
3334
if not id and not name:
3435
return "Error: Either 'id' or 'name' must be provided"
3536

36-
# Build the find clause
37-
find_clause = generate_find_clause(item_type, "theItem", id, name)
38-
3937
# Build result message
4038
if id:
4139
result_msg = f"{item_type.capitalize()} dropped successfully (by ID)"
4240
else:
4341
result_msg = f"{item_type.capitalize()} dropped successfully: {name}"
4442

45-
# Drop items instead of deleting - different syntax for tasks vs projects
43+
# Projects: use AppleScript (no inbox issue for projects)
4644
if item_type == "project":
47-
drop_statement = "set status of theItem to dropped status"
48-
else:
49-
drop_statement = "set dropped of theItem to true"
50-
51-
script = f'''
45+
find_clause = generate_find_clause(item_type, "theItem", id, name)
46+
script = f'''
5247
tell application "OmniFocus"
5348
tell default document
5449
{find_clause}
55-
{drop_statement}
50+
set status of theItem to dropped status
5651
end tell
5752
end tell
5853
return "{result_msg}"
5954
'''
55+
proc = await asyncio.create_subprocess_exec(
56+
"osascript",
57+
"-e",
58+
script,
59+
stdout=asyncio.subprocess.PIPE,
60+
stderr=asyncio.subprocess.PIPE,
61+
)
62+
stdout, stderr = await proc.communicate()
63+
64+
if proc.returncode != 0:
65+
return f"Error: {stderr.decode()}"
66+
67+
return stdout.decode().strip()
68+
69+
# Tasks: use OmniJS (handles inbox tasks correctly)
70+
task_id = id
71+
if not task_id:
72+
# Resolve name to ID via AppleScript
73+
find_clause = generate_find_clause(item_type, "theItem", id, name)
74+
script = f"""
75+
tell application "OmniFocus"
76+
tell default document
77+
{find_clause}
78+
return id of theItem
79+
end tell
80+
end tell
81+
"""
82+
proc = await asyncio.create_subprocess_exec(
83+
"osascript",
84+
"-e",
85+
script,
86+
stdout=asyncio.subprocess.PIPE,
87+
stderr=asyncio.subprocess.PIPE,
88+
)
89+
stdout, stderr = await proc.communicate()
90+
91+
if proc.returncode != 0:
92+
return f"Error: {stderr.decode()}"
6093

61-
proc = await asyncio.create_subprocess_exec(
62-
"osascript",
63-
"-e",
64-
script,
65-
stdout=asyncio.subprocess.PIPE,
66-
stderr=asyncio.subprocess.PIPE,
67-
)
68-
stdout, stderr = await proc.communicate()
94+
task_id = stdout.decode().strip()
6995

70-
if proc.returncode != 0:
71-
return f"Error: {stderr.decode()}"
96+
success, msg = await change_task_status(task_id, "dropped")
97+
if success:
98+
return result_msg
99+
else:
100+
return f"Error: {msg}"
72101

73-
return stdout.decode().strip()
74102
except Exception as e:
75103
return f"Error removing {item_type}: {str(e)}"
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
"""Helper for changing task status via OmniJS.
2+
3+
Uses OmniJS instead of AppleScript to handle all task types
4+
including inbox tasks, which don't support AppleScript property setters.
5+
"""
6+
7+
import json
8+
9+
from ..response import omnijs_json_response
10+
11+
12+
async def change_task_status(
13+
task_id: str,
14+
status: str,
15+
) -> tuple[bool, str]:
16+
"""
17+
Change a task's status via OmniJS.
18+
19+
Works for all task types including inbox tasks.
20+
21+
Args:
22+
task_id: ID of the task to modify
23+
status: Target status - 'completed', 'dropped', or 'incomplete'
24+
25+
Returns:
26+
Tuple of (success: bool, message: str)
27+
"""
28+
params = {
29+
"task_id": task_id,
30+
"status": status,
31+
}
32+
33+
result_json = await omnijs_json_response("change_task_status", params)
34+
result = json.loads(result_json)
35+
36+
if result.get("error"):
37+
return False, result["error"]
38+
39+
return result.get("success", False), result.get("message", "Unknown result")
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Change a task's status (completed, dropped, incomplete)
2+
// Works for all task types including inbox tasks.
3+
// Params:
4+
// task_id: ID of the task to change (required)
5+
// status: Target status - "completed", "dropped", or "incomplete" (required)
6+
7+
try {
8+
var taskId = params.task_id;
9+
var status = params.status;
10+
11+
if (!taskId) {
12+
return JSON.stringify({ error: "task_id is required" });
13+
}
14+
if (!status) {
15+
return JSON.stringify({ error: "status is required" });
16+
}
17+
18+
// Find the task
19+
var task = null;
20+
flattenedTasks.forEach(function(t) {
21+
if (t.id && t.id.primaryKey === taskId) {
22+
task = t;
23+
}
24+
});
25+
26+
if (!task) {
27+
return JSON.stringify({ error: "Task not found: " + taskId });
28+
}
29+
30+
// Apply status change
31+
if (status === "completed") {
32+
task.markComplete();
33+
} else if (status === "dropped") {
34+
task.drop(true);
35+
} else if (status === "incomplete") {
36+
task.markIncomplete();
37+
task.drop(false);
38+
} else {
39+
return JSON.stringify({ error: "Invalid status: " + status + ". Must be completed, dropped, or incomplete" });
40+
}
41+
42+
return JSON.stringify({
43+
success: true,
44+
message: "Task status changed to " + status,
45+
taskId: taskId
46+
});
47+
48+
} catch (error) {
49+
return JSON.stringify({
50+
error: "Error changing task status: " + error.toString()
51+
});
52+
}

tests/test_status_helper.py

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
"""Tests for task status change helper."""
2+
3+
from unittest.mock import patch
4+
5+
import pytest
6+
7+
from omnifocus_mcp.mcp_tools.tasks.status_helper import change_task_status
8+
9+
10+
class TestChangeTaskStatus:
11+
"""Tests for change_task_status helper."""
12+
13+
@pytest.fixture
14+
def mock_execute(self):
15+
"""Create a mock for execute_omnijs_with_params."""
16+
with patch("omnifocus_mcp.mcp_tools.response.execute_omnijs_with_params") as mock:
17+
yield mock
18+
19+
@pytest.mark.asyncio
20+
async def test_complete_task(self, mock_execute):
21+
"""Test marking a task as completed."""
22+
mock_execute.return_value = {
23+
"success": True,
24+
"message": "Task status changed to completed",
25+
"taskId": "task-123",
26+
}
27+
28+
success, message = await change_task_status("task-123", "completed")
29+
30+
assert success is True
31+
assert "completed" in message
32+
mock_execute.assert_called_once()
33+
params = mock_execute.call_args[0][1]
34+
assert params["task_id"] == "task-123"
35+
assert params["status"] == "completed"
36+
37+
@pytest.mark.asyncio
38+
async def test_drop_task(self, mock_execute):
39+
"""Test dropping a task."""
40+
mock_execute.return_value = {
41+
"success": True,
42+
"message": "Task status changed to dropped",
43+
"taskId": "task-123",
44+
}
45+
46+
success, message = await change_task_status("task-123", "dropped")
47+
48+
assert success is True
49+
assert "dropped" in message
50+
51+
@pytest.mark.asyncio
52+
async def test_mark_incomplete(self, mock_execute):
53+
"""Test marking a task as incomplete."""
54+
mock_execute.return_value = {
55+
"success": True,
56+
"message": "Task status changed to incomplete",
57+
"taskId": "task-123",
58+
}
59+
60+
success, message = await change_task_status("task-123", "incomplete")
61+
62+
assert success is True
63+
assert "incomplete" in message
64+
65+
@pytest.mark.asyncio
66+
async def test_task_not_found(self, mock_execute):
67+
"""Test error when task is not found."""
68+
mock_execute.return_value = {"error": "Task not found: bad-id"}
69+
70+
success, message = await change_task_status("bad-id", "completed")
71+
72+
assert success is False
73+
assert "not found" in message
74+
75+
@pytest.mark.asyncio
76+
async def test_invalid_status(self, mock_execute):
77+
"""Test error for invalid status value."""
78+
mock_execute.return_value = {"error": "Invalid status: bogus"}
79+
80+
success, message = await change_task_status("task-123", "bogus")
81+
82+
assert success is False
83+
assert "Invalid status" in message
84+
85+
@pytest.mark.asyncio
86+
async def test_script_name(self, mock_execute):
87+
"""Test that the correct script name is used."""
88+
mock_execute.return_value = {
89+
"success": True,
90+
"message": "Task status changed to completed",
91+
"taskId": "task-123",
92+
}
93+
94+
await change_task_status("task-123", "completed")
95+
96+
script_name = mock_execute.call_args[0][0]
97+
assert script_name == "change_task_status"

0 commit comments

Comments
 (0)