From f80df3ba8783a402a35c62cf698cf15ac6485450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Mon, 17 Aug 2026 20:06:16 -0300 Subject: [PATCH 1/3] feat: add Flutter integration pack --- .../assets/skills/pickforge-flutter/SKILL.md | 26 +++ crates/pickforge-cli/src/adapters.rs | 171 ++++++++++++++++- crates/pickforge-cli/src/init.rs | 60 +++++- crates/pickforge-cli/src/main.rs | 8 +- crates/pickforge-cli/tests/cli.rs | 80 ++++++++ crates/pickforge-cli/tests/init.rs | 180 ++++++++++++++++-- docs/releases/UNRELEASED.md | 12 +- 7 files changed, 514 insertions(+), 23 deletions(-) create mode 100644 crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md diff --git a/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md b/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md new file mode 100644 index 0000000..f834891 --- /dev/null +++ b/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md @@ -0,0 +1,26 @@ +--- +name: pickforge-flutter +description: >- + Inspect, fix, hot reload, and verify Flutter runtime issues with the official + Dart and Flutter MCP server. +--- + +# Pickforge Flutter workflow + +1. Protect pre-existing work. Inspect the current Git state and relevant source + before changing anything. +2. Establish the runtime and static-analysis baseline with the official + Dart/Flutter MCP tools and resources. Map the observed widget/runtime behavior + back to its source. +3. Make the smallest source-only fix. Do not edit generated code or + configuration, install packages, create project artifacts, add + `flutter_driver`, or enable the driver extension. Ask first if any of those + are necessary. +4. Run scoped analysis and tests for the changed source, then use hot reload. +5. Repeat the same runtime scenario and capture before/after evidence. Hand off + the source mapping, change, checks, and observed result. + +If an MCP tool, resource, runtime, or hot-reload capability is unavailable, name +that exact capability and run `pickforge doctor`; never fabricate evidence. +Official Dart and Flutter skills are complementary to this workflow, but are not +installed by it. diff --git a/crates/pickforge-cli/src/adapters.rs b/crates/pickforge-cli/src/adapters.rs index eff52f3..f8e7661 100644 --- a/crates/pickforge-cli/src/adapters.rs +++ b/crates/pickforge-cli/src/adapters.rs @@ -8,7 +8,7 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use thiserror::Error; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum Harness { ClaudeCode, @@ -43,12 +43,42 @@ impl FromStr for Harness { } } +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct HarnessArgsSpec { + pub harness: Harness, + pub extra_args: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize)] #[serde(rename_all = "camelCase")] pub struct McpServerSpec { pub name: String, pub command: String, pub args: Vec, + pub harness_args: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum WorkflowRoot { + ClaudeSkills, + SharedAgentSkills, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowTargetSpec { + pub harness: Harness, + pub root: WorkflowRoot, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowSpec { + pub name: String, + pub content: Vec, + pub targets: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -57,6 +87,8 @@ pub struct IntegrationPack { pub name: String, pub version: u32, pub mcp_servers: Vec, + pub required_tools: Vec, + pub workflows: Vec, } impl IntegrationPack { @@ -65,6 +97,49 @@ impl IntegrationPack { name: "pickforge-base".into(), version: 1, mcp_servers: Vec::new(), + required_tools: Vec::new(), + workflows: Vec::new(), + } + } + + pub fn flutter() -> Self { + Self { + name: "pickforge-flutter".into(), + version: 1, + mcp_servers: vec![McpServerSpec { + name: "pickforge-dart".into(), + command: "dart".into(), + args: vec!["mcp-server".into()], + harness_args: vec![ + HarnessArgsSpec { + harness: Harness::Codex, + extra_args: vec!["--force-roots-fallback".into()], + }, + HarnessArgsSpec { + harness: Harness::Pi, + extra_args: vec!["--force-roots-fallback".into()], + }, + ], + }], + required_tools: vec!["dart".into()], + workflows: vec![WorkflowSpec { + name: "pickforge-flutter".into(), + content: include_bytes!("../assets/skills/pickforge-flutter/SKILL.md").to_vec(), + targets: vec![ + WorkflowTargetSpec { + harness: Harness::ClaudeCode, + root: WorkflowRoot::ClaudeSkills, + }, + WorkflowTargetSpec { + harness: Harness::Codex, + root: WorkflowRoot::SharedAgentSkills, + }, + WorkflowTargetSpec { + harness: Harness::Pi, + root: WorkflowRoot::SharedAgentSkills, + }, + ], + }], } } @@ -87,10 +162,57 @@ impl IntegrationPack { } if std::iter::once(&server.command) .chain(&server.args) + .chain( + server + .harness_args + .iter() + .flat_map(|policy| policy.extra_args.iter()), + ) .any(|value| value.chars().any(char::is_control)) { return Err(AdapterError::ControlCharacter(server.name.clone())); } + let mut harnesses = BTreeSet::new(); + for policy in &server.harness_args { + if !harnesses.insert(policy.harness) { + return Err(AdapterError::DuplicateHarnessPolicy(server.name.clone())); + } + if policy.extra_args.is_empty() { + return Err(AdapterError::InvalidHarnessPolicy(server.name.clone())); + } + } + } + let mut tools = BTreeSet::new(); + for tool in &self.required_tools { + if tool.is_empty() || tool.chars().any(char::is_control) || !tools.insert(tool) { + return Err(AdapterError::InvalidRequiredTool(tool.clone())); + } + } + let mut workflows = BTreeSet::new(); + for workflow in &self.workflows { + if !workflow.name.starts_with("pickforge-") + || !workflow + .name + .chars() + .all(|character| character.is_ascii_alphanumeric() || character == '-') + { + return Err(AdapterError::InvalidWorkflowName(workflow.name.clone())); + } + if !workflows.insert(&workflow.name) { + return Err(AdapterError::DuplicateWorkflowName(workflow.name.clone())); + } + if workflow.content.is_empty() || std::str::from_utf8(&workflow.content).is_err() { + return Err(AdapterError::InvalidWorkflowContent(workflow.name.clone())); + } + let mut harnesses = BTreeSet::new(); + for target in &workflow.targets { + if !harnesses.insert(target.harness) + || matches!(target.harness, Harness::ClaudeCode) + != matches!(target.root, WorkflowRoot::ClaudeSkills) + { + return Err(AdapterError::InvalidWorkflowTarget(workflow.name.clone())); + } + } } Ok(()) } @@ -106,6 +228,20 @@ pub enum AdapterError { EmptyServerCommand(String), #[error("MCP server command and arguments must not contain control characters: {0}")] ControlCharacter(String), + #[error("MCP server has duplicate per-harness argument policy: {0}")] + DuplicateHarnessPolicy(String), + #[error("MCP server per-harness argument policy must add at least one argument: {0}")] + InvalidHarnessPolicy(String), + #[error("required tool must be nonempty, unique, and contain no control characters: {0}")] + InvalidRequiredTool(String), + #[error("workflow name must begin with 'pickforge-' and contain only ASCII letters, digits, or '-': {0}")] + InvalidWorkflowName(String), + #[error("workflow name is duplicated: {0}")] + DuplicateWorkflowName(String), + #[error("workflow content must be nonempty UTF-8: {0}")] + InvalidWorkflowContent(String), + #[error("workflow has a duplicate or incompatible harness target: {0}")] + InvalidWorkflowTarget(String), #[error("{0} must contain a JSON object")] JsonObject(&'static str), #[error("{0} contains malformed JSON: {1}")] @@ -128,9 +264,22 @@ pub enum AdapterError { ManagedTableOutsideBlock(String), } +fn server_args(server: &McpServerSpec, harness: Harness) -> Vec { + let mut args = server.args.clone(); + if let Some(policy) = server + .harness_args + .iter() + .find(|policy| policy.harness == harness) + { + args.extend(policy.extra_args.clone()); + } + args +} + pub fn json_config( existing: Option<&str>, pack: &IntegrationPack, + harness: Harness, label: &'static str, ) -> Result>, AdapterError> { pack.validate()?; @@ -155,7 +304,7 @@ pub fn json_config( for server in &pack.mcp_servers { servers.insert( server.name.clone(), - serde_json::json!({"command": server.command, "args": server.args}), + serde_json::json!({"command": server.command, "args": server_args(server, harness)}), ); } let mut rendered = serde_json::to_vec_pretty(&root).expect("JSON values serialize"); @@ -271,8 +420,7 @@ pub fn codex_config( block.push(format!("command = {}", toml_string(&server.command))); block.push(format!( "args = [{}]", - server - .args + server_args(server, Harness::Codex) .iter() .map(|arg| toml_string(arg)) .collect::>() @@ -313,6 +461,21 @@ pub fn codex_config( Ok(Some(output.into_bytes())) } +pub(crate) fn workflow_target( + root: WorkflowRoot, + name: &str, + env: &crate::Environment, +) -> Result { + let home = env + .home_dir() + .ok_or_else(|| "no home directory could be resolved".to_string())?; + let skills = match root { + WorkflowRoot::ClaudeSkills => home.join(".claude").join("skills"), + WorkflowRoot::SharedAgentSkills => home.join(".agents").join("skills"), + }; + Ok(skills.join(name).join("SKILL.md")) +} + pub(crate) fn target_for(harness: Harness, env: &crate::Environment) -> Result { let home = || { env.home_dir() diff --git a/crates/pickforge-cli/src/init.rs b/crates/pickforge-cli/src/init.rs index 04e3bc4..c1a0d58 100644 --- a/crates/pickforge-cli/src/init.rs +++ b/crates/pickforge-cli/src/init.rs @@ -5,9 +5,9 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::adapters::{self, Harness, IntegrationPack}; +use crate::adapters::{self, Harness, IntegrationPack, WorkflowRoot}; use crate::transaction::{self, FilePlan}; -use crate::{project, state, Environment}; +use crate::{project, state, tools, Environment}; pub const INIT_SCHEMA_VERSION: u32 = 1; const MAX_STATE_ARTIFACT_BYTES: u64 = 1024 * 1024; @@ -263,6 +263,19 @@ pub fn plan_init(request: &InitRequest, env: &Environment) -> Result Result { - adapters::json_config(text, &request.pack, "Claude Code config") + Harness::ClaudeCode => adapters::json_config( + text, + &request.pack, + Harness::ClaudeCode, + "Claude Code config", + ), + Harness::Pi => { + adapters::json_config(text, &request.pack, Harness::Pi, "Pi MCP config") } - Harness::Pi => adapters::json_config(text, &request.pack, "Pi MCP config"), Harness::Codex => adapters::codex_config(text, &request.pack), }; match transformed { @@ -331,6 +349,38 @@ pub fn plan_init(request: &InitRequest, env: &Environment) -> Result path, + Err(error) => { + conflicts.push(error); + continue; + } + }; + match transaction::plan_file(target_path, workflow.content.clone(), true) { + Ok((file, _)) => { + actions.push(action( + file.path(), + &file, + format!("Install {} workflow", workflow.name), + vec![], + None, + )); + files.push(file); + } + Err(error) => conflicts.push(error.to_string()), + } + } + } + let project_path = canonical .to_str() .ok_or(project::ProjectIdentityError::NonUtf8Path)? diff --git a/crates/pickforge-cli/src/main.rs b/crates/pickforge-cli/src/main.rs index e2e9688..cc3405b 100644 --- a/crates/pickforge-cli/src/main.rs +++ b/crates/pickforge-cli/src/main.rs @@ -3,7 +3,7 @@ use std::process::ExitCode; use std::time::{SystemTime, UNIX_EPOCH}; use clap::{Parser, Subcommand}; -use pickforge_cli::adapters::Harness; +use pickforge_cli::adapters::{Harness, IntegrationPack}; use pickforge_cli::init::{ApplyState, InitRequest}; use pickforge_cli::{apply_init, diagnose, plan_init, render, Environment}; use serde::Serialize; @@ -39,6 +39,8 @@ enum Command { dry_run: bool, #[arg(long)] json: bool, + #[arg(long, hide = true)] + mobile_integration_alpha: bool, }, } @@ -90,11 +92,15 @@ fn main() -> ExitCode { harness, dry_run, json, + mobile_integration_alpha, } => { let project_dir = project_dir .or_else(|| std::env::current_dir().ok()) .unwrap_or_else(|| PathBuf::from(".")); let mut request = InitRequest::new(project_dir); + if mobile_integration_alpha { + request.pack = IntegrationPack::flutter(); + } if !harness.is_empty() { request.harnesses = harness .iter() diff --git a/crates/pickforge-cli/tests/cli.rs b/crates/pickforge-cli/tests/cli.rs index 06b6406..e9a0d20 100644 --- a/crates/pickforge-cli/tests/cli.rs +++ b/crates/pickforge-cli/tests/cli.rs @@ -38,6 +38,8 @@ fn pickforge(root: &Path, tools: &[&str]) -> Command { command .env_clear() .env("PATH", fake_bin(root, tools)) + .env("HOME", root.join("home")) + .env("USERPROFILE", root.join("home")) .env("PICKFORGE_HOME", root.join("state")); #[cfg(windows)] command.env("PATHEXT", ".EXE"); @@ -292,6 +294,84 @@ fn init_human_output_escapes_path_control_characters() { assert!(!stdout.contains(&unsafe_state.to_string_lossy().into_owned())); } +#[test] +fn hidden_mobile_alpha_flag_requires_but_never_executes_dart_and_honors_subset() { + let temp = TempDir::new().unwrap(); + let project_dir = flutter_project(temp.path()); + git(&project_dir, &["init", "--quiet"]); + git(&project_dir, &["add", "pubspec.yaml"]); + git( + &project_dir, + &[ + "-c", + "user.name=Pickforge Test", + "-c", + "user.email=test@invalid.example", + "commit", + "--quiet", + "-m", + "fixture", + ], + ); + let status_before = git(&project_dir, &["status", "--porcelain=v1"]); + let tree_before = snapshot_without_git(&project_dir); + let help = pickforge(temp.path(), &[]) + .args(["init", "--help"]) + .assert() + .success(); + assert!( + !String::from_utf8_lossy(&help.get_output().stdout).contains("mobile-integration-alpha") + ); + + let missing = pickforge(temp.path(), &[]) + .args(["init", "--mobile-integration-alpha", "--project-dir"]) + .arg(&project_dir) + .assert() + .code(1); + assert!(String::from_utf8_lossy(&missing.get_output().stdout).contains("requires dart on PATH")); + assert!(!temp.path().join("state").exists()); + assert!(!temp.path().join("home").exists()); + + let marker = temp.path().join("dart-ran"); + let bin = fake_bin(temp.path(), &["dart"]); + #[cfg(not(windows))] + { + let dart = bin.join("dart"); + std::fs::write(&dart, format!("#!/bin/sh\ntouch '{}'\n", marker.display())).unwrap(); + } + let output = pickforge(temp.path(), &[]) + .env("PATH", bin) + .args([ + "init", + "--mobile-integration-alpha", + "--harness", + "codex", + "--json", + "--project-dir", + ]) + .arg(&project_dir) + .assert() + .success(); + assert!(!marker.exists()); + let value: serde_json::Value = serde_json::from_slice(&output.get_output().stdout).unwrap(); + assert_eq!(value["plan"]["pack"]["name"], "pickforge-flutter"); + assert_eq!(value["plan"]["harnesses"], serde_json::json!(["codex"])); + assert_eq!(value["plan"]["actions"].as_array().unwrap().len(), 3); + assert!(temp.path().join("home/.codex/config.toml").is_file()); + assert!(temp + .path() + .join("home/.agents/skills/pickforge-flutter/SKILL.md") + .is_file()); + assert!(!temp.path().join("home/.claude.json").exists()); + assert!(!temp.path().join("home/.claude/skills").exists()); + assert!(!temp.path().join("home/.config/mcp/mcp.json").exists()); + assert_eq!( + git(&project_dir, &["status", "--porcelain=v1"]), + status_before + ); + assert_eq!(snapshot_without_git(&project_dir), tree_before); +} + #[test] fn init_precondition_failure_exits_one_without_writing() { let temp = TempDir::new().unwrap(); diff --git a/crates/pickforge-cli/tests/init.rs b/crates/pickforge-cli/tests/init.rs index 87da3c3..a066e22 100644 --- a/crates/pickforge-cli/tests/init.rs +++ b/crates/pickforge-cli/tests/init.rs @@ -2,7 +2,8 @@ use std::path::Path; use std::time::SystemTime; use pickforge_cli::adapters::{ - codex_config, json_config, AdapterError, Harness, IntegrationPack, McpServerSpec, + codex_config, json_config, AdapterError, Harness, HarnessArgsSpec, IntegrationPack, + McpServerSpec, }; #[cfg(windows)] use pickforge_cli::init::ActionKind; @@ -20,10 +21,29 @@ fn pack() -> IntegrationPack { name: "pickforge-helper".into(), command: "pickforge".into(), args: vec!["serve".into(), "a b".into()], + harness_args: vec![], }], + required_tools: vec![], + workflows: vec![], } } +fn fake_tool(root: &Path, name: &str) -> std::path::PathBuf { + let bin = root.join("bin"); + std::fs::create_dir_all(&bin).unwrap(); + #[cfg(windows)] + let tool = bin.join(format!("{name}.EXE")); + #[cfg(not(windows))] + let tool = bin.join(name); + std::fs::write(&tool, "not executed").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&tool, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + bin +} + fn fixture() -> (TempDir, std::path::PathBuf, Environment) { let temp = TempDir::new().unwrap(); let project = temp.path().join("app"); @@ -155,7 +175,9 @@ fn symlinked_home_and_state_roots_are_resolved_safely() { #[test] fn json_adapters_create_merge_validate_and_are_equivalent_and_idempotent() { - let created = json_config(None, &pack(), "config").unwrap().unwrap(); + let created = json_config(None, &pack(), Harness::ClaudeCode, "config") + .unwrap() + .unwrap(); let created_value: serde_json::Value = serde_json::from_slice(&created).unwrap(); assert_eq!( created_value["mcpServers"]["pickforge-helper"]["command"], @@ -163,10 +185,15 @@ fn json_adapters_create_merge_validate_and_are_equivalent_and_idempotent() { ); let input = r#"{"root":1,"mcpServers":{"foreign":{"command":"x"}}}"#; - let claude = json_config(Some(input), &pack(), "Claude Code config") - .unwrap() - .unwrap(); - let pi = json_config(Some(input), &pack(), "Pi MCP config") + let claude = json_config( + Some(input), + &pack(), + Harness::ClaudeCode, + "Claude Code config", + ) + .unwrap() + .unwrap(); + let pi = json_config(Some(input), &pack(), Harness::Pi, "Pi MCP config") .unwrap() .unwrap(); assert_eq!(claude, pi); @@ -174,6 +201,7 @@ fn json_adapters_create_merge_validate_and_are_equivalent_and_idempotent() { json_config( Some(std::str::from_utf8(&claude).unwrap()), &pack(), + Harness::ClaudeCode, "Claude Code config" ) .unwrap() @@ -183,16 +211,22 @@ fn json_adapters_create_merge_validate_and_are_equivalent_and_idempotent() { let value: serde_json::Value = serde_json::from_slice(&claude).unwrap(); assert_eq!(value["root"], 1); assert_eq!(value["mcpServers"]["foreign"]["command"], "x"); - assert!(json_config(Some("[]"), &pack(), "config").is_err()); - assert!(json_config(Some("{"), &pack(), "config").is_err()); - assert!(json_config(Some(r#"{"mcpServers":1}"#), &pack(), "config").is_err()); + assert!(json_config(Some("[]"), &pack(), Harness::ClaudeCode, "config").is_err()); + assert!(json_config(Some("{"), &pack(), Harness::ClaudeCode, "config").is_err()); + assert!(json_config( + Some(r#"{"mcpServers":1}"#), + &pack(), + Harness::ClaudeCode, + "config", + ) + .is_err()); let mut invalid = pack(); invalid.mcp_servers[0].name = "pickforge-invalid.name".into(); - assert!(json_config(None, &invalid, "config").is_err()); + assert!(json_config(None, &invalid, Harness::ClaudeCode, "config").is_err()); let mut duplicate = pack(); duplicate.mcp_servers.push(duplicate.mcp_servers[0].clone()); - assert!(json_config(None, &duplicate, "config").is_err()); + assert!(json_config(None, &duplicate, Harness::ClaudeCode, "config").is_err()); let mut control = pack(); control.mcp_servers[0].args.push("bad\u{7f}".into()); assert!(codex_config(None, &control).is_err()); @@ -247,6 +281,130 @@ fn codex_managed_block_creates_and_preserves_surroundings_and_newline_style() { assert!(mixed_output.ends_with("\n[other]\r\nx = 1\r\n")); } +#[test] +fn flutter_pack_plans_owned_harness_configs_and_deduplicated_workflows_in_order() { + let (temp, project, env) = fixture(); + let bin = fake_tool(temp.path(), "dart"); + let env = env.with_var("PATH", &bin).with_var("PATHEXT", ".EXE"); + let mut request = InitRequest::new(&project); + request.pack = IntegrationPack::flutter(); + + let before = std::fs::read(project.join("pubspec.yaml")).unwrap(); + let plan = plan_init(&request, &env).unwrap(); + assert_eq!(plan.report.pack.name, "pickforge-flutter"); + assert_eq!(plan.report.actions.len(), 6); + assert!(plan.report.actions[0].target.ends_with(".claude.json")); + assert!(plan.report.actions[1].target.ends_with("config.toml")); + assert!(plan.report.actions[2].target.ends_with("mcp.json")); + assert!(Path::new(&plan.report.actions[3].target) + .ends_with(Path::new(".claude/skills/pickforge-flutter/SKILL.md"))); + assert!(Path::new(&plan.report.actions[4].target) + .ends_with(Path::new(".agents/skills/pickforge-flutter/SKILL.md"))); + assert!(plan.report.actions[5].target.ends_with("project.json")); + assert!(plan.report.actions[2].warning.is_some()); + assert!(!temp.path().join("home").exists()); + assert_eq!(std::fs::read(project.join("pubspec.yaml")).unwrap(), before); + + assert!(apply_init(&plan, "flutter").changed); + let asset = include_bytes!("../assets/skills/pickforge-flutter/SKILL.md"); + assert_eq!( + std::fs::read( + temp.path() + .join("home/.claude/skills/pickforge-flutter/SKILL.md") + ) + .unwrap(), + asset + ); + assert_eq!( + std::fs::read( + temp.path() + .join("home/.agents/skills/pickforge-flutter/SKILL.md") + ) + .unwrap(), + asset + ); + let claude: serde_json::Value = + serde_json::from_slice(&std::fs::read(temp.path().join("home/.claude.json")).unwrap()) + .unwrap(); + assert_eq!( + claude["mcpServers"]["pickforge-dart"], + serde_json::json!({"command":"dart","args":["mcp-server"]}) + ); + let pi: serde_json::Value = serde_json::from_slice( + &std::fs::read(temp.path().join("home/.config/mcp/mcp.json")).unwrap(), + ) + .unwrap(); + assert_eq!( + pi["mcpServers"]["pickforge-dart"]["args"], + serde_json::json!(["mcp-server", "--force-roots-fallback"]) + ); + let codex = std::fs::read_to_string(temp.path().join("home/.codex/config.toml")).unwrap(); + assert!(codex.contains("args = [\"mcp-server\", \"--force-roots-fallback\"]")); + + let snapshots = plan + .report + .actions + .iter() + .map(|action| { + let path = std::path::PathBuf::from(&action.target); + ( + path.clone(), + std::fs::read(&path).unwrap(), + std::fs::metadata(&path).unwrap().modified().unwrap(), + ) + }) + .collect::>(); + let second = plan_init(&request, &env).unwrap(); + let outcome = apply_init(&second, "flutter-second"); + assert!(!outcome.changed); + assert!(outcome.backup_paths.is_empty()); + for (path, bytes, mtime) in snapshots { + assert_eq!(std::fs::read(&path).unwrap(), bytes); + assert_eq!(std::fs::metadata(path).unwrap().modified().unwrap(), mtime); + } +} + +#[test] +fn flutter_pack_requires_dart_only_when_a_harness_is_selected() { + let (temp, project, env) = fixture(); + let mut request = InitRequest::new(&project); + request.pack = IntegrationPack::flutter(); + let error = plan_init(&request, &env).unwrap_err().to_string(); + assert!(error.contains("requires dart on PATH"), "{error}"); + assert!(error.contains("pickforge doctor"), "{error}"); + assert!(!temp.path().join("state").exists()); + assert!(!temp.path().join("home").exists()); + + request.harnesses.clear(); + let plan = plan_init(&request, &env).unwrap(); + assert_eq!(plan.report.actions.len(), 1); +} + +#[test] +fn pack_validation_rejects_duplicate_and_empty_harness_policy() { + let mut pack = pack(); + pack.mcp_servers[0].harness_args = vec![ + HarnessArgsSpec { + harness: Harness::Pi, + extra_args: vec!["one".into()], + }, + HarnessArgsSpec { + harness: Harness::Pi, + extra_args: vec!["two".into()], + }, + ]; + assert!(matches!( + pack.validate(), + Err(AdapterError::DuplicateHarnessPolicy(_)) + )); + pack.mcp_servers[0].harness_args.truncate(1); + pack.mcp_servers[0].harness_args[0].extra_args.clear(); + assert!(matches!( + pack.validate(), + Err(AdapterError::InvalidHarnessPolicy(_)) + )); +} + #[test] fn planning_nonempty_pack_is_read_only_and_deduplicates_in_fixed_order() { let (temp, project, env) = fixture(); diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index e17b4d4..c88025c 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -25,6 +25,12 @@ GitHub release description, then reset it after the release is published. completed writes while retaining backups. There is deliberately no durable journal or daemon: a process interruption can leave a partially applied set; owned temporary/backup artifacts are recognized so a later rerun converges it. +- Added a default-off Flutter integration alpha for `pickforge init`. The hidden + `--mobile-integration-alpha` flag configures the owned `pickforge-dart` MCP + server for selected harnesses and installs one portable Flutter workflow skill + into Claude's skill root and/or the shared Codex/Pi agent-skill root. It + requires discoverable `dart` but never executes it; the default base pack is + unchanged. ## Internal/release changes @@ -49,8 +55,10 @@ GitHub release description, then reset it after the release is published. project/framework detection, tool and harness discovery, state and project-id boundaries, adapter preservation/refusal, transaction rollback and drift, dry-run, receipt ownership, file modes, idempotency, Git-tree cleanliness, - JSON/text safety, and CLI exits. The Windows MSVC target also passes - cross-target check and clippy; Windows-native tests run in the CI matrix. + JSON/text safety, CLI exits, owned Flutter MCP configuration, per-harness + arguments, workflow targeting/deduplication, and alpha tool preconditions. The + Windows MSVC target also passes cross-target check and clippy; Windows-native + tests run in the CI matrix. - Manual smoke runs of `pickforge doctor` and `pickforge doctor --json` against temporary fake Flutter and non-Flutter projects with an isolated `PATH`/`PICKFORGE_HOME`. From 822fbd7deed28c32ab75dbef0f9916a3b203a13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Mon, 17 Aug 2026 20:16:19 -0300 Subject: [PATCH 2/3] fix: protect Flutter workflow installation --- .../assets/skills/pickforge-flutter/SKILL.md | 2 + crates/pickforge-cli/src/adapters.rs | 15 +++ crates/pickforge-cli/src/init.rs | 29 +++- crates/pickforge-cli/tests/cli.rs | 50 +++---- crates/pickforge-cli/tests/init.rs | 126 +++++++++++++++++- docs/releases/UNRELEASED.md | 15 +-- 6 files changed, 203 insertions(+), 34 deletions(-) diff --git a/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md b/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md index f834891..96c95b3 100644 --- a/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md +++ b/crates/pickforge-cli/assets/skills/pickforge-flutter/SKILL.md @@ -5,6 +5,8 @@ description: >- Dart and Flutter MCP server. --- + + # Pickforge Flutter workflow 1. Protect pre-existing work. Inspect the current Git state and relevant source diff --git a/crates/pickforge-cli/src/adapters.rs b/crates/pickforge-cli/src/adapters.rs index f8e7661..d2732b3 100644 --- a/crates/pickforge-cli/src/adapters.rs +++ b/crates/pickforge-cli/src/adapters.rs @@ -78,6 +78,7 @@ pub struct WorkflowTargetSpec { pub struct WorkflowSpec { pub name: String, pub content: Vec, + pub ownership_marker: String, pub targets: Vec, } @@ -125,6 +126,7 @@ impl IntegrationPack { workflows: vec![WorkflowSpec { name: "pickforge-flutter".into(), content: include_bytes!("../assets/skills/pickforge-flutter/SKILL.md").to_vec(), + ownership_marker: "".into(), targets: vec![ WorkflowTargetSpec { harness: Harness::ClaudeCode, @@ -204,6 +206,17 @@ impl IntegrationPack { if workflow.content.is_empty() || std::str::from_utf8(&workflow.content).is_err() { return Err(AdapterError::InvalidWorkflowContent(workflow.name.clone())); } + if workflow.ownership_marker.is_empty() + || workflow.ownership_marker.chars().any(char::is_control) + || !workflow + .content + .windows(workflow.ownership_marker.len()) + .any(|window| window == workflow.ownership_marker.as_bytes()) + { + return Err(AdapterError::InvalidWorkflowOwnership( + workflow.name.clone(), + )); + } let mut harnesses = BTreeSet::new(); for target in &workflow.targets { if !harnesses.insert(target.harness) @@ -240,6 +253,8 @@ pub enum AdapterError { DuplicateWorkflowName(String), #[error("workflow content must be nonempty UTF-8: {0}")] InvalidWorkflowContent(String), + #[error("workflow content must contain its nonempty, control-free ownership marker: {0}")] + InvalidWorkflowOwnership(String), #[error("workflow has a duplicate or incompatible harness target: {0}")] InvalidWorkflowTarget(String), #[error("{0} must contain a JSON object")] diff --git a/crates/pickforge-cli/src/init.rs b/crates/pickforge-cli/src/init.rs index c1a0d58..01d4c05 100644 --- a/crates/pickforge-cli/src/init.rs +++ b/crates/pickforge-cli/src/init.rs @@ -1,5 +1,6 @@ //! Read-only init planning and transactional orchestration. +use std::collections::BTreeSet; use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -366,7 +367,31 @@ pub fn plan_init(request: &InitRequest, env: &Environment) -> Result { + Ok((file, existing)) => { + let foreign_content = existing.as_ref().is_some_and(|content| { + content != &workflow.content + && !content + .windows(workflow.ownership_marker.len()) + .any(|window| window == workflow.ownership_marker.as_bytes()) + }); + if foreign_content { + conflicts.push(format!( + "{} contains a workflow not managed by Pickforge; move or remove it first", + file.path().display() + )); + continue; + } + if let Some(planned) = + files.iter().find(|planned| planned.path() == file.path()) + { + if planned.desired != file.desired { + conflicts.push(format!( + "workflow targets resolve to {} with different contents", + file.path().display() + )); + } + continue; + } actions.push(action( file.path(), &file, @@ -436,6 +461,8 @@ pub fn plan_init(request: &InitRequest, env: &Environment) -> Result conflicts.push(error.to_string()), } if !conflicts.is_empty() { + let mut unique = BTreeSet::new(); + conflicts.retain(|conflict| unique.insert(conflict.clone())); return Err(InitError::Conflicts(conflicts.join("\n"))); } diff --git a/crates/pickforge-cli/tests/cli.rs b/crates/pickforge-cli/tests/cli.rs index e9a0d20..c2acaca 100644 --- a/crates/pickforge-cli/tests/cli.rs +++ b/crates/pickforge-cli/tests/cli.rs @@ -295,26 +295,9 @@ fn init_human_output_escapes_path_control_characters() { } #[test] -fn hidden_mobile_alpha_flag_requires_but_never_executes_dart_and_honors_subset() { +fn mobile_alpha_flag_is_hidden_and_missing_dart_fails_without_writing() { let temp = TempDir::new().unwrap(); let project_dir = flutter_project(temp.path()); - git(&project_dir, &["init", "--quiet"]); - git(&project_dir, &["add", "pubspec.yaml"]); - git( - &project_dir, - &[ - "-c", - "user.name=Pickforge Test", - "-c", - "user.email=test@invalid.example", - "commit", - "--quiet", - "-m", - "fixture", - ], - ); - let status_before = git(&project_dir, &["status", "--porcelain=v1"]); - let tree_before = snapshot_without_git(&project_dir); let help = pickforge(temp.path(), &[]) .args(["init", "--help"]) .assert() @@ -331,14 +314,35 @@ fn hidden_mobile_alpha_flag_requires_but_never_executes_dart_and_honors_subset() assert!(String::from_utf8_lossy(&missing.get_output().stdout).contains("requires dart on PATH")); assert!(!temp.path().join("state").exists()); assert!(!temp.path().join("home").exists()); +} +#[cfg(unix)] +#[test] +fn mobile_alpha_never_executes_dart_and_keeps_the_project_clean() { + let temp = TempDir::new().unwrap(); + let project_dir = flutter_project(temp.path()); + git(&project_dir, &["init", "--quiet"]); + git(&project_dir, &["add", "pubspec.yaml"]); + git( + &project_dir, + &[ + "-c", + "user.name=Pickforge Test", + "-c", + "user.email=test@invalid.example", + "commit", + "--quiet", + "-m", + "fixture", + ], + ); + let status_before = git(&project_dir, &["status", "--porcelain=v1"]); + let tree_before = snapshot_without_git(&project_dir); let marker = temp.path().join("dart-ran"); let bin = fake_bin(temp.path(), &["dart"]); - #[cfg(not(windows))] - { - let dart = bin.join("dart"); - std::fs::write(&dart, format!("#!/bin/sh\ntouch '{}'\n", marker.display())).unwrap(); - } + let dart = bin.join("dart"); + std::fs::write(&dart, format!("#!/bin/sh\ntouch '{}'\n", marker.display())).unwrap(); + let output = pickforge(temp.path(), &[]) .env("PATH", bin) .args([ diff --git a/crates/pickforge-cli/tests/init.rs b/crates/pickforge-cli/tests/init.rs index a066e22..c858a76 100644 --- a/crates/pickforge-cli/tests/init.rs +++ b/crates/pickforge-cli/tests/init.rs @@ -3,7 +3,7 @@ use std::time::SystemTime; use pickforge_cli::adapters::{ codex_config, json_config, AdapterError, Harness, HarnessArgsSpec, IntegrationPack, - McpServerSpec, + McpServerSpec, WorkflowRoot, }; #[cfg(windows)] use pickforge_cli::init::ActionKind; @@ -381,7 +381,7 @@ fn flutter_pack_requires_dart_only_when_a_harness_is_selected() { } #[test] -fn pack_validation_rejects_duplicate_and_empty_harness_policy() { +fn pack_validation_rejects_invalid_harness_tool_and_workflow_policy() { let mut pack = pack(); pack.mcp_servers[0].harness_args = vec![ HarnessArgsSpec { @@ -403,6 +403,128 @@ fn pack_validation_rejects_duplicate_and_empty_harness_policy() { pack.validate(), Err(AdapterError::InvalidHarnessPolicy(_)) )); + + let mut invalid = IntegrationPack::flutter(); + invalid.required_tools = vec!["".into()]; + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidRequiredTool(_)) + )); + let mut invalid = IntegrationPack::flutter(); + invalid.workflows[0].name = "../foreign".into(); + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidWorkflowName(_)) + )); + let mut invalid = IntegrationPack::flutter(); + invalid.workflows.push(invalid.workflows[0].clone()); + assert!(matches!( + invalid.validate(), + Err(AdapterError::DuplicateWorkflowName(_)) + )); + let mut invalid = IntegrationPack::flutter(); + invalid.workflows[0].content.clear(); + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidWorkflowContent(_)) + )); + let mut invalid = IntegrationPack::flutter(); + invalid.workflows[0].ownership_marker = "foreign".into(); + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidWorkflowOwnership(_)) + )); + let mut invalid = IntegrationPack::flutter(); + invalid.workflows[0].targets[0].root = WorkflowRoot::SharedAgentSkills; + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidWorkflowTarget(_)) + )); +} + +#[test] +fn foreign_workflow_is_refused_while_owned_updates_receive_backups() { + let (temp, project, env) = fixture(); + let bin = fake_tool(temp.path(), "dart"); + let env = env.with_var("PATH", &bin).with_var("PATHEXT", ".EXE"); + let mut request = InitRequest::new(&project); + request.pack = IntegrationPack::flutter(); + request.harnesses = vec![Harness::Codex]; + let workflow = temp + .path() + .join("home/.agents/skills/pickforge-flutter/SKILL.md"); + std::fs::create_dir_all(workflow.parent().unwrap()).unwrap(); + std::fs::write(&workflow, "user-owned\n").unwrap(); + + let error = plan_init(&request, &env).unwrap_err().to_string(); + assert!(error.contains("not managed by Pickforge"), "{error}"); + assert_eq!(std::fs::read_to_string(&workflow).unwrap(), "user-owned\n"); + assert!(!temp.path().join("home/.codex/config.toml").exists()); + assert!(!temp.path().join("state").exists()); + + let owned = "\nold\n"; + std::fs::write(&workflow, owned).unwrap(); + let plan = plan_init(&request, &env).unwrap(); + let outcome = apply_init(&plan, "owned-update"); + assert_eq!(outcome.outcome, ApplyState::Success); + assert!(outcome + .backup_paths + .iter() + .any(|path| path.ends_with("SKILL.md.pickforge-backup-owned-update"))); + assert_eq!( + std::fs::read(&workflow).unwrap(), + include_bytes!("../assets/skills/pickforge-flutter/SKILL.md") + ); +} + +#[cfg(unix)] +#[test] +fn physically_shared_workflow_roots_are_planned_and_written_once() { + use std::os::unix::fs::symlink; + + let (temp, project, env) = fixture(); + let bin = fake_tool(temp.path(), "dart"); + let env = env.with_var("PATH", &bin); + let claude_skills = temp.path().join("home/.claude/skills"); + std::fs::create_dir_all(&claude_skills).unwrap(); + std::fs::create_dir_all(temp.path().join("home/.agents")).unwrap(); + symlink(&claude_skills, temp.path().join("home/.agents/skills")).unwrap(); + let mut request = InitRequest::new(&project); + request.pack = IntegrationPack::flutter(); + request.harnesses = vec![Harness::ClaudeCode, Harness::Codex]; + + let plan = plan_init(&request, &env).unwrap(); + let workflow_actions = plan + .report + .actions + .iter() + .filter(|action| action.summary.contains("workflow")) + .count(); + assert_eq!(workflow_actions, 1); + assert_eq!( + apply_init(&plan, "shared-root").outcome, + ApplyState::Success + ); + assert!(claude_skills.join("pickforge-flutter/SKILL.md").is_file()); +} + +#[test] +fn repeated_missing_home_conflicts_are_reported_once() { + let (temp, project, _) = fixture(); + let bin = fake_tool(temp.path(), "dart"); + let env = Environment::empty() + .with_var("PATH", &bin) + .with_var("PATHEXT", ".EXE") + .with_var("PICKFORGE_HOME", temp.path().join("state")); + let mut request = InitRequest::new(&project); + request.pack = IntegrationPack::flutter(); + + let error = plan_init(&request, &env).unwrap_err().to_string(); + assert_eq!( + error.matches("no home directory could be resolved").count(), + 1 + ); + assert!(!temp.path().join("state").exists()); } #[test] diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index c88025c..2b284cd 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -25,15 +25,14 @@ GitHub release description, then reset it after the release is published. completed writes while retaining backups. There is deliberately no durable journal or daemon: a process interruption can leave a partially applied set; owned temporary/backup artifacts are recognized so a later rerun converges it. -- Added a default-off Flutter integration alpha for `pickforge init`. The hidden - `--mobile-integration-alpha` flag configures the owned `pickforge-dart` MCP - server for selected harnesses and installs one portable Flutter workflow skill - into Claude's skill root and/or the shared Codex/Pi agent-skill root. It - requires discoverable `dart` but never executes it; the default base pack is - unchanged. - ## Internal/release changes +- Added an unpublished, default-off Flutter integration alpha for `pickforge + init`. The hidden `--mobile-integration-alpha` flag configures the owned + `pickforge-dart` MCP server for selected harnesses and installs one portable + Flutter workflow skill into Claude's skill root and/or the shared Codex/Pi + agent-skill root. It requires discoverable `dart` but never executes it; the + default base pack and every release surface remain unchanged. - Raised the vulnerable `fast-uri` and `hono` overrides, plus lockfile resolutions for both `brace-expansion` majors, `fast-uri`, `hono`, `ip-address`, and `nanoid`, to patched releases. @@ -51,7 +50,7 @@ GitHub release description, then reset it after the release is published. one skips, coverage passes at 82.48% lines, and build passes. - The pinned OSV Scanner v2.3.8 image reports no unfiltered advisories. - `cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D - warnings`, and `cargo test --workspace --locked` pass with 56 tests covering + warnings`, and `cargo test --workspace --locked` pass with 64 tests covering project/framework detection, tool and harness discovery, state and project-id boundaries, adapter preservation/refusal, transaction rollback and drift, dry-run, receipt ownership, file modes, idempotency, Git-tree cleanliness, From 1acb945af0c2df344a54cfe59911ac64603b5902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Mon, 17 Aug 2026 20:20:07 -0300 Subject: [PATCH 3/3] test: cover workflow target conflicts --- crates/pickforge-cli/tests/init.rs | 65 +++++++++++++++++++++++++++--- docs/releases/UNRELEASED.md | 2 +- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/crates/pickforge-cli/tests/init.rs b/crates/pickforge-cli/tests/init.rs index c858a76..dcd0ec9 100644 --- a/crates/pickforge-cli/tests/init.rs +++ b/crates/pickforge-cli/tests/init.rs @@ -5,6 +5,8 @@ use pickforge_cli::adapters::{ codex_config, json_config, AdapterError, Harness, HarnessArgsSpec, IntegrationPack, McpServerSpec, WorkflowRoot, }; +#[cfg(unix)] +use pickforge_cli::adapters::{WorkflowSpec, WorkflowTargetSpec}; #[cfg(windows)] use pickforge_cli::init::ActionKind; use pickforge_cli::init::{ApplyReport, ApplyState}; @@ -428,12 +430,14 @@ fn pack_validation_rejects_invalid_harness_tool_and_workflow_policy() { invalid.validate(), Err(AdapterError::InvalidWorkflowContent(_)) )); - let mut invalid = IntegrationPack::flutter(); - invalid.workflows[0].ownership_marker = "foreign".into(); - assert!(matches!( - invalid.validate(), - Err(AdapterError::InvalidWorkflowOwnership(_)) - )); + for marker in ["", "bad\nmarker", "foreign"] { + let mut invalid = IntegrationPack::flutter(); + invalid.workflows[0].ownership_marker = marker.into(); + assert!(matches!( + invalid.validate(), + Err(AdapterError::InvalidWorkflowOwnership(_)) + )); + } let mut invalid = IntegrationPack::flutter(); invalid.workflows[0].targets[0].root = WorkflowRoot::SharedAgentSkills; assert!(matches!( @@ -508,6 +512,55 @@ fn physically_shared_workflow_roots_are_planned_and_written_once() { assert!(claude_skills.join("pickforge-flutter/SKILL.md").is_file()); } +#[cfg(unix)] +#[test] +fn physically_shared_workflow_targets_with_different_contents_conflict() { + use std::os::unix::fs::symlink; + + let (temp, project, env) = fixture(); + let first_dir = temp.path().join("home/.claude/skills/pickforge-first"); + std::fs::create_dir_all(&first_dir).unwrap(); + let shared_skills = temp.path().join("home/.agents/skills"); + std::fs::create_dir_all(&shared_skills).unwrap(); + symlink(&first_dir, shared_skills.join("pickforge-second")).unwrap(); + let mut request = InitRequest::new(&project); + request.harnesses = vec![Harness::ClaudeCode, Harness::Codex]; + request.pack = IntegrationPack { + name: "fixture".into(), + version: 1, + mcp_servers: vec![], + required_tools: vec![], + workflows: vec![ + WorkflowSpec { + name: "pickforge-first".into(), + content: b"\nfirst\n".to_vec(), + ownership_marker: "".into(), + targets: vec![WorkflowTargetSpec { + harness: Harness::ClaudeCode, + root: WorkflowRoot::ClaudeSkills, + }], + }, + WorkflowSpec { + name: "pickforge-second".into(), + content: b"\nsecond\n".to_vec(), + ownership_marker: "".into(), + targets: vec![WorkflowTargetSpec { + harness: Harness::Codex, + root: WorkflowRoot::SharedAgentSkills, + }], + }, + ], + }; + + let error = plan_init(&request, &env).unwrap_err().to_string(); + assert!( + error.contains("workflow targets resolve to") && error.contains("with different contents"), + "{error}" + ); + assert!(!first_dir.join("SKILL.md").exists()); + assert!(!temp.path().join("state").exists()); +} + #[test] fn repeated_missing_home_conflicts_are_reported_once() { let (temp, project, _) = fixture(); diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index 2b284cd..837a42a 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -50,7 +50,7 @@ GitHub release description, then reset it after the release is published. one skips, coverage passes at 82.48% lines, and build passes. - The pinned OSV Scanner v2.3.8 image reports no unfiltered advisories. - `cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D - warnings`, and `cargo test --workspace --locked` pass with 64 tests covering + warnings`, and `cargo test --workspace --locked` pass with 65 tests covering project/framework detection, tool and harness discovery, state and project-id boundaries, adapter preservation/refusal, transaction rollback and drift, dry-run, receipt ownership, file modes, idempotency, Git-tree cleanliness,