Skip to content

Commit 3670b80

Browse files
devwhodevsdevwhodevs
authored
Merge pull request #38 from brndnblck/fix/edit-frontmatter-schema
fix(mcp): type edit_frontmatter operations for OpenAI schema compatibility
2 parents ab45232 + 1bdd8be commit 3670b80

2 files changed

Lines changed: 176 additions & 145 deletions

File tree

‎src/http.rs‎

Lines changed: 39 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::health;
2020
use crate::llm::{EmbedModel, OrchestratorModel, RerankModel};
2121
use crate::profile::VaultProfile;
2222
use crate::search;
23-
use crate::serve::RecentWrites;
23+
use crate::serve::{FrontmatterOpInput, FrontmatterOpKind, RecentWrites};
2424
use crate::store::Store;
2525
use crate::writer::{
2626
self, AppendInput, CreateNoteInput, DeleteMode, EditFrontmatterInput, EditInput, EditMode,
@@ -295,7 +295,7 @@ struct RewriteBody {
295295
#[derive(Debug, Deserialize)]
296296
struct EditFrontmatterBody {
297297
file: String,
298-
operations: Vec<serde_json::Value>,
298+
operations: Vec<FrontmatterOpInput>,
299299
}
300300

301301
#[derive(Debug, Deserialize)]
@@ -633,76 +633,54 @@ async fn record_write(recent_writes: &RecentWrites, path: &std::path::Path) {
633633
}
634634
}
635635

636-
/// Parse a JSON operations array into `Vec<FrontmatterOp>`.
637-
fn parse_frontmatter_ops(operations: &[serde_json::Value]) -> Result<Vec<FrontmatterOp>, ApiError> {
636+
/// Convert typed operation inputs into `Vec<FrontmatterOp>`.
637+
fn parse_frontmatter_ops(
638+
operations: &[FrontmatterOpInput],
639+
) -> Result<Vec<FrontmatterOp>, ApiError> {
638640
let mut ops = Vec::with_capacity(operations.len());
639-
for op_val in operations {
640-
let op_str = op_val.get("op").and_then(|v| v.as_str()).ok_or_else(|| {
641-
ApiError::bad_request("each operation must have an \"op\" string field")
642-
})?;
643-
match op_str {
644-
"set" => {
645-
let key = op_val.get("key").and_then(|v| v.as_str()).ok_or_else(|| {
641+
for input in operations {
642+
let op = match input.op {
643+
FrontmatterOpKind::Set => {
644+
let key = input.key.as_deref().ok_or_else(|| {
646645
ApiError::bad_request("\"set\" operation requires a \"key\" field")
647646
})?;
648-
let value = op_val
649-
.get("value")
650-
.and_then(|v| v.as_str())
651-
.ok_or_else(|| {
652-
ApiError::bad_request("\"set\" operation requires a \"value\" field")
653-
})?;
654-
ops.push(FrontmatterOp::Set(key.to_string(), value.to_string()));
647+
let value = input.value.as_deref().ok_or_else(|| {
648+
ApiError::bad_request("\"set\" operation requires a \"value\" field")
649+
})?;
650+
FrontmatterOp::Set(key.to_string(), value.to_string())
655651
}
656-
"remove" => {
657-
let key = op_val.get("key").and_then(|v| v.as_str()).ok_or_else(|| {
652+
FrontmatterOpKind::Remove => {
653+
let key = input.key.as_deref().ok_or_else(|| {
658654
ApiError::bad_request("\"remove\" operation requires a \"key\" field")
659655
})?;
660-
ops.push(FrontmatterOp::Remove(key.to_string()));
661-
}
662-
"add_tag" => {
663-
let value = op_val
664-
.get("value")
665-
.and_then(|v| v.as_str())
666-
.ok_or_else(|| {
667-
ApiError::bad_request("\"add_tag\" operation requires a \"value\" field")
668-
})?;
669-
ops.push(FrontmatterOp::AddTag(value.to_string()));
656+
FrontmatterOp::Remove(key.to_string())
670657
}
671-
"remove_tag" => {
672-
let value = op_val
673-
.get("value")
674-
.and_then(|v| v.as_str())
675-
.ok_or_else(|| {
676-
ApiError::bad_request("\"remove_tag\" operation requires a \"value\" field")
677-
})?;
678-
ops.push(FrontmatterOp::RemoveTag(value.to_string()));
658+
FrontmatterOpKind::AddTag => {
659+
let value = input.value.as_deref().ok_or_else(|| {
660+
ApiError::bad_request("\"add_tag\" operation requires a \"value\" field")
661+
})?;
662+
FrontmatterOp::AddTag(value.to_string())
679663
}
680-
"add_alias" => {
681-
let value = op_val
682-
.get("value")
683-
.and_then(|v| v.as_str())
684-
.ok_or_else(|| {
685-
ApiError::bad_request("\"add_alias\" operation requires a \"value\" field")
686-
})?;
687-
ops.push(FrontmatterOp::AddAlias(value.to_string()));
664+
FrontmatterOpKind::RemoveTag => {
665+
let value = input.value.as_deref().ok_or_else(|| {
666+
ApiError::bad_request("\"remove_tag\" operation requires a \"value\" field")
667+
})?;
668+
FrontmatterOp::RemoveTag(value.to_string())
688669
}
689-
"remove_alias" => {
690-
let value = op_val
691-
.get("value")
692-
.and_then(|v| v.as_str())
693-
.ok_or_else(|| {
694-
ApiError::bad_request(
695-
"\"remove_alias\" operation requires a \"value\" field",
696-
)
697-
})?;
698-
ops.push(FrontmatterOp::RemoveAlias(value.to_string()));
670+
FrontmatterOpKind::AddAlias => {
671+
let value = input.value.as_deref().ok_or_else(|| {
672+
ApiError::bad_request("\"add_alias\" operation requires a \"value\" field")
673+
})?;
674+
FrontmatterOp::AddAlias(value.to_string())
699675
}
700-
unknown => {
701-
return Err(ApiError::bad_request(&format!(
702-
"unknown frontmatter operation: \"{unknown}\""
703-
)));
676+
FrontmatterOpKind::RemoveAlias => {
677+
let value = input.value.as_deref().ok_or_else(|| {
678+
ApiError::bad_request("\"remove_alias\" operation requires a \"value\" field")
679+
})?;
680+
FrontmatterOp::RemoveAlias(value.to_string())
704681
}
705-
}
682+
};
683+
ops.push(op);
706684
}
707685
Ok(ops)
708686
}

‎src/serve.rs‎

Lines changed: 137 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,35 @@ pub struct RewriteParams {
170170
pub preserve_frontmatter: Option<bool>,
171171
}
172172

173+
/// Operation kind for frontmatter edits.
174+
#[derive(Debug, Clone, Copy, Deserialize, JsonSchema)]
175+
#[serde(rename_all = "snake_case")]
176+
pub enum FrontmatterOpKind {
177+
Set,
178+
Remove,
179+
AddTag,
180+
RemoveTag,
181+
AddAlias,
182+
RemoveAlias,
183+
}
184+
185+
/// A single frontmatter operation.
186+
#[derive(Debug, Clone, Deserialize, JsonSchema)]
187+
pub struct FrontmatterOpInput {
188+
/// Operation type.
189+
pub op: FrontmatterOpKind,
190+
/// Property key (required for "set" and "remove").
191+
pub key: Option<String>,
192+
/// Value (required for "set", "add_tag", "remove_tag", "add_alias", "remove_alias").
193+
pub value: Option<String>,
194+
}
195+
173196
#[derive(Debug, Deserialize, JsonSchema)]
174197
pub struct EditFrontmatterParams {
175198
/// Target note: file path, basename, or #docid.
176199
pub file: String,
177200
/// Operations to apply. Array of objects like {"op": "add_tag", "value": "rust"} or {"op": "set", "key": "status", "value": "done"} or {"op": "remove", "key": "status"} or {"op": "remove_tag", "value": "old"}.
178-
pub operations: Vec<serde_json::Value>,
201+
pub operations: Vec<FrontmatterOpInput>,
179202
}
180203

181204
#[derive(Debug, Deserialize, JsonSchema)]
@@ -266,108 +289,82 @@ async fn record_write(recent_writes: &RecentWrites, path: &Path) {
266289
}
267290
}
268291

269-
/// Parse a JSON operations array into `Vec<FrontmatterOp>`.
270-
fn parse_frontmatter_ops(operations: &[serde_json::Value]) -> Result<Vec<FrontmatterOp>, McpError> {
292+
/// Convert typed operation inputs into `Vec<FrontmatterOp>`.
293+
fn parse_frontmatter_ops(
294+
operations: &[FrontmatterOpInput],
295+
) -> Result<Vec<FrontmatterOp>, McpError> {
271296
let mut ops = Vec::with_capacity(operations.len());
272-
for op_val in operations {
273-
let op_str = op_val.get("op").and_then(|v| v.as_str()).ok_or_else(|| {
274-
McpError::new(
275-
rmcp::model::ErrorCode::INVALID_PARAMS,
276-
"each operation must have an \"op\" string field",
277-
None::<serde_json::Value>,
278-
)
279-
})?;
280-
match op_str {
281-
"set" => {
282-
let key = op_val.get("key").and_then(|v| v.as_str()).ok_or_else(|| {
297+
for input in operations {
298+
let op = match input.op {
299+
FrontmatterOpKind::Set => {
300+
let key = input.key.as_deref().ok_or_else(|| {
283301
McpError::new(
284302
rmcp::model::ErrorCode::INVALID_PARAMS,
285303
"\"set\" operation requires a \"key\" field",
286304
None::<serde_json::Value>,
287305
)
288306
})?;
289-
let value = op_val
290-
.get("value")
291-
.and_then(|v| v.as_str())
292-
.ok_or_else(|| {
293-
McpError::new(
294-
rmcp::model::ErrorCode::INVALID_PARAMS,
295-
"\"set\" operation requires a \"value\" field",
296-
None::<serde_json::Value>,
297-
)
298-
})?;
299-
ops.push(FrontmatterOp::Set(key.to_string(), value.to_string()));
307+
let value = input.value.as_deref().ok_or_else(|| {
308+
McpError::new(
309+
rmcp::model::ErrorCode::INVALID_PARAMS,
310+
"\"set\" operation requires a \"value\" field",
311+
None::<serde_json::Value>,
312+
)
313+
})?;
314+
FrontmatterOp::Set(key.to_string(), value.to_string())
300315
}
301-
"remove" => {
302-
let key = op_val.get("key").and_then(|v| v.as_str()).ok_or_else(|| {
316+
FrontmatterOpKind::Remove => {
317+
let key = input.key.as_deref().ok_or_else(|| {
303318
McpError::new(
304319
rmcp::model::ErrorCode::INVALID_PARAMS,
305320
"\"remove\" operation requires a \"key\" field",
306321
None::<serde_json::Value>,
307322
)
308323
})?;
309-
ops.push(FrontmatterOp::Remove(key.to_string()));
310-
}
311-
"add_tag" => {
312-
let value = op_val
313-
.get("value")
314-
.and_then(|v| v.as_str())
315-
.ok_or_else(|| {
316-
McpError::new(
317-
rmcp::model::ErrorCode::INVALID_PARAMS,
318-
"\"add_tag\" operation requires a \"value\" field",
319-
None::<serde_json::Value>,
320-
)
321-
})?;
322-
ops.push(FrontmatterOp::AddTag(value.to_string()));
324+
FrontmatterOp::Remove(key.to_string())
323325
}
324-
"remove_tag" => {
325-
let value = op_val
326-
.get("value")
327-
.and_then(|v| v.as_str())
328-
.ok_or_else(|| {
329-
McpError::new(
330-
rmcp::model::ErrorCode::INVALID_PARAMS,
331-
"\"remove_tag\" operation requires a \"value\" field",
332-
None::<serde_json::Value>,
333-
)
334-
})?;
335-
ops.push(FrontmatterOp::RemoveTag(value.to_string()));
326+
FrontmatterOpKind::AddTag => {
327+
let value = input.value.as_deref().ok_or_else(|| {
328+
McpError::new(
329+
rmcp::model::ErrorCode::INVALID_PARAMS,
330+
"\"add_tag\" operation requires a \"value\" field",
331+
None::<serde_json::Value>,
332+
)
333+
})?;
334+
FrontmatterOp::AddTag(value.to_string())
336335
}
337-
"add_alias" => {
338-
let value = op_val
339-
.get("value")
340-
.and_then(|v| v.as_str())
341-
.ok_or_else(|| {
342-
McpError::new(
343-
rmcp::model::ErrorCode::INVALID_PARAMS,
344-
"\"add_alias\" operation requires a \"value\" field",
345-
None::<serde_json::Value>,
346-
)
347-
})?;
348-
ops.push(FrontmatterOp::AddAlias(value.to_string()));
336+
FrontmatterOpKind::RemoveTag => {
337+
let value = input.value.as_deref().ok_or_else(|| {
338+
McpError::new(
339+
rmcp::model::ErrorCode::INVALID_PARAMS,
340+
"\"remove_tag\" operation requires a \"value\" field",
341+
None::<serde_json::Value>,
342+
)
343+
})?;
344+
FrontmatterOp::RemoveTag(value.to_string())
349345
}
350-
"remove_alias" => {
351-
let value = op_val
352-
.get("value")
353-
.and_then(|v| v.as_str())
354-
.ok_or_else(|| {
355-
McpError::new(
356-
rmcp::model::ErrorCode::INVALID_PARAMS,
357-
"\"remove_alias\" operation requires a \"value\" field",
358-
None::<serde_json::Value>,
359-
)
360-
})?;
361-
ops.push(FrontmatterOp::RemoveAlias(value.to_string()));
346+
FrontmatterOpKind::AddAlias => {
347+
let value = input.value.as_deref().ok_or_else(|| {
348+
McpError::new(
349+
rmcp::model::ErrorCode::INVALID_PARAMS,
350+
"\"add_alias\" operation requires a \"value\" field",
351+
None::<serde_json::Value>,
352+
)
353+
})?;
354+
FrontmatterOp::AddAlias(value.to_string())
362355
}
363-
unknown => {
364-
return Err(McpError::new(
365-
rmcp::model::ErrorCode::INVALID_PARAMS,
366-
format!("unknown frontmatter operation: \"{unknown}\""),
367-
None::<serde_json::Value>,
368-
));
356+
FrontmatterOpKind::RemoveAlias => {
357+
let value = input.value.as_deref().ok_or_else(|| {
358+
McpError::new(
359+
rmcp::model::ErrorCode::INVALID_PARAMS,
360+
"\"remove_alias\" operation requires a \"value\" field",
361+
None::<serde_json::Value>,
362+
)
363+
})?;
364+
FrontmatterOp::RemoveAlias(value.to_string())
369365
}
370-
}
366+
};
367+
ops.push(op);
371368
}
372369
Ok(ops)
373370
}
@@ -1165,3 +1162,59 @@ pub async fn run_serve(
11651162

11661163
Ok(())
11671164
}
1165+
1166+
#[cfg(test)]
1167+
mod tests {
1168+
use super::*;
1169+
1170+
/// Regression test for <https://github.com/devwhodevs/engraph/issues/32>.
1171+
#[test]
1172+
fn edit_frontmatter_operations_schema_has_object_items() {
1173+
let schema = schemars::schema_for!(EditFrontmatterParams);
1174+
let json = serde_json::to_value(&schema).unwrap();
1175+
1176+
let items = &json["properties"]["operations"]["items"];
1177+
assert!(
1178+
items.is_object(),
1179+
"operations.items must be an object schema, got: {items}"
1180+
);
1181+
1182+
// schemars may inline properties or use a $ref to $defs; both are
1183+
// valid object schemas that OpenAI accepts.
1184+
let has_properties = items.get("properties").is_some();
1185+
let has_ref = items.get("$ref").is_some();
1186+
assert!(
1187+
has_properties || has_ref,
1188+
"operations.items must define properties or $ref, got: {items}"
1189+
);
1190+
}
1191+
1192+
#[test]
1193+
fn frontmatter_op_input_deserializes_all_variants() {
1194+
let cases = [
1195+
(r#"{"op":"set","key":"status","value":"done"}"#, "set"),
1196+
(r#"{"op":"remove","key":"status"}"#, "remove"),
1197+
(r#"{"op":"add_tag","value":"rust"}"#, "add_tag"),
1198+
(r#"{"op":"remove_tag","value":"old"}"#, "remove_tag"),
1199+
(r#"{"op":"add_alias","value":"eng"}"#, "add_alias"),
1200+
(r#"{"op":"remove_alias","value":"eng"}"#, "remove_alias"),
1201+
];
1202+
for (json, label) in cases {
1203+
let input: FrontmatterOpInput = serde_json::from_str(json)
1204+
.unwrap_or_else(|e| panic!("failed to deserialize {label}: {e}"));
1205+
// Verify the parsed input converts to a valid FrontmatterOp.
1206+
let ops = parse_frontmatter_ops(&[input]);
1207+
assert!(ops.is_ok(), "{label} should produce a valid op: {:?}", ops);
1208+
}
1209+
}
1210+
1211+
#[test]
1212+
fn frontmatter_op_input_rejects_unknown_variant() {
1213+
let json = r#"{"op":"unknown_op","value":"x"}"#;
1214+
let result: Result<FrontmatterOpInput, _> = serde_json::from_str(json);
1215+
assert!(
1216+
result.is_err(),
1217+
"unknown op variant should fail deserialization"
1218+
);
1219+
}
1220+
}

0 commit comments

Comments
 (0)