From 13b6ecd958dea9ec73a9165716cc5eb38682b33f Mon Sep 17 00:00:00 2001 From: dave Date: Mon, 23 Mar 2026 12:57:49 +0000 Subject: [PATCH] storkit: merge 370_bug_scaffold_does_not_create_mcp_json_in_project_root --- server/src/agents/pool/mod.rs | 4 + server/src/http/project.rs | 1 + server/src/io/fs.rs | 142 +++++++++++++++++++++++++--------- server/src/main.rs | 2 + 4 files changed, 113 insertions(+), 36 deletions(-) diff --git a/server/src/agents/pool/mod.rs b/server/src/agents/pool/mod.rs index 0d0b751..9c123e7 100644 --- a/server/src/agents/pool/mod.rs +++ b/server/src/agents/pool/mod.rs @@ -144,6 +144,10 @@ impl AgentPool { } } + pub fn port(&self) -> u16 { + self.port + } + /// Create a pool with a dummy watcher channel for unit tests. #[cfg(test)] pub fn new_test(port: u16) -> Self { diff --git a/server/src/http/project.rs b/server/src/http/project.rs index ac07b18..37a48ab 100644 --- a/server/src/http/project.rs +++ b/server/src/http/project.rs @@ -39,6 +39,7 @@ impl ProjectApi { payload.0.path, &self.ctx.state, self.ctx.store.as_ref(), + self.ctx.agents.port(), ) .await .map_err(bad_request)?; diff --git a/server/src/io/fs.rs b/server/src/io/fs.rs index fef42c7..01788e0 100644 --- a/server/src/io/fs.rs +++ b/server/src/io/fs.rs @@ -369,7 +369,7 @@ fn write_story_kit_gitignore(root: &Path) -> Result<(), String> { /// the project root and git does not support `../` patterns in `.gitignore` /// files, so they cannot be expressed in `.storkit/.gitignore`. fn append_root_gitignore_entries(root: &Path) -> Result<(), String> { - let entries = [".storkit_port", "store.json"]; + let entries = [".storkit_port", "store.json", ".mcp.json"]; let gitignore_path = root.join(".gitignore"); let existing = if gitignore_path.exists() { @@ -404,7 +404,7 @@ fn append_root_gitignore_entries(root: &Path) -> Result<(), String> { Ok(()) } -fn scaffold_story_kit(root: &Path) -> Result<(), String> { +fn scaffold_story_kit(root: &Path, port: u16) -> Result<(), String> { let story_kit_root = root.join(".storkit"); let specs_root = story_kit_root.join("specs"); let tech_root = specs_root.join("tech"); @@ -440,6 +440,14 @@ fn scaffold_story_kit(root: &Path) -> Result<(), String> { write_script_if_missing(&script_root.join("test"), STORY_KIT_SCRIPT_TEST)?; write_file_if_missing(&root.join("CLAUDE.md"), STORY_KIT_CLAUDE_MD)?; + // Write .mcp.json at the project root so agents can find the MCP server. + // Only written when missing — never overwrites an existing file, because + // the port is environment-specific and must not clobber a running instance. + let mcp_content = format!( + "{{\n \"mcpServers\": {{\n \"storkit\": {{\n \"type\": \"http\",\n \"url\": \"http://localhost:{port}/mcp\"\n }}\n }}\n}}\n" + ); + write_file_if_missing(&root.join(".mcp.json"), &mcp_content)?; + // Create .claude/settings.json with sensible permission defaults so that // Claude Code (both agents and web UI chat) can operate without constant // permission prompts. @@ -505,14 +513,14 @@ fn scaffold_story_kit(root: &Path) -> Result<(), String> { Ok(()) } -async fn ensure_project_root_with_story_kit(path: PathBuf) -> Result<(), String> { +async fn ensure_project_root_with_story_kit(path: PathBuf, port: u16) -> Result<(), String> { tokio::task::spawn_blocking(move || { if !path.exists() { fs::create_dir_all(&path) .map_err(|e| format!("Failed to create project directory: {}", e))?; } if !path.join(".storkit").is_dir() { - scaffold_story_kit(&path)?; + scaffold_story_kit(&path, port)?; } Ok(()) }) @@ -524,10 +532,11 @@ pub async fn open_project( path: String, state: &SessionState, store: &dyn StoreOps, + port: u16, ) -> Result { let p = PathBuf::from(&path); - ensure_project_root_with_story_kit(p.clone()).await?; + ensure_project_root_with_story_kit(p.clone(), port).await?; validate_project_path(p.clone()).await?; { @@ -816,7 +825,7 @@ mod tests { let store = make_store(&dir); let state = SessionState::default(); - let result = open_project(project_dir.to_string_lossy().to_string(), &state, &store).await; + let result = open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001).await; assert!(result.is_ok()); let root = state.get_project_root().unwrap(); @@ -824,26 +833,47 @@ mod tests { } #[tokio::test] - async fn open_project_does_not_write_mcp_json() { - // open_project must NOT overwrite .mcp.json — test servers started by QA - // agents share the real project root, so writing here would clobber the - // root .mcp.json with the wrong port. .mcp.json is written once during - // worktree creation (worktree.rs) and should not be touched again. + async fn open_project_does_not_overwrite_existing_mcp_json() { + // scaffold must NOT overwrite .mcp.json when it already exists — QA + // test servers share the real project root, and re-writing would + // clobber the file with the wrong port. + let dir = tempdir().unwrap(); + let project_dir = dir.path().join("myproject"); + fs::create_dir_all(&project_dir).unwrap(); + // Pre-write .mcp.json with a different port to simulate an already-configured project. + let mcp_path = project_dir.join(".mcp.json"); + fs::write(&mcp_path, "{\"existing\": true}").unwrap(); + let store = make_store(&dir); + let state = SessionState::default(); + + open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(&mcp_path).unwrap(), + "{\"existing\": true}", + "open_project must not overwrite an existing .mcp.json" + ); + } + + #[tokio::test] + async fn open_project_writes_mcp_json_when_missing() { let dir = tempdir().unwrap(); let project_dir = dir.path().join("myproject"); fs::create_dir_all(&project_dir).unwrap(); let store = make_store(&dir); let state = SessionState::default(); - open_project(project_dir.to_string_lossy().to_string(), &state, &store) + open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001) .await .unwrap(); let mcp_path = project_dir.join(".mcp.json"); - assert!( - !mcp_path.exists(), - "open_project must not write .mcp.json — that would overwrite the root with the wrong port" - ); + assert!(mcp_path.exists(), "open_project should write .mcp.json for new projects"); + let content = fs::read_to_string(&mcp_path).unwrap(); + assert!(content.contains("3001"), "mcp.json should reference the server port"); + assert!(content.contains("localhost"), "mcp.json should reference localhost"); } #[tokio::test] @@ -898,7 +928,7 @@ mod tests { let store = make_store(&dir); let state = SessionState::default(); - open_project(project_dir.to_string_lossy().to_string(), &state, &store) + open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001) .await .unwrap(); @@ -1071,7 +1101,7 @@ mod tests { #[test] fn scaffold_story_kit_creates_structure() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); assert!(dir.path().join(".storkit/README.md").exists()); assert!(dir.path().join(".storkit/project.toml").exists()); @@ -1085,7 +1115,7 @@ mod tests { #[test] fn scaffold_story_kit_creates_work_pipeline_dirs() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let stages = [ "1_backlog", @@ -1109,7 +1139,7 @@ mod tests { #[test] fn scaffold_story_kit_project_toml_has_coder_qa_mergemaster() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/project.toml")).unwrap(); assert!(content.contains("[[agent]]")); @@ -1122,7 +1152,7 @@ mod tests { #[test] fn scaffold_context_is_blank_template_not_story_kit_content() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/specs/00_CONTEXT.md")).unwrap(); assert!(content.contains("")); @@ -1138,7 +1168,7 @@ mod tests { #[test] fn scaffold_stack_is_blank_template_not_story_kit_content() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/specs/tech/STACK.md")).unwrap(); assert!(content.contains("")); @@ -1157,7 +1187,7 @@ mod tests { use std::os::unix::fs::PermissionsExt; let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let script_test = dir.path().join("script/test"); assert!(script_test.exists(), "script/test should be created"); @@ -1175,7 +1205,7 @@ mod tests { fs::create_dir_all(readme.parent().unwrap()).unwrap(); fs::write(&readme, "custom content").unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); assert_eq!(fs::read_to_string(&readme).unwrap(), "custom content"); } @@ -1183,13 +1213,13 @@ mod tests { #[test] fn scaffold_story_kit_is_idempotent() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let readme_content = fs::read_to_string(dir.path().join(".storkit/README.md")).unwrap(); let toml_content = fs::read_to_string(dir.path().join(".storkit/project.toml")).unwrap(); // Run again — must not change content or add duplicate .gitignore entries - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); assert_eq!( fs::read_to_string(dir.path().join(".storkit/README.md")).unwrap(), @@ -1237,7 +1267,7 @@ mod tests { .status() .unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); // Only 1 commit should exist — scaffold must not commit into an existing repo let log_output = std::process::Command::new("git") @@ -1256,7 +1286,7 @@ mod tests { #[test] fn scaffold_creates_story_kit_gitignore_with_relative_entries() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); // .storkit/.gitignore must contain relative patterns for files under .storkit/ let sk_content = fs::read_to_string(dir.path().join(".storkit/.gitignore")).unwrap(); @@ -1287,7 +1317,7 @@ mod tests { ) .unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/.gitignore")).unwrap(); let worktrees_count = content.lines().filter(|l| l.trim() == "worktrees/").count(); @@ -1303,7 +1333,7 @@ mod tests { #[test] fn scaffold_creates_claude_md_at_project_root() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let claude_md = dir.path().join("CLAUDE.md"); assert!( @@ -1332,7 +1362,7 @@ mod tests { let claude_md = dir.path().join("CLAUDE.md"); fs::write(&claude_md, "custom CLAUDE.md content").unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); assert_eq!( fs::read_to_string(&claude_md).unwrap(), @@ -1341,6 +1371,46 @@ mod tests { ); } + #[test] + fn scaffold_story_kit_writes_mcp_json_with_port() { + let dir = tempdir().unwrap(); + scaffold_story_kit(dir.path(), 4242).unwrap(); + + let mcp_path = dir.path().join(".mcp.json"); + assert!(mcp_path.exists(), ".mcp.json should be created by scaffold"); + let content = fs::read_to_string(&mcp_path).unwrap(); + assert!(content.contains("4242"), ".mcp.json should reference the given port"); + assert!(content.contains("localhost"), ".mcp.json should reference localhost"); + assert!(content.contains("storkit"), ".mcp.json should name the storkit server"); + } + + #[test] + fn scaffold_story_kit_does_not_overwrite_existing_mcp_json() { + let dir = tempdir().unwrap(); + let mcp_path = dir.path().join(".mcp.json"); + fs::write(&mcp_path, "{\"custom\": true}").unwrap(); + + scaffold_story_kit(dir.path(), 3001).unwrap(); + + assert_eq!( + fs::read_to_string(&mcp_path).unwrap(), + "{\"custom\": true}", + "scaffold should not overwrite an existing .mcp.json" + ); + } + + #[test] + fn scaffold_gitignore_includes_mcp_json() { + let dir = tempdir().unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); + + let root_gitignore = fs::read_to_string(dir.path().join(".gitignore")).unwrap(); + assert!( + root_gitignore.contains(".mcp.json"), + "root .gitignore should include .mcp.json (port is environment-specific)" + ); + } + // --- open_project scaffolding --- #[tokio::test] @@ -1351,7 +1421,7 @@ mod tests { let store = make_store(&dir); let state = SessionState::default(); - open_project(project_dir.to_string_lossy().to_string(), &state, &store) + open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001) .await .unwrap(); @@ -1370,7 +1440,7 @@ mod tests { let store = make_store(&dir); let state = SessionState::default(); - open_project(project_dir.to_string_lossy().to_string(), &state, &store) + open_project(project_dir.to_string_lossy().to_string(), &state, &store, 3001) .await .unwrap(); @@ -1572,7 +1642,7 @@ mod tests { ) .unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/project.toml")).unwrap(); assert!( @@ -1592,7 +1662,7 @@ mod tests { #[test] fn scaffold_project_toml_fallback_when_no_stack_detected() { let dir = tempdir().unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(dir.path().join(".storkit/project.toml")).unwrap(); assert!( @@ -1614,7 +1684,7 @@ mod tests { let existing = "[[component]]\nname = \"custom\"\npath = \".\"\nsetup = [\"make build\"]\n"; fs::write(sk_dir.join("project.toml"), existing).unwrap(); - scaffold_story_kit(dir.path()).unwrap(); + scaffold_story_kit(dir.path(), 3001).unwrap(); let content = fs::read_to_string(sk_dir.join("project.toml")).unwrap(); assert_eq!( diff --git a/server/src/main.rs b/server/src/main.rs index a053685..f7b420d 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -142,6 +142,7 @@ async fn main() -> Result<(), std::io::Error> { explicit_root.to_string_lossy().to_string(), &app_state, store.as_ref(), + port, ) .await { @@ -164,6 +165,7 @@ async fn main() -> Result<(), std::io::Error> { project_root.to_string_lossy().to_string(), &app_state, store.as_ref(), + port, ) .await .unwrap_or_else(|e| {