diff --git a/CHANGELOG.md b/CHANGELOG.md index 14d5c40..bbe426e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ All notable changes to the Toolpath workspace are documented here. +## Actor validation fixes — derived paths conform to the base schema — unreleased + +`derive_path` produced actor strings the base JSON Schema rejected: event steps +used `provider:`, system turns used `system:`, and `Role::Other` +turns used `:unknown` — none of those prefixes are in the schema's +`actorRef` vocabulary. Separately the `actorRef` pattern's name segment +disallowed `.`, so dotted model identifiers like `agent:gpt-5.5` failed +validation. + +`toolpath`: `actorRef`'s name segment now allows `.`. `toolpath-convo`: +`derive_path` emits `tool:` for event steps, system turns, and +`Role::Other` turns (the role label stays in the change's `role` field), and a +new test derives a path covering every actor variant and validates it against +the embedded base schema so this can't regress. Touches `toolpath` and +`toolpath-convo`; versions to be bumped at release. + ## `path resume` — one-shot resume into a coding agent — 2026-05-09 `path-cli` 0.9.0. New subcommand `path resume ` that fetches a diff --git a/Cargo.lock b/Cargo.lock index c7e90b3..cf42b08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2690,6 +2690,7 @@ name = "toolpath-convo" version = "0.8.0" dependencies = [ "chrono", + "jsonschema", "serde", "serde_json", "similar", diff --git a/RFC.md b/RFC.md index a3d062c..873575d 100644 --- a/RFC.md +++ b/RFC.md @@ -260,12 +260,17 @@ An **actor** is anything that performs steps. The short form is a string: ``` human:alex agent:claude-code +agent:gpt-5.5 agent:claude-code/session-xyz tool:rustfmt tool:rustfmt/1.5.0 ci:github-actions/workflow-123 ``` +The prefix is one of `human`, `agent`, `tool`, or `ci`; the name segment may +contain letters, digits, `.`, `_`, and `-`, with an optional `/`-delimited +suffix. + Actor strings are referenced in `step.actor`. Full actor definitions with identity and key information are provided in `meta.actors`. diff --git a/crates/toolpath-codex/tests/fidelity.rs b/crates/toolpath-codex/tests/fidelity.rs index 1af3f30..0441c73 100644 --- a/crates/toolpath-codex/tests/fidelity.rs +++ b/crates/toolpath-codex/tests/fidelity.rs @@ -151,9 +151,9 @@ fn head_equals_last_step_id() { #[test] fn actor_scheme_matches_source_role() { // Source role → actor-prefix mapping must be consistent: - // "developer" | "system" → "system:*" - // "user" → "human:*" - // "assistant" → "agent:*" + // "user" → "human:*" + // "assistant" → "agent:*" + // "developer" | "system" → "tool:*" // We can't assert a strict 1:1 turn→step mapping (carrier turns // may collapse), but we can assert every observed role in the // view reaches a step with the expected actor prefix. @@ -178,7 +178,7 @@ fn actor_scheme_matches_source_role() { assert!(prefixes.contains("agent"), "no step has an agent:* actor"); } if system_seen { - assert!(prefixes.contains("system"), "no step has a system:* actor"); + assert!(prefixes.contains("tool"), "no step has a tool:* actor"); } } diff --git a/crates/toolpath-convo/Cargo.toml b/crates/toolpath-convo/Cargo.toml index 53a6598..9a23c0e 100644 --- a/crates/toolpath-convo/Cargo.toml +++ b/crates/toolpath-convo/Cargo.toml @@ -15,3 +15,6 @@ serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } similar = { workspace = true } + +[dev-dependencies] +jsonschema = { version = "0.46", default-features = false } diff --git a/crates/toolpath-convo/src/derive.rs b/crates/toolpath-convo/src/derive.rs index faabfb5..73893b4 100644 --- a/crates/toolpath-convo/src/derive.rs +++ b/crates/toolpath-convo/src/derive.rs @@ -318,7 +318,7 @@ pub fn derive_path(view: &ConversationView, config: &DeriveConfig) -> Path { } else { event.id.clone() }; - let actor = format!("provider:{}", provider); + let actor = format!("tool:{}", provider); actors .entry(actor.clone()) .or_insert_with(|| ActorDefinition { @@ -448,8 +448,8 @@ fn actor_for_turn(turn: &Turn, provider: &str) -> String { let model = turn.model.as_deref().unwrap_or("unknown"); format!("agent:{}", model) } - Role::System => format!("system:{}", provider), - Role::Other(s) => format!("{}:unknown", s), + Role::System => format!("tool:{}", provider), + Role::Other(_) => format!("tool:{}", provider), } } @@ -477,10 +477,10 @@ fn record_actor( ..Default::default() } } else { - // system:*, other:* let name = actor.split_once(':').map(|x| x.1).unwrap_or("").to_string(); ActorDefinition { name: Some(name), + provider: Some(provider.to_string()), ..Default::default() } }; @@ -719,7 +719,7 @@ mod tests { let turn = base_turn("t1", Role::System); let view = view_with(vec![turn]); let path = derive_path(&view, &DeriveConfig::default()); - assert_eq!(path.steps[0].step.actor, "system:pi"); + assert_eq!(path.steps[0].step.actor, "tool:pi"); } #[test] @@ -727,7 +727,7 @@ mod tests { let turn = base_turn("t1", Role::Other("tool".into())); let view = view_with(vec![turn]); let path = derive_path(&view, &DeriveConfig::default()); - assert_eq!(path.steps[0].step.actor, "tool:unknown"); + assert_eq!(path.steps[0].step.actor, "tool:pi"); } #[test] @@ -741,6 +741,39 @@ mod tests { assert_eq!(path.steps[1].step.parents, vec!["t1".to_string()]); } + #[test] + fn derived_path_validates_against_base_schema() { + let user = base_turn("t1", Role::User); + let mut assistant = base_turn("t2", Role::Assistant); + assistant.parent_id = Some("t1".into()); + assistant.model = Some("gpt-5.5".into()); + let system = base_turn("t3", Role::System); + let other = base_turn("t4", Role::Other("bash".into())); + + let mut view = view_with(vec![user, assistant, system, other]); + view.events.push(crate::ConversationEvent { + id: "e1".into(), + timestamp: "2026-01-01T00:00:00Z".into(), + parent_id: None, + event_type: "attachment".into(), + data: HashMap::new(), + }); + + let path = derive_path(&view, &DeriveConfig::default()); + let graph = serde_json::json!({ + "graph": { "id": "g1" }, + "paths": [serde_json::to_value(&path).unwrap()], + }); + + let schema: serde_json::Value = serde_json::from_str(toolpath::SCHEMA_JSON).unwrap(); + let validator = jsonschema::validator_for(&schema).unwrap(); + let errors: Vec = validator + .iter_errors(&graph) + .map(|e| format!("at {}: {e}", e.instance_path())) + .collect(); + assert!(errors.is_empty(), "base-schema violations:\n{}", errors.join("\n")); + } + fn fw_tool(name: &str, id: &str, input: serde_json::Value) -> ToolInvocation { ToolInvocation { id: id.to_string(), diff --git a/crates/toolpath/schema/toolpath.schema.json b/crates/toolpath/schema/toolpath.schema.json index aab7962..2a3778c 100644 --- a/crates/toolpath/schema/toolpath.schema.json +++ b/crates/toolpath/schema/toolpath.schema.json @@ -13,9 +13,9 @@ "actorRef": { "type": "string", - "pattern": "^(human|agent|tool|ci):[a-zA-Z0-9_-]+(\/[a-zA-Z0-9_.-]+)?$", - "description": "Actor reference string (e.g., 'human:alex', 'agent:claude-code', 'tool:rustfmt/1.5.0')", - "examples": ["human:alex", "agent:claude-code", "tool:rustfmt", "ci:github-actions"] + "pattern": "^(human|agent|tool|ci):[a-zA-Z0-9_.-]+(\/[a-zA-Z0-9_.-]+)?$", + "description": "Actor reference string (e.g., 'human:alex', 'agent:gpt-5.5', 'tool:rustfmt/1.5.0').", + "examples": ["human:alex", "agent:claude-code", "agent:gpt-5.5", "tool:rustfmt", "ci:github-actions"] }, "identity": { diff --git a/docs/agents/formats/codex.md b/docs/agents/formats/codex.md index 83f738b..a5b3458 100644 --- a/docs/agents/formats/codex.md +++ b/docs/agents/formats/codex.md @@ -809,7 +809,7 @@ The mapping below is what the provider actually emits. Source: | `turn_context` (full) | `ConversationEvent` (round-trip preservation) | | `message` role `user` | `Turn { role: User }` → Step with `actor: "human:user"` | | `message` role `assistant` | `Turn { role: Assistant, model }` → Step with `actor: "agent:"` | -| `message` role `developer` | `Turn { role: System }` → Step with `actor: "system:codex"` | +| `message` role `developer` | `Turn { role: System }` → Step with `actor: "tool:codex"` | | `reasoning.encrypted_content` | `Turn.extra["codex"]["reasoning_encrypted"]` (**not** `Turn.thinking` — it would render as ciphertext) | | `reasoning.summary[].text` / `reasoning.content[].text` (plaintext) | `Turn.thinking` on the next assistant turn | | `function_call` / `function_call_output` paired by `call_id` | `Turn.tool_uses[].{input, result}` |