Skip to main content

max / goingson

Give recurrence a real schema, so the rich rule is reachable The `recurrence` fragment carried a description and no `type`. An untyped property is serialised as a string, so `{"pattern":"Monthly","interval":12}` arrived as one long word, took `parse_recurrence`'s `as_str` branch and failed against the four-word enum. The object branch was unreachable from every MCP client, and every recurring task or event filed since the rich rule landed was quietly degraded to a bare word. The fragment now declares `type: ["string","object"]` and a `oneOf` with a typed object arm, and lives once in tools/mod.rs rather than as two copies that had drifted. Both the task and event surfaces take it from there, so update_event is covered through event_fields(). `parse_recurrence` also re-parses a string that is not one of the four words, for a client whose validator drops the object arm: failing there reported the whole object as an unknown pattern, which points the caller at the wrong thing.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-21 23:14 UTC
Signed with PGP, not checked
Commit: 44d489d72e0629c98e2441fb16f1a5845c6f3ecf
Parent: 20198b4
5 files changed, +198 insertions, -17 deletions
M crates/go-mcp/src/convert.rs +19 -4
@@ -205,10 +205,25 @@
205 205 return Ok((Recurrence::None, None));
206 206 }
207 207
208 - if let Some(word) = raw.as_str() {
209 - let pattern: Recurrence = parse_enum(tool, "recurrence", word, RECURRENCE_PATTERNS)?;
210 - return Ok((pattern, None));
211 - }
208 + // A client whose schema validator does not carry the object arm sends the
209 + // rich rule stringified. Re-parse rather than failing with a message that
210 + // points at the enum: the whole object would appear where a word belongs,
211 + // which is exactly the wrong thing to show the caller.
212 + let reparsed;
213 + let raw = match raw.as_str() {
214 + Some(word) => match serde_json::from_str::<Value>(word) {
215 + Ok(v) if v.is_object() => {
216 + reparsed = v;
217 + &reparsed
218 + }
219 + _ => {
220 + let pattern: Recurrence =
221 + parse_enum(tool, "recurrence", word, RECURRENCE_PATTERNS)?;
222 + return Ok((pattern, None));
223 + }
224 + },
225 + None => raw,
226 + };
212 227
213 228 let obj = raw.as_object().ok_or_else(|| {
214 229 invalid(format!(
M crates/go-mcp/tests/e2e.rs +120
@@ -257,6 +257,126 @@
257 257 assert!(done["next_task"].is_null(), "a cleared rule must not recur");
258 258 }
259 259
260 + #[tokio::test]
261 + async fn every_recurrence_schema_declares_the_object_arm() {
262 + let reg = tools::registry(Arc::new(Ctx::new(seed_db())));
263 +
264 + // The bug this guards: the fragment carried a `description` and no `type`,
265 + // so a client serialised the rich rule as a string and the object branch of
266 + // `parse_recurrence` was unreachable from every MCP client.
267 + for tool in [
268 + "create_task",
269 + "bulk_import_tasks",
270 + "update_task",
271 + "create_event",
272 + "bulk_import_events",
273 + "update_event",
274 + ] {
275 + let schema = reg
276 + .get(tool)
277 + .unwrap_or_else(|| panic!("{tool} registered"))
278 + .input_schema();
279 + let text = schema.to_string();
280 + let field = find_recurrence(&schema)
281 + .unwrap_or_else(|| panic!("{tool} declares a recurrence field, schema was {text}"));
282 +
283 + assert!(
284 + field.get("type").is_some(),
285 + "{tool}'s recurrence needs a type, or the object arm is unreachable"
286 + );
287 + let arms = field["oneOf"]
288 + .as_array()
289 + .unwrap_or_else(|| panic!("{tool}'s recurrence needs a oneOf"));
290 + let object_arm = arms
291 + .iter()
292 + .find(|a| a["type"] == "object")
293 + .unwrap_or_else(|| panic!("{tool}'s recurrence needs an object arm"));
294 + assert_eq!(
295 + object_arm["properties"]["interval"]["type"], "integer",
296 + "{tool}'s interval must be typed"
297 + );
298 + assert_eq!(object_arm["properties"]["pattern"]["type"], "string");
299 + }
300 + }
301 +
302 + /// The `recurrence` subschema, wherever a tool nests it (the bulk tools put
303 + /// their item fields one level down).
304 + fn find_recurrence(schema: &Value) -> Option<&Value> {
305 + match schema {
306 + Value::Object(map) => {
307 + if let Some(found) = map.get("recurrence") {
308 + return Some(found);
309 + }
310 + map.values().find_map(find_recurrence)
311 + }
312 + Value::Array(items) => items.iter().find_map(find_recurrence),
313 + _ => None,
314 + }
315 + }
316 +
317 + #[tokio::test]
318 + async fn an_annual_rule_round_trips_through_the_wire_shape() {
319 + let reg = tools::registry(Arc::new(Ctx::new(seed_db())));
320 +
321 + // There is no `Yearly` word. Annual is Monthly every twelfth month, and it
322 + // is only reachable if the object arrives as an object.
323 + let created = call(
324 + &reg,
325 + "create_task",
326 + json!({
327 + "description": "re-check the payments landscape",
328 + "due": "2028-01-15",
329 + "recurrence": { "pattern": "Monthly", "interval": 12 },
330 + }),
331 + )
332 + .await;
333 + let id = created["id"].as_str().unwrap().to_string();
334 +
335 + let got = call(&reg, "get_task", json!({ "id": id })).await;
336 + assert_eq!(got["recurrence"]["pattern"], "Monthly");
337 + assert_eq!(got["recurrence"]["interval"], 12);
338 +
339 + // And through update, which takes the same fragment from the same helper.
340 + let updated = call(
341 + &reg,
342 + "update_task",
343 + json!({ "id": id, "recurrence": { "pattern": "Monthly", "interval": 6 } }),
344 + )
345 + .await;
346 + assert_eq!(updated["recurrence"]["interval"], 6);
347 + }
348 +
349 + #[tokio::test]
350 + async fn a_stringified_rule_object_is_reparsed_rather_than_refused() {
351 + let reg = tools::registry(Arc::new(Ctx::new(seed_db())));
352 +
353 + // Belt and braces for a client whose validator drops the object arm: the
354 + // whole rule arrives as one string. Failing here would report the object
355 + // as an unknown pattern word, which points the caller at the wrong thing.
356 + let created = call(
357 + &reg,
358 + "create_task",
359 + json!({
360 + "description": "quarterly review",
361 + "recurrence": "{\"pattern\":\"Monthly\",\"interval\":3}",
362 + }),
363 + )
364 + .await;
365 + let got = call(&reg, "get_task", json!({ "id": created["id"] })).await;
366 + assert_eq!(got["recurrence"]["pattern"], "Monthly");
367 + assert_eq!(got["recurrence"]["interval"], 3);
368 +
369 + // A word is still a word, and a string that is neither still fails as one.
370 + let plain = call(
371 + &reg,
372 + "create_task",
373 + json!({ "description": "standup", "recurrence": "Daily" }),
374 + )
375 + .await;
376 + let got = call(&reg, "get_task", json!({ "id": plain["id"] })).await;
377 + assert_eq!(got["recurrence"]["pattern"], "Daily");
378 + }
379 +
260 380 #[tokio::test]
261 381 async fn recurrence_rejects_a_bad_rule_instead_of_defaulting() {
262 382 let reg = tools::registry(Arc::new(Ctx::new(seed_db())));
M crates/go-mcp/src/tools/event.rs +2 -4
@@ -44,7 +44,7 @@
44 44 use serde_json::{Value, json};
45 45
46 46 use super::contact::{ContactCache, contact_arg, contact_fields};
47 - use super::{project_id_arg, project_id_field};
47 + use super::{project_id_arg, project_id_field, recurrence_field};
48 48 use crate::caps;
49 49 use crate::context::Ctx;
50 50 use crate::convert::{
@@ -424,9 +424,7 @@
424 424 "description": { "type": "string" },
425 425 "project_id": project_id_field(),
426 426 "location": { "type": "string" },
427 - "recurrence": {
428 - "description": "Either a word (None|Daily|Weekly|Monthly) or an object {pattern, interval?, weekdays?, monthly_spec?, until?}. weekdays are 0=Mon..6=Sun. monthly_spec is {type: dayOfMonth, day} or {type: nthWeekday, week, weekday} (week -1 = last). until is RFC 3339 and ends the series inclusively; omit it to repeat forever."
429 - },
427 + "recurrence": recurrence_field(),
430 428 "tz_kind": {
431 429 "type": "string",
432 430 "description": "relative (a wall clock that follows the user, e.g. a daily routine) | local (a wall clock anchored to `timezone`, correct across that zone's DST) | absolute (a fixed instant). Defaults to absolute."
M crates/go-mcp/src/tools/mod.rs +56
@@ -151,3 +151,59 @@
151 151 "description": "Project id (UUID) from list_projects. Names are labels and can be renamed, so they are not accepted here."
152 152 })
153 153 }
154 +
155 + /// The `recurrence` schema fragment, worded once for the task and event write
156 + /// surfaces, which store the identical rule type.
157 + ///
158 + /// The object arm has to declare its own types. An untyped property is handed
159 + /// to the server as a string, so a client sending the rich rule stringified the
160 + /// whole object and `parse_recurrence` never reached its object branch.
161 + pub(super) fn recurrence_field() -> Value {
162 + json!({
163 + // The union is stated twice on purpose. `oneOf` is the precise form,
164 + // and a validator that only reads `type` still learns that an object
165 + // is allowed rather than defaulting the value to a string.
166 + "type": ["string", "object"],
167 + "description": "Either a word (None|Daily|Weekly|Monthly) or an object {pattern, interval?, weekdays?, monthly_spec?, until?}. weekdays are 0=Mon..6=Sun. monthly_spec is {type: dayOfMonth, day} or {type: nthWeekday, week, weekday} (week -1 = last). until is RFC 3339 and ends the series inclusively; omit it to repeat forever. Completing a recurring task opens its successor rather than editing it, and the instance landing on until opens none. An annual rule is {pattern: Monthly, interval: 12}; there is no Yearly word.",
168 + "oneOf": [
169 + {
170 + "type": "string",
171 + "enum": ["None", "Daily", "Weekly", "Monthly"]
172 + },
173 + {
174 + "type": "object",
175 + "required": ["pattern"],
176 + "properties": {
177 + "pattern": {
178 + "type": "string",
179 + "enum": ["None", "Daily", "Weekly", "Monthly"]
180 + },
181 + "interval": {
182 + "type": "integer",
183 + "minimum": 1,
184 + "description": "Repeat every N patterns. Defaults to 1; interval 12 on Monthly is annual."
185 + },
186 + "weekdays": {
187 + "type": "array",
188 + "items": { "type": "integer", "minimum": 0, "maximum": 6 },
189 + "description": "0=Mon .. 6=Sun."
190 + },
191 + "monthly_spec": {
192 + "type": "object",
193 + "description": "{type: dayOfMonth, day} or {type: nthWeekday, week, weekday}; week -1 is the last.",
194 + "properties": {
195 + "type": { "type": "string", "enum": ["dayOfMonth", "day_of_month", "nthWeekday", "nth_weekday"] },
196 + "day": { "type": "integer", "minimum": 1, "maximum": 31 },
197 + "week": { "type": "integer", "minimum": -1, "maximum": 5 },
198 + "weekday": { "type": "integer", "minimum": 0, "maximum": 6 }
199 + }
200 + },
201 + "until": {
202 + "type": "string",
203 + "description": "RFC 3339; the last instant the series may land on."
204 + }
205 + }
206 + }
207 + ]
208 + })
209 + }
M crates/go-mcp/src/tools/task.rs +1 -9
@@ -22,7 +22,7 @@
22 22 use kberg::{Error, Result, Tool, ToolCallResult, ToolKind};
23 23 use serde_json::{Value, json};
24 24
25 - use super::{project_id_arg, project_id_field};
25 + use super::{project_id_arg, project_id_field, recurrence_field};
26 26 use crate::caps;
27 27 use crate::context::Ctx;
28 28 use crate::convert::{
@@ -31,14 +31,6 @@
31 31 subtask_row, task_row, task_summary_row,
32 32 };
33 33
34 - /// The `recurrence` schema fragment, identical across the task write tools and
35 - /// worded the same as the event surface's, since it is the same rule type.
36 - fn recurrence_field() -> Value {
37 - json!({
38 - "description": "Either a word (None|Daily|Weekly|Monthly) or an object {pattern, interval?, weekdays?, monthly_spec?, until?}. weekdays are 0=Mon..6=Sun. monthly_spec is {type: dayOfMonth, day} or {type: nthWeekday, week, weekday} (week -1 = last). until is RFC 3339 and ends the series inclusively; omit it to repeat forever. Completing a recurring task opens its successor rather than editing it, and the instance landing on until opens none."
39 - })
40 - }
41 -
42 34 fn fail(tool: &str, e: impl std::fmt::Display) -> Error {
43 35 Error::ToolFailed {
44 36 tool: tool.to_string(),