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 <noreply@anthropic.com>
This commit is contained in:
Krzysztof Rudnicki 2026-11-09 18:24:24 +01:00
parent ba25e8f176
commit 135f3f07ea
4 changed files with 150 additions and 21 deletions

View File

@ -17,7 +17,7 @@ class TaskService {
final BoardClient? _board; final BoardClient? _board;
Future<void> _tail = Future.value(); Future<void> _tail = Future.value();
Future<String> _serial(Future<String> Function() body) { Future<T> _serial<T>(Future<T> Function() body) {
final run = _tail.then((_) => body()); final run = _tail.then((_) => body());
_tail = run.then((_) {}, onError: (_) {}); _tail = run.then((_) {}, onError: (_) {});
return run; return run;
@ -44,8 +44,14 @@ class TaskService {
} }
} }
/// `add_task`: joins the repo's open note (by: claude), or creates it. /// `add_task`: joins the repo's open note (by: claude), or creates it. A
Future<String> add(Map<String, dynamic> args) => _serial(() async { /// board Start after a merge runs once the other calls may go again.
Future<String> add(Map<String, dynamic> args) async {
final (:reply, :start) = await _serial(() => _add(args));
return start == null ? reply : '$reply\n${await start()}';
}
Future<SplitReply> _add(Map<String, dynamic> args) async {
final a = _Args(args); final a = _Args(args);
final repo = a.repo(required: true)!; final repo = a.repo(required: true)!;
final title = a.required('title'); final title = a.required('title');
@ -71,8 +77,9 @@ class TaskService {
priority: a.priority() ?? Priority.medium, priority: a.priority() ?? Priority.medium,
by: 'claude', 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. /// `list_tasks`: repo tasks, open ones unless `status` says otherwise.
Future<String> list(Map<String, dynamic> args) => _serial(() async { Future<String> list(Map<String, dynamic> args) => _serial(() async {

View File

@ -10,9 +10,14 @@ import 'package:todo/mcp/board_client.dart';
/// Saves [note] and returns `<verb> <id>` plus its list line. /// Saves [note] and returns `<verb> <id>` plus its list line.
typedef SaveNote = Future<String> Function(Note note, String verb); typedef SaveNote = Future<String> Function(Note note, String verb);
/// Adds a separate task, merging into a duplicate's note when it is idle, /// A saved split add: its reply so far, and the board Start still to run.
/// and clicks Start on the board after a merge that may run. /// The caller runs [start] after releasing its lock, since the board learns
Future<String> addSplit( /// about the merged section by calling this same server (T1865).
typedef SplitReply = ({String reply, Future<String> 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<SplitReply> addSplit(
NoteRepository repository, NoteRepository repository,
BoardClient? board, BoardClient? board,
SaveNote save, { SaveNote save, {
@ -34,19 +39,28 @@ Future<String> addSplit(
); );
if (added.kind != SplitKind.merged) { if (added.kind != SplitKind.merged) {
final reply = await save(added.note, 'added'); 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 reply = await save(added.note, 'merged into');
final block = startBlock(doneCheck, body); final block = startBlock(doneCheck, body);
if (block != null) return '$reply\nstart: skipped ($block)'; if (block != null) {
final start = board == null return (reply: '$reply\nstart: skipped ($block)', start: null);
? (ok: false, reason: 'no board configured', item: null) }
: await board.start( Future<String> start() async {
added.note.id, final s = board == null
title.trim(), ? (ok: false, reason: 'no board configured', item: null)
sections: sectionCount(added.note.text), : await board.start(
); added.note.id,
if (start.ok) return '$reply\nstart: ok ${start.item}'; title.trim(),
return '$reply\nstart: {ok: false, reason: "${start.reason}"}; ' sections: sectionCount(added.note.text),
'the merge is saved, click Start on the board by hand'; );
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);
} }

View File

@ -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 # 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 # 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. # 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: environment:
sdk: ^3.12.2 sdk: ^3.12.2

View File

@ -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 = <Map<String, dynamic>>[];
final posts = <String>[];
// 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<List<Map<String, dynamic>>> 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('<meta name="todoimpl-csrf" content="t">', 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'));
});
}