diff --git a/server/src/http/mcp/story_tools/bug.rs b/server/src/http/mcp/story_tools/bug.rs index 772a9de7..3117e017 100644 --- a/server/src/http/mcp/story_tools/bug.rs +++ b/server/src/http/mcp/story_tools/bug.rs @@ -26,6 +26,10 @@ pub(crate) fn tool_create_bug(args: &Value, ctx: &AppContext) -> Result Result Result { let req = CreateEpicRequest::from_json(args)?; + // Bug 1102: resolve and validate origin BEFORE creating the epic so a + // missing-attribution call leaves no half-state behind. + let origin = super::build_origin(args)?; + let root = ctx.state.get_project_root()?; let success_criteria = req.success_criteria_strings(); @@ -29,7 +33,7 @@ pub(crate) fn tool_create_epic(args: &Value, ctx: &AppContext) -> Result}`. +/// `args` must contain an `"origin"` object with a non-empty `id` field and an +/// optional `kind` (defaulting to `"user"`) and `ts` (defaulting to now). The +/// `id` MUST identify the calling actor — e.g. `coder-1@story=42` for a coder +/// agent, `chat-bot:Timmy@` for a chat-bot session, or a human's user +/// id for a CLI/MCP-direct call. Empty / whitespace-only `id` is rejected so +/// that every work item carries a usable provenance trail (bug 1102 — we lost +/// 1102's attribution because the default was `id=""`). /// -/// Callers that create items on behalf of system automation (e.g. gate-failure -/// auto-filing) should pass `kind = "system"` and `id = ""`. -pub(super) fn build_origin(args: &serde_json::Value) -> String { +/// Returns the canonical origin JSON string on success. Returns `Err` with a +/// human-readable explanation when the caller failed to identify itself; the +/// caller (`tool_create_*` handlers) must propagate the error without creating +/// the work item, so a missing-attribution call leaves no half-state behind. +pub(super) fn build_origin(args: &serde_json::Value) -> Result { let ts = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_secs_f64(); - if let Some(origin_obj) = args.get("origin").and_then(|v| v.as_object()) { - let kind = origin_obj - .get("kind") - .and_then(|v| v.as_str()) - .unwrap_or("user"); - let id = origin_obj.get("id").and_then(|v| v.as_str()).unwrap_or(""); - let ts_val = origin_obj.get("ts").and_then(|v| v.as_f64()).unwrap_or(ts); - serde_json::json!({"kind": kind, "id": id, "ts": ts_val}).to_string() - } else { - serde_json::json!({"kind": "user", "id": "", "ts": ts}).to_string() - } + let origin_obj = args.get("origin").and_then(|v| v.as_object()).ok_or( + "Missing required argument: 'origin'. Every create_* MCP call must \ + identify the calling actor. Pass origin = { \"kind\": \"agent\" | \ + \"chat-bot\" | \"user\" | \"system\", \"id\": \"\", \ + \"ts\": }. Example: { \"kind\": \"agent\", \ + \"id\": \"coder-1@story=42\" }.", + )?; + + let id = origin_obj + .get("id") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .ok_or( + "origin.id must be a non-empty string identifying the calling \ + actor (e.g. \"coder-1@story=42\", \"chat-bot:Timmy@!room:home\", \ + or a human user id). See bug 1102 / story 1104 for the rationale.", + )?; + + let kind = origin_obj + .get("kind") + .and_then(|v| v.as_str()) + .unwrap_or("user"); + let ts_val = origin_obj.get("ts").and_then(|v| v.as_f64()).unwrap_or(ts); + + Ok(serde_json::json!({"kind": kind, "id": id, "ts": ts_val}).to_string()) } pub(crate) use bug::{tool_close_bug, tool_create_bug, tool_list_bugs}; diff --git a/server/src/http/mcp/story_tools/refactor.rs b/server/src/http/mcp/story_tools/refactor.rs index 040e6c11..17debcb1 100644 --- a/server/src/http/mcp/story_tools/refactor.rs +++ b/server/src/http/mcp/story_tools/refactor.rs @@ -27,6 +27,10 @@ pub(crate) fn tool_create_refactor(args: &Value, ctx: &AppContext) -> Result Result Result Result Result { let req = CreateStoryRequest::from_json(args)?; + // Bug 1102: resolve and validate origin BEFORE creating the story file so + // a missing-attribution call leaves no half-state behind. + let origin = super::super::build_origin(args)?; + let root = ctx.state.get_project_root()?; let depends_on_ids = req.depends_on_ids(); @@ -31,7 +35,7 @@ pub(crate) fn tool_create_story(args: &Value, ctx: &AppContext) -> Resultbold name", - "acceptance_criteria": ["AC1"] + "acceptance_criteria": ["AC1"], + "origin": {"kind": "test", "id": "test-suite"} }), &ctx, ); diff --git a/server/src/http/mcp/story_tools/story/query.rs b/server/src/http/mcp/story_tools/story/query.rs index 800201fe..c2f36d10 100644 --- a/server/src/http/mcp/story_tools/story/query.rs +++ b/server/src/http/mcp/story_tools/story/query.rs @@ -130,7 +130,11 @@ mod tests { let ctx = test_ctx(tmp.path()); let result = super::super::tool_create_story( - &json!({"name": "Test Story", "acceptance_criteria": ["AC1", "AC2"]}), + &json!({ + "name": "Test Story", + "acceptance_criteria": ["AC1", "AC2"], + "origin": {"kind": "test", "id": "test-suite"} + }), &ctx, ) .unwrap(); diff --git a/server/src/http/mcp/tools_list/story_tools.rs b/server/src/http/mcp/tools_list/story_tools.rs index 6148278c..88217ed5 100644 --- a/server/src/http/mcp/tools_list/story_tools.rs +++ b/server/src/http/mcp/tools_list/story_tools.rs @@ -2,6 +2,31 @@ use serde_json::{Value, json}; +/// JSON schema fragment for the `origin` argument required by every `create_*` +/// tool (bug 1102). The caller MUST identify itself — empty `id` is rejected +/// server-side so every work item carries a usable provenance trail. +fn origin_schema() -> Value { + json!({ + "type": "object", + "description": "Required: identifies the calling actor so every work item carries provenance. Empty/missing id is rejected (bug 1102). Examples: { \"kind\": \"agent\", \"id\": \"coder-1@story=42\" }, { \"kind\": \"chat-bot\", \"id\": \"Timmy@!room:home.local\" }, { \"kind\": \"user\", \"id\": \"dave\" }.", + "properties": { + "kind": { + "type": "string", + "description": "One of: \"agent\" (LLM coder/mergemaster/qa), \"chat-bot\" (Timmy or other chat-routed bot), \"user\" (human via CLI/MCP), \"system\" (server-automation)." + }, + "id": { + "type": "string", + "description": "Non-empty identifier of the caller. For agents include the story id (e.g. \"coder-1@story=42\"); for chat-bots include the room/session (e.g. \"Timmy@!room:home.local\"); for users the user id or short name." + }, + "ts": { + "type": "number", + "description": "Optional unix-seconds timestamp. Defaults to the server's clock when absent." + } + }, + "required": ["kind", "id"] + }) +} + /// Returns tool schemas for story/work-item lifecycle management. pub(super) fn story_tools() -> Vec { vec![ @@ -37,9 +62,10 @@ pub(super) fn story_tools() -> Vec { "commit": { "type": "boolean", "description": "If true, git-add and git-commit the new story file to the current branch" - } + }, + "origin": origin_schema() }, - "required": ["name", "acceptance_criteria"] + "required": ["name", "acceptance_criteria", "origin"] } }), json!({ @@ -282,9 +308,10 @@ pub(super) fn story_tools() -> Vec { "items": { "type": "string" }, "minItems": 1, "description": "List of acceptance criteria (at least one required)" - } + }, + "origin": origin_schema() }, - "required": ["name", "acceptance_criteria"] + "required": ["name", "acceptance_criteria", "origin"] } }), json!({ @@ -323,9 +350,10 @@ pub(super) fn story_tools() -> Vec { "type": "array", "items": { "type": "integer" }, "description": "Optional list of story numbers this bug depends on (e.g. [42, 43]). Persisted as depends_on in YAML front matter." - } + }, + "origin": origin_schema() }, - "required": ["name", "description", "steps_to_reproduce", "actual_result", "expected_result", "acceptance_criteria"] + "required": ["name", "description", "steps_to_reproduce", "actual_result", "expected_result", "acceptance_criteria", "origin"] } }), json!({ @@ -360,9 +388,10 @@ pub(super) fn story_tools() -> Vec { "type": "array", "items": { "type": "integer" }, "description": "Optional list of story numbers this refactor depends on (e.g. [42, 43]). Persisted as depends_on in YAML front matter." - } + }, + "origin": origin_schema() }, - "required": ["name", "acceptance_criteria"] + "required": ["name", "acceptance_criteria", "origin"] } }), json!({ @@ -399,9 +428,10 @@ pub(super) fn story_tools() -> Vec { "type": "array", "items": { "type": "string" }, "description": "Optional: list of high-level success criteria for the epic" - } + }, + "origin": origin_schema() }, - "required": ["name", "goal"] + "required": ["name", "goal", "origin"] } }), json!({