From 135f3f07eac4d91b81f8974a524fdb09a0802e28 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Mon, 9 Nov 2026 18:24:24 +0100 Subject: [PATCH] Start a merged task outside the MCP lock The board refreshes its notes through this MCP server, and add_task held the server's serial lock while waiting for the board to show the merged section, so the board could never see it (T1865). The Start now runs after the lock is released. Todo: 64d39ab5 Co-Authored-By: Claude Opus 5.5 --- lib/mcp/task_service.dart | 17 +++-- lib/mcp/task_split.dart | 44 +++++++---- pubspec.yaml | 2 +- test/mcp/mcp_board_refresh_test.dart | 108 +++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 21 deletions(-) create mode 100644 test/mcp/mcp_board_refresh_test.dart diff --git a/lib/mcp/task_service.dart b/lib/mcp/task_service.dart index fe965ac..8f23e0f 100644 --- a/lib/mcp/task_service.dart +++ b/lib/mcp/task_service.dart @@ -17,7 +17,7 @@ class TaskService { final BoardClient? _board; Future _tail = Future.value(); - Future _serial(Future Function() body) { + Future _serial(Future Function() body) { final run = _tail.then((_) => body()); _tail = run.then((_) {}, onError: (_) {}); return run; @@ -44,8 +44,14 @@ class TaskService { } } - /// `add_task`: joins the repo's open note (by: claude), or creates it. - Future add(Map args) => _serial(() async { + /// `add_task`: joins the repo's open note (by: claude), or creates it. A + /// board Start after a merge runs once the other calls may go again. + Future add(Map args) async { + final (:reply, :start) = await _serial(() => _add(args)); + return start == null ? reply : '$reply\n${await start()}'; + } + + Future _add(Map args) async { final a = _Args(args); final repo = a.repo(required: true)!; final title = a.required('title'); @@ -71,8 +77,9 @@ class TaskService { priority: a.priority() ?? Priority.medium, by: 'claude', ); - return await _save(added.note, added.appended ? 'appended to' : 'added'); - }); + final verb = added.appended ? 'appended to' : 'added'; + return (reply: await _save(added.note, verb), start: null); + } /// `list_tasks`: repo tasks, open ones unless `status` says otherwise. Future list(Map args) => _serial(() async { diff --git a/lib/mcp/task_split.dart b/lib/mcp/task_split.dart index 8b88b55..9cfef54 100644 --- a/lib/mcp/task_split.dart +++ b/lib/mcp/task_split.dart @@ -10,9 +10,14 @@ import 'package:todo/mcp/board_client.dart'; /// Saves [note] and returns ` ` plus its list line. typedef SaveNote = Future Function(Note note, String verb); -/// Adds a separate task, merging into a duplicate's note when it is idle, -/// and clicks Start on the board after a merge that may run. -Future addSplit( +/// A saved split add: its reply so far, and the board Start still to run. +/// The caller runs [start] after releasing its lock, since the board learns +/// about the merged section by calling this same server (T1865). +typedef SplitReply = ({String reply, Future Function()? start}); + +/// Adds a separate task, merging into a duplicate's note when it is idle; +/// a merge that may run comes back with the Start to click on the board. +Future addSplit( NoteRepository repository, BoardClient? board, SaveNote save, { @@ -34,19 +39,28 @@ Future addSplit( ); if (added.kind != SplitKind.merged) { final reply = await save(added.note, 'added'); - return added.why == null ? reply : '$reply\n${added.why}'; + return ( + reply: added.why == null ? reply : '$reply\n${added.why}', + start: null, + ); } final reply = await save(added.note, 'merged into'); final block = startBlock(doneCheck, body); - if (block != null) return '$reply\nstart: skipped ($block)'; - final start = board == null - ? (ok: false, reason: 'no board configured', item: null) - : await board.start( - added.note.id, - title.trim(), - sections: sectionCount(added.note.text), - ); - if (start.ok) return '$reply\nstart: ok ${start.item}'; - return '$reply\nstart: {ok: false, reason: "${start.reason}"}; ' - 'the merge is saved, click Start on the board by hand'; + if (block != null) { + return (reply: '$reply\nstart: skipped ($block)', start: null); + } + Future start() async { + final s = board == null + ? (ok: false, reason: 'no board configured', item: null) + : await board.start( + added.note.id, + title.trim(), + sections: sectionCount(added.note.text), + ); + if (s.ok) return 'start: ok ${s.item}'; + return 'start: {ok: false, reason: "${s.reason}"}; ' + 'the merge is saved, click Start on the board by hand'; + } + + return (reply: reply, start: start); } diff --git a/pubspec.yaml b/pubspec.yaml index e24c481..60ceda1 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html # In Windows, build-name is used as the major, minor, and patch parts # of the product and file versions while build-number is used as the build suffix. -version: 1.0.63+1 +version: 1.0.64+1 environment: sdk: ^3.12.2 diff --git a/test/mcp/mcp_board_refresh_test.dart b/test/mcp/mcp_board_refresh_test.dart new file mode 100644 index 0000000..ac505b4 --- /dev/null +++ b/test/mcp/mcp_board_refresh_test.dart @@ -0,0 +1,108 @@ +// T1865: the real board learns about notes by calling this MCP server, so a +// merged add_task must not hold the server while it waits for the board. +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:mcp_dart/mcp_dart.dart'; +import 'package:todo/mcp/board_client.dart'; + +import 'mcp_harness.dart'; + +void main() { + late McpHarness h; + late McpClient c; + late McpClient boardSide; + String? note; + var cards = >[]; + final posts = []; + + // A board whose every read refreshes its cards from this MCP server, as + // the real one does; a refresh that cannot get through keeps stale cards. + Future>> refresh() async { + final id = note; + if (id == null) return cards; + try { + final text = await h + .call(boardSide, 'get_task', {'id': id}) + .timeout(const Duration(milliseconds: 300)); + final n = '\n$text'.split('\n## ').length - 1; + cards = [ + for (var i = 0; i < n; i++) + { + 'id': '${id.substring(0, 8)}-$i', + 'title': 'Młody', + 'column': 'backlog', + 'actions': [ + {'id': 'start', 'enabled': true}, + ], + }, + ]; + } on TimeoutException { + // Stale, like the real cache when the server does not answer. + } + return cards; + } + + final board = BoardClient( + wait: const Duration(seconds: 2), + poll: const Duration(milliseconds: 20), + client: MockClient((req) async { + if (req.method == 'POST') { + posts.add(req.url.path); + return http.Response('{"ok":true}', 200); + } + if (req.url.path == '/api/board') { + return http.Response.bytes( + utf8.encode(jsonEncode({'tasks': await refresh()})), + 200, + ); + } + return http.Response('', 200); + }), + ); + + setUp(() async { + note = null; + cards = []; + posts.clear(); + h = McpHarness(board: board); + await h.start(); + c = await h.connect(); + boardSide = await h.connect(); + }); + tearDown(() => h.dispose()); + + test('T1865: Start after a merge sees the board refreshed', () async { + final args = { + 'repo': 'kuhyx/herosi', + 'title': 'Młody', + 'body': 'what', + 'new_note': true, + 'done_check': 'true', + }; + note = idIn(await h.call(c, 'add_task', args)); + final r = await h.call(c, 'add_task', args); + expect(r, startsWith('merged into $note')); + expect(r, contains('start: ok ${note!.substring(0, 8)}-1')); + expect(posts, ['/api/task/${note!.substring(0, 8)}-1/start']); + }); + + test('a merge with no board configured says click Start', () async { + final bare = McpHarness(); + await bare.start(); + addTearDown(bare.dispose); + final client = await bare.connect(); + final args = {'repo': 'kuhyx/herosi', 'title': 'A', 'new_note': true}; + final x = idIn(await bare.call(client, 'add_task', args)); + final r = await bare.call(client, 'add_task', { + ...args, + 'done_check': 'true', + }); + expect(r, startsWith('merged into $x')); + expect(r, contains('reason: "no board configured"')); + expect(r, contains('click Start on the board by hand')); + }); +}