Skip to content

Hold a field change that a saved query or play names in core's update_schema (closes #3786) #3790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
malibio merged 3 commits into from
Oct 13, 2026
Merged

Hold a field change that a saved query or play names in core's update_schema (closes #3786) #3790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d237a25
Hold a field change that a saved query or play names in core's update…
malibio Oct 13, 2026
6dade87
Address review: refuse a template-named removal ahead of the hold, co…
malibio Oct 13, 2026
f0d9713
Address review: a removed field the call declares again keeps its tem…
malibio Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 12 additions & 146 deletions packages/agent/src/local_agent/tools.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,6 @@ const NODE_MARKDOWN_TRUNCATE: usize = 16_000;
/// window with its references.
const FIELD_REFERENCES_SHOWN: usize = 30;

/// How many are named when `update_schema` refuses a change for them. Fewer
/// than a read lists: they go into one sentence the user reads.
const FIELD_CHANGE_REFERENCES_SHOWN: usize = 10;

/// Maximum characters for node body in list/summary results.
const BODY_TRUNCATE_SUMMARY: usize = 500;

Expand Down Expand Up @@ -1065,119 +1061,6 @@ fn resolved_payload(row: &Value) -> Value {
payload
}

/// What an `update_schema` call that renames or removes a field would leave
/// pointing at nothing, as a refusal to return to the model, or `None` when it
/// changes no field that anything names.
///
/// The fields are the call's `remove_fields` and each `rename_fields` entry
/// whose name changes (a relabel keeps the key). What names them is
/// [`find_field_references`](nodespace_core::schema::field_references::find_field_references).
async fn field_change_refusal(ns: &NodeService, args: &Value) -> Option<String> {
let schema_id = args.get("schema_id").and_then(Value::as_str)?;
let mut fields: Vec<&str> = args
.get("remove_fields")
.and_then(Value::as_array)
.into_iter()
.flatten()
.filter_map(Value::as_str)
.collect();
for rename in args
.get("rename_fields")
.and_then(Value::as_array)
.into_iter()
.flatten()
{
let from = rename.get("from").and_then(Value::as_str);
if let (Some(from), Some(to)) = (from, rename.get("to").and_then(Value::as_str)) {
if from != to {
fields.push(from);
}
}
}
if fields.is_empty() {
return None;
}

// A built-in field is refused by the schema write itself, whatever is
// agreed to, so the user is not asked to agree to it first. The guard
// holds only a field the user may change.
let schema = match ns.get_schema_node(schema_id).await {
Ok(Some(schema)) => schema,
Ok(None) => return None,
Err(e) => {
tracing::warn!(schema_id, error = %e, "could not read the schema to check what names a field; the change is not held");
return None;
}
};
fields.retain(|name| {
schema.fields.iter().any(|field| {
field.name == *name
&& field.protection == nodespace_core::models::SchemaProtectionLevel::User
})
});
if fields.is_empty() {
return None;
}

let found = match nodespace_core::schema::field_references::find_field_references(ns, schema_id)
.await
{
Ok(found) => found,
Err(e) => {
tracing::warn!(schema_id, error = %e, "could not look up what names a field; the change is not held");
return None;
}
};
let named: Vec<_> = found
.references
.iter()
.filter(|r| fields.contains(&r.field.as_str()))
.collect();
if named.is_empty() {
return None;
}

// The what-names-it list is the first sentence, whole: the failure notice
// a user sees when the model says nothing keeps only that sentence. It is
// capped, with the rest counted, as a schema read's is.
let more = named.len().saturating_sub(FIELD_CHANGE_REFERENCES_SHOWN);
let mut names: Vec<String> = named
.iter()
.take(FIELD_CHANGE_REFERENCES_SHOWN)
.map(|reference| {
let kind = match reference.kind {
nodespace_core::schema::field_references::ReferenceKind::SavedQuery => {
"saved query"
}
nodespace_core::schema::field_references::ReferenceKind::Play => "play",
nodespace_core::schema::field_references::ReferenceKind::TitleTemplate => {
"title template of"
}
};
format!(
"{kind} \"{}\" ({}, field `{}`)",
reference.title, reference.place, reference.field
)
})
.collect();
if more > 0 {
names.push(format!("{more} more"));
}
Some(format!(
"Not applied: changing {} would leave {} naming a field that is gone.\n\
Searched: {}.\n\
Tell the user which of these names it, and ask whether to go ahead. Once they agree, \
call update_schema again with the same arguments and confirmed=true.",
fields
.iter()
.map(|f| format!("`{f}`"))
.collect::<Vec<_>>()
.join(", "),
names.join("; "),
found.searched.join("; ")
))
}

/// Whether a read node is a schema.
fn is_schema_read(node_data: &Value) -> bool {
node_data
Expand Down Expand Up @@ -2451,9 +2334,9 @@ fn def_update_schema() -> ToolDefinition {
"description": "Context paths to remove, written as they were added (e.g. \"spec.decisions\").",
"items": { "type": "string" }
},
"confirmed": {
"force": {
"type": "boolean",
"description": "Set true only to repeat a call that was refused because the change would leave saved queries, plays or title templates naming a field that is gone, and only after the user has agreed to it."
"description": "Set true only to repeat a call that was refused because the change would leave saved queries or plays naming a field that is gone or would affect active plays, and only after the user has agreed to it."
}
},
"required": ["schema_id"]
Expand Down Expand Up @@ -5814,23 +5697,6 @@ impl GraphToolExecutor {
let mut args = args;
let as_text = enum_fields_without_values_as_text(&mut args, "add_fields");

// A rename or a removal leaves whatever names the field naming
// nothing. Said before it is written, so the user can agree to it
// (ADR-100 §4, §5); `confirmed` is the agreement. It is this tool's
// own and is not passed on: the schema write's `force` (for the
// plays a change affects) is a different agreement, and stays out of
// the tool's parameters.
let confirmed = args
.as_object_mut()
.and_then(|object| object.remove("confirmed"))
.and_then(|value| value.as_bool())
== Some(true);
if !confirmed {
if let Some(refusal) = field_change_refusal(&ns, &args).await {
return Ok(error_result(tool_call_id, "update_schema", &refusal));
}
}

let result = handle_update_schema(&ns, args).await;

match result {
Expand Down Expand Up @@ -10831,9 +10697,9 @@ mod tests {
}

/// A rename or a removal that would leave a saved query naming a
/// field that is gone is refused with what names it, and goes through
/// once repeated with `confirmed`. A relabel, an addition and a
/// built-in field are not held.
/// field that is gone is refused by the schema write with what names
/// it, and goes through once repeated with `force`. A relabel is not
/// held, and a built-in field is refused whatever is agreed.
#[tokio::test(flavor = "multi_thread")]
async fn renaming_a_named_field_is_refused_until_forced() {
let (ns, _tmp) = make_test_service().await;
Expand Down Expand Up @@ -10881,7 +10747,7 @@ mod tests {
&& text.contains(
"saved query \\\"Open tickets\\\" (filter on state, field `state`)"
)
&& text.contains("confirmed=true"),
&& text.contains("force=true"),
"{text}"
);
let names: Vec<String> = ns
Expand All @@ -10908,13 +10774,13 @@ mod tests {
.unwrap();
assert!(!relabel.is_error, "{}", relabel.result);

let mut confirmed = rename;
confirmed["confirmed"] = json!(true);
let done = executor.execute("update_schema", confirmed).await.unwrap();
let mut forced = rename;
forced["force"] = json!(true);
let done = executor.execute("update_schema", forced).await.unwrap();
assert!(!done.is_error, "{}", done.result);

// A built-in field is not held for agreement: the schema write
// refuses it whatever is agreed, and says so itself.
// A core-protected field is not held for agreement: the schema
// write refuses it whatever is agreed, and says so itself.
ns.create_node(nodespace_core::models::Node::new(
"query".to_string(),
"Open tasks".to_string(),
Expand All @@ -10938,7 +10804,7 @@ mod tests {
assert!(built_in.is_error);
let text = built_in.result.to_string();
assert!(
!text.contains("confirmed=true") && text.contains("protected"),
!text.contains("force=true") && text.contains("protected"),
"{text}"
);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/agent/src/seeds/skills/understanding-a-type.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ This skill explains a type and changes it. A type is a schema: its fields, each
READ THE TYPE FIRST: get the schema node and read it as stored. Its fields are the effective set, the type's own and then the ones it inherits, and a field's `protection` says whether it is built in. The read also lists `field_references`: the saved queries, plays and title templates that name each of the type's own fields, and what was searched to find them.

SAY WHICH CASE THE TYPE IS IN BEFORE OFFERING ANY CHANGE:
- A built-in type (`isCore`) is closed. Its declared fields cannot be changed or removed. What may be added is a field of the user's own, whose name carries a prefix such as `custom:`, and the user's own values to a list that accepts them, which is a field with `extensible` set.
- A type the user defined is open. Its own fields can be added, changed and removed. A field it inherits from a built-in type stays closed.
- A built-in type (`isCore`) is closed, with one exception. A declared field whose `protection` is `core` or `system` cannot be changed or removed. A declared field whose `protection` is `user` (`priority` and the dates on a task) can be, after what names it is dealt with. What may be added is a field of the user's own, whose name carries a prefix such as `custom:`, and the user's own values to a list that accepts them, which is a field with `extensible` set.
- A type the user defined is open. Its own fields can be added, changed and removed. A field it inherits from a built-in type is as closed as it is there.
- A user's type that extends a built-in one has both: its own fields are open and the inherited ones are closed.
Say the case in one sentence, then what the user can do in it. A request to change or remove a declared field of a built-in type is explained and not attempted: say that it is built in and offer a field of their own or a value instead.
Say the case in one sentence, then what the user can do in it. A request to change or remove a `core` or `system` field of a built-in type is explained and not attempted: say that it is built in and offer a field of their own or a value instead.

EXPLAINING WRITES NOTHING: answer what a field means, which values it takes, what is required, what the type extends and what may be changed from the stored schema, and name only what it declares. Say when the schema gives no description for a field instead of guessing one.

A WELL-FORMED FIELD: a snake_case name, the field type that fits what it holds, a default where one is needed, a value list in place of free text where the values are known, and a relationship in place of a list of ids when the field refers to another type of node. A name that is already a field of the type, or of a type it extends, is refused with the reason, and so is a declared field's name behind a prefix (`custom:status` beside `status`): use the existing field, or ask for a different name. Take the fields from what the user asked for.

CHECK WHAT A CHANGE WOULD BREAK: before renaming a field or removing one, read the `field_references` for it and tell the user each saved query, play and title template that names it, and where, with what was searched. A result of none rules out only what was searched. If the read has no `field_references`, search the saved queries and plays for the field's name and say that was a search by name. A rename or a removal that something names is refused with that list and changes nothing: tell the user, and repeat the call with `confirmed` set once they agree. A rename is a rekey of every node's stored value and breaks each name that is not changed with it. A field the title template names cannot be renamed until the template is set to one that does not name it, then set again with the new name. A removal drops the declaration and leaves the values nodes already hold stored but unread, and a template that names the field blocks it.
CHECK WHAT A CHANGE WOULD BREAK: before renaming a field or removing one, read the `field_references` for it and tell the user each saved query, play and title template that names it, and where, with what was searched. A result of none rules out only what was searched. If the read has no `field_references`, search the saved queries and plays for the field's name and say that was a search by name. A rename or a removal that a saved query or play names is refused with that list and changes nothing: tell the user, and repeat the call with `force` set once they agree. A rename is a rekey of every node's stored value and breaks each name that is not changed with it. A field the title template names cannot be renamed until the template is set to one that does not name it, then set again with the new name. A removal drops the declaration and leaves the values nodes already hold stored but unread, and a template that names the field blocks it.

SAY WHAT WILL CHANGE, THEN ASK: before writing, say in one or two plain sentences what the type will look like afterwards and what the change affects, and write only after the user agrees. A change that adds a field or a value affects nothing that exists, and still waits for the yes.

Expand Down
8 changes: 4 additions & 4 deletions packages/agent/tests/golden/prompt_assembly/stage2_tool_surface.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,14 +688,14 @@ parameters_schema:
},
"type": "array"
},
"confirmed": {
"description": "Set true only to repeat a call that was refused because the change would leave saved queries, plays or title templates naming a field that is gone, and only after the user has agreed to it.",
"type": "boolean"
},
"description": {
"description": "New description of the SCHEMA TYPE ITSELF, not of any one field — a summary of what this entity type represents. Optional.",
"type": "string"
},
"force": {
"description": "Set true only to repeat a call that was refused because the change would leave saved queries or plays naming a field that is gone or would affect active plays, and only after the user has agreed to it.",
"type": "boolean"
},
"remove_context_paths": {
"description": "Context paths to remove, written as they were added (e.g. \"spec.decisions\").",
"items": {
Expand Down
Loading