diff --git a/.gitignore b/.gitignore index 5918d39..deeb679 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,10 @@ result worktrees/ .direnv/ + +# Test artifacts from e2e agent tests +solo-*.txt +e2e-*.txt +skill-*.txt +pair-*.txt +todo-run-cmd-*.md diff --git a/README.md b/README.md index a1047a9..8702465 100644 --- a/README.md +++ b/README.md @@ -2,26 +2,27 @@ Step-by-step coordinator for multi-agent coding sessions. -## Command Wrappers +## Skill Wrappers Rally now provides a wrapper-oriented command surface: -- `rally command install --target ` -- `rally command run ...` (canonical wrapper entrypoint) -- `rally command exec ...` (alias for `run`) -- `rally command doctor --target ...` -- `rally command uninstall --target ...` +- `rally skill install --target ` +- `rally skill run ...` (canonical wrapper entrypoint) +- `rally skill exec ...` (alias for `run`) +- `rally skill agent-howto ...` (prints the current detailed agent loop instructions) +- `rally skill doctor --target ...` +- `rally skill uninstall --target ...` Install wrappers everywhere: ```bash -rally command install --target all +rally skill install --target all ``` Installed `/rally` wrappers are thin adapters that delegate back to Rally core: ```bash -rally command run "$@" +rally skill agent-howto "$@" ``` ## /rally Usage @@ -33,7 +34,7 @@ Typical wrapper invocations: - `/rally build reviewer` - `/rally ` -`rally command run` resolves context in this order: +`rally skill run` resolves context in this order: 1. explicit args (`--session`, `--as`, positional selectors) 2. invite token input @@ -45,7 +46,7 @@ Typical wrapper invocations: Use doctor to inspect path/status per target: ```bash -rally command doctor --target all +rally skill doctor --target all ``` -For details and target path mapping, see [docs/command-install-run.md](docs/command-install-run.md). +For details and target path mapping, see [docs/skill-install-run.md](docs/skill-install-run.md). diff --git a/docs/manual-qa-command-install-run.md b/docs/manual-qa-skill-install-run.md similarity index 55% rename from docs/manual-qa-command-install-run.md rename to docs/manual-qa-skill-install-run.md index c534df8..3154a26 100644 --- a/docs/manual-qa-command-install-run.md +++ b/docs/manual-qa-skill-install-run.md @@ -1,11 +1,11 @@ -# Manual QA: Command Install/Run +# Manual QA: Skill Install/Run Date: 2026-03-02 Environment: - Rally binary: `/Users/justin/code/rally/target/debug/rally` -- Workspace: `/Users/justin/code/rally/worktrees/command-install-run` +- Workspace: `/Users/justin/code/rally/worktrees/skill-install-run` - Temporary HOME used for isolated QA harness roots ## Scenario 1: Install wrappers for two targets @@ -13,35 +13,35 @@ Environment: Commands: ```bash -HOME="$TMP_HOME" rally command install --target codex -HOME="$TMP_HOME" rally command install --target droid +HOME="$TMP_HOME" rally skill install --target codex +HOME="$TMP_HOME" rally skill install --target droid ``` Observed: -- `~/.codex/commands/rally.md` created -- `~/.droid/commands/rally.md` created +- `~/.codex/skills/rally/SKILL.md` created +- `~/.factory/skills/rally/SKILL.md` created ## Scenario 2: `/rally` routing behavior -Created implement session `qa-command-install-run-1772430801` with a single step. +Created implement session `qa-skill-install-run-1772430801` with a single step. Commands and observed results: ```bash -HOME="$TMP_HOME" rally command run --session "$SESSION" --as implementer --non-interactive +HOME="$TMP_HOME" rally skill run --session "$SESSION" --as implementer --non-interactive ``` - Routed to implement instruction (`Implement step 1: Validate wrapper routing`) ```bash -HOME="$TMP_HOME" rally command run build --non-interactive +HOME="$TMP_HOME" rally skill run build --non-interactive ``` - Routed through saved context fallback and printed same instruction ```bash -HOME="$TMP_HOME" rally command run "rly:${SESSION}:implementer" --non-interactive +HOME="$TMP_HOME" rally skill run "rly:${SESSION}:implementer" --non-interactive ``` - Invite-token resolution worked and printed same instruction @@ -51,19 +51,19 @@ HOME="$TMP_HOME" rally command run "rly:${SESSION}:implementer" --non-interactiv Doctor output validated per-target status/path reporting: ```bash -HOME="$TMP_HOME" rally command doctor --target all +HOME="$TMP_HOME" rally skill doctor --target all ``` Created unmanaged custom file: ```bash -echo "custom user wrapper" > "$TMP_HOME/.pi/commands/rally.md" +echo "custom user wrapper" > "$TMP_HOME/.pi/agent/skills/rally/SKILL.md" ``` Confirmed non-destructive install behavior: ```bash -HOME="$TMP_HOME" rally command install --target pi +HOME="$TMP_HOME" rally skill install --target pi ``` - Failed as expected without `--force` and did not overwrite custom file @@ -71,19 +71,19 @@ HOME="$TMP_HOME" rally command install --target pi Verified uninstall behavior: ```bash -HOME="$TMP_HOME" rally command uninstall --target all --dry-run -HOME="$TMP_HOME" rally command uninstall --target all +HOME="$TMP_HOME" rally skill uninstall --target all --dry-run +HOME="$TMP_HOME" rally skill uninstall --target all ``` - Managed codex/droid wrappers removed -- Unmanaged `~/.pi/commands/rally.md` preserved +- Unmanaged `~/.pi/agent/skills/rally/SKILL.md` preserved Reinstall check: ```bash -HOME="$TMP_HOME" rally command install --target codex -HOME="$TMP_HOME" rally command install --target droid -HOME="$TMP_HOME" rally command doctor --target all --dry-run +HOME="$TMP_HOME" rally skill install --target codex +HOME="$TMP_HOME" rally skill install --target droid +HOME="$TMP_HOME" rally skill doctor --target all --dry-run ``` - Managed wrappers recreated cleanly diff --git a/docs/command-install-run.md b/docs/skill-install-run.md similarity index 56% rename from docs/command-install-run.md rename to docs/skill-install-run.md index 0fc989e..e9f1ed4 100644 --- a/docs/command-install-run.md +++ b/docs/skill-install-run.md @@ -1,4 +1,4 @@ -# Rally Command Install/Run +# Rally Skill Install/Run This document describes Rally's wrapper entrypoint for harness slash commands. @@ -7,27 +7,26 @@ This document describes Rally's wrapper entrypoint for harness slash commands. Install wrapper artifacts for one harness: ```bash -rally command install --target codex +rally skill install --target codex ``` Install for all supported harnesses: ```bash -rally command install --target all +rally skill install --target all ``` Supported targets: -- `codex` -> `~/.codex/commands/rally.md` -- `droid` -> `~/.droid/commands/rally.md` -- `pi` -> `~/.pi/commands/rally.md` -- `claude-code` -> `~/.claude/commands/rally.md` -- `factory` -> `~/.factory/commands/rally.md` +- `codex` -> `~/.codex/skills/rally/SKILL.md` +- `droid` -> `~/.factory/skills/rally/SKILL.md` +- `pi` -> `~/.pi/agent/skills/rally/SKILL.md` +- `claude-code` -> `~/.claude/skills/rally/SKILL.md` Each wrapper is Rally-managed content and delegates back to: ```bash -rally command run "$@" +rally skill agent-howto "$@" ``` ## Wrapper Behavior @@ -35,7 +34,8 @@ rally command run "$@" The generated `/rally` wrapper is intentionally thin: - accepts wrapper arguments unchanged -- forwards arguments to `rally command run` +- forwards arguments to `rally skill agent-howto` +- keeps the mutable prompt in Rally source code (`prompts/skill-agent-howto.md`) - keeps orchestration logic centralized in Rally Examples: @@ -45,28 +45,38 @@ Examples: - `/rally build reviewer` - `/rally ` -## `rally command run` Patterns +## `rally skill` Patterns + +`agent-howto` prints the current instruction loop and exact follow-up run command: + +```bash +rally skill agent-howto +rally skill agent-howto implement todos/create-todo.md implement +``` `run` and `exec` are equivalent: ```bash -rally command run -rally command exec +rally skill run +rally skill exec ``` Resolution precedence: 1. explicit args (`--session`, `--as`, positional selectors) 2. invite token (for example `rly::`, `rally:///`) -3. saved workspace context (`~/.rally/command-context.json`) +3. saved workspace context (`~/.rally/skill-context.json`) 4. interactive fallback prompt +Backward compatibility note: +- If `skill-context.json` is missing, Rally falls back to reading legacy `command-context.json`. + ## Troubleshooting with Doctor Inspect current installation health: ```bash -rally command doctor --target all +rally skill doctor --target all ``` Doctor reports per target: @@ -79,7 +89,7 @@ Doctor reports per target: For preview mode: ```bash -rally command doctor --target all --dry-run +rally skill doctor --target all --dry-run ``` ## Uninstall Behavior @@ -87,7 +97,7 @@ rally command doctor --target all --dry-run Remove Rally-managed wrappers only: ```bash -rally command uninstall --target all +rally skill uninstall --target all ``` Unmanaged user files are never removed. @@ -95,12 +105,12 @@ Unmanaged user files are never removed. Preview removal decisions: ```bash -rally command uninstall --target all --dry-run +rally skill uninstall --target all --dry-run ``` ## Mapping to Rally Core Commands -Wrappers map to Rally core loop behavior through `rally command run`, which then joins/polls the existing session workflows: +Wrappers map to Rally core loop behavior through `rally skill run`, which then joins/polls existing session workflows: - registration path: `rally join --session --as ` (as needed) - instruction polling path: `rally next --session --as ` diff --git a/flake.nix b/flake.nix index 78046d2..0a62180 100644 --- a/flake.nix +++ b/flake.nix @@ -93,6 +93,7 @@ pkgs.just pkgs.git pkgs.jq + pkgs.python3 ]; RUST_SRC_PATH = "${rustToolchain}/lib/rustlib/src/rust/library"; diff --git a/justfile b/justfile index 97f707e..b1484f4 100644 --- a/justfile +++ b/justfile @@ -12,9 +12,11 @@ pre-merge: nix-check test-agents: ./tests/e2e-agents.sh -# Test full 2-agent implement protocol end-to-end -test-implement: - ./tests/e2e-implement.sh +# Run the full e2e agent test matrix (solo + pairs, concurrent) +test-matrix *ARGS: + cargo install --path . --quiet + rally skill install --target all --force + python3 tests/e2e_matrix.py {{ARGS}} # Pick a todo and print copy-pasteable prompts for implement session agents implement: diff --git a/prompts/skill-agent-howto.md b/prompts/skill-agent-howto.md new file mode 100644 index 0000000..7d22bd0 --- /dev/null +++ b/prompts/skill-agent-howto.md @@ -0,0 +1,20 @@ +# Rally Agent How-To + +IMPORTANT: You MUST run bash commands to participate in this session. Do NOT try to do the work without running rally commands first. + +FIRST, run this bash command immediately: + +```bash +{{RUN_COMMAND}} +``` + +This will print your instruction. Then follow this loop: + +1. READ the instruction printed by the command above. +2. DO exactly what it says - write files, run tests, git commit, etc. +3. RUN the follow-up commands listed at the end of your instruction (checkpoint, review, etc). Run them ALL, in order. +4. RUN `{{RALLY_BIN}} skill run` again with no arguments to get your next instruction. +5. REPEAT from step 1. Do NOT stop until the output says "session complete; stop working". + +If the command prints "Waiting for ..." - keep waiting. Another agent needs to act first. +If the command exits with code 2 or prints "session complete" - you are done. Stop. diff --git a/prompts/skill-wrapper.md b/prompts/skill-wrapper.md new file mode 100644 index 0000000..0425c1e --- /dev/null +++ b/prompts/skill-wrapper.md @@ -0,0 +1,15 @@ +--- +name: rally +description: "Run rally multi-agent coding sessions. Use when the user invokes /rally or asks to run a rally workflow." +rally-managed: true +--- + +# Rally + +Run this command first with the user's arguments: + +```bash +{{RALLY_BIN}} skill agent-howto $ARGUMENTS +``` + +Then follow the printed instructions exactly. diff --git a/solo-claude-1772595331697.txt b/solo-claude-1772595331697.txt new file mode 100644 index 0000000..22a8e2d --- /dev/null +++ b/solo-claude-1772595331697.txt @@ -0,0 +1 @@ +hello claude diff --git a/solo-codex-1772595331697.txt b/solo-codex-1772595331697.txt new file mode 100644 index 0000000..50dd156 --- /dev/null +++ b/solo-codex-1772595331697.txt @@ -0,0 +1 @@ +hello codex \ No newline at end of file diff --git a/solo-pi-1772595331697.txt b/solo-pi-1772595331697.txt new file mode 100644 index 0000000..ecc5c1a --- /dev/null +++ b/solo-pi-1772595331697.txt @@ -0,0 +1 @@ +hello pi \ No newline at end of file diff --git a/src/cli.rs b/src/cli.rs index f558e7f..c653216 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -24,7 +24,7 @@ COMMAND GROUPS: Core loop: create, join, next, done, status, sessions Canonical built-ins: plan ..., build ... Generic workflows: workflow list, workflow action - Wrapper surface: command install|run|exec|doctor|uninstall + Wrapper surface: skill install|run|exec|agent-howto|doctor|uninstall Compatibility aliases (deprecated): file-issue, challenge, agree, checkpoint, review Handoff: chain @@ -80,8 +80,11 @@ pub enum Command { Build(BuildCommandArgs), #[command(about = "Generic workflow commands")] Workflow(WorkflowCommandArgs), - #[command(about = "Install and run short `/rally ...` wrapper commands")] - Command(CommandCommandArgs), + #[command( + about = "Install and run short `/rally ...` wrapper commands", + alias = "command" + )] + Skill(SkillCommandArgs), } #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] @@ -294,45 +297,49 @@ pub struct WorkflowActionArgs { } #[derive(Args, Debug)] -pub struct CommandCommandArgs { +pub struct SkillCommandArgs { #[command(subcommand)] - pub command: CommandSubcommand, + pub command: SkillSubcommand, } #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] -pub enum CommandTargetArg { +pub enum SkillTargetArg { Codex, Droid, Pi, ClaudeCode, - Factory, All, } #[derive(Subcommand, Debug)] -pub enum CommandSubcommand { +pub enum SkillSubcommand { #[command(about = "Install Rally-managed wrapper artifacts")] - Install(CommandInstallArgs), + Install(SkillInstallArgs), #[command( about = "Wrapper execution entrypoint", - long_about = "Wrapper execution entrypoint.\n\nArgument contract:\n rally command run\n rally command run \n rally command run \n\nResolution precedence (implemented by this command family):\n 1) explicit args\n 2) invite token\n 3) saved context\n 4) interactive fallback" + long_about = "Wrapper execution entrypoint.\n\nArgument contract:\n rally skill run\n rally skill run \n rally skill run \n\nResolution precedence (implemented by this command family):\n 1) explicit args\n 2) invite token\n 3) saved context\n 4) interactive fallback" )] - Run(CommandRunArgs), + Run(SkillRunArgs), #[command( about = "Alias for `run` (identical behavior and argument contract)", - long_about = "Alias for `run` with identical behavior.\n\nArgument contract:\n rally command exec\n rally command exec \n rally command exec \n\nResolution precedence (implemented by this command family):\n 1) explicit args\n 2) invite token\n 3) saved context\n 4) interactive fallback" + long_about = "Alias for `run` with identical behavior.\n\nArgument contract:\n rally skill exec\n rally skill exec \n rally skill exec \n\nResolution precedence (implemented by this command family):\n 1) explicit args\n 2) invite token\n 3) saved context\n 4) interactive fallback" + )] + Exec(SkillRunArgs), + #[command( + about = "Print the latest Rally agent instructions for `/rally` wrappers", + long_about = "Print the latest Rally agent instructions for `/rally` wrappers.\n\nPass the same wrapper args so this command can print the exact follow-up `rally skill run ...` command to execute." )] - Exec(CommandRunArgs), + AgentHowto(SkillAgentHowtoArgs), #[command(about = "Validate wrapper installation and target paths")] - Doctor(CommandDoctorArgs), + Doctor(SkillDoctorArgs), #[command(about = "Uninstall Rally-managed wrapper artifacts")] - Uninstall(CommandUninstallArgs), + Uninstall(SkillUninstallArgs), } #[derive(Args, Debug)] -pub struct CommandInstallArgs { +pub struct SkillInstallArgs { #[arg(long, value_enum, help = "Install target harness")] - pub target: CommandTargetArg, + pub target: SkillTargetArg, #[arg(long, help = "Overwrite existing non-managed files")] pub force: bool, #[arg(long, help = "Preview writes without touching files")] @@ -340,7 +347,7 @@ pub struct CommandInstallArgs { } #[derive(Args, Debug, Clone)] -pub struct CommandRunArgs { +pub struct SkillRunArgs { #[arg( value_name = "FIRST", help = "Optional workflow shorthand (for example `build`) or invite token" @@ -366,22 +373,33 @@ pub struct CommandRunArgs { } #[derive(Args, Debug)] -pub struct CommandDoctorArgs { +pub struct SkillAgentHowtoArgs { + #[arg( + value_name = "ARGUMENTS", + trailing_var_arg = true, + allow_hyphen_values = true, + help = "Wrapper arguments, for example `implement todos/foo.md implement`" + )] + pub args: Vec, +} + +#[derive(Args, Debug)] +pub struct SkillDoctorArgs { #[arg( long, value_enum, default_value = "all", help = "Target harness to inspect" )] - pub target: CommandTargetArg, + pub target: SkillTargetArg, #[arg(long, help = "Print checks without changing filesystem")] pub dry_run: bool, } #[derive(Args, Debug)] -pub struct CommandUninstallArgs { +pub struct SkillUninstallArgs { #[arg(long, value_enum, help = "Uninstall target harness")] - pub target: CommandTargetArg, + pub target: SkillTargetArg, #[arg(long, help = "Preview removals without touching files")] pub dry_run: bool, } diff --git a/src/command_surface.rs b/src/command_surface.rs index 65a07b4..f1d0270 100644 --- a/src/command_surface.rs +++ b/src/command_surface.rs @@ -11,8 +11,8 @@ use serde::{Deserialize, Serialize}; use crate::{ WorkflowRegistry, cli::{ - CommandDoctorArgs, CommandInstallArgs, CommandRunArgs, CommandTargetArg, - CommandUninstallArgs, JoinArgs, + JoinArgs, SkillAgentHowtoArgs, SkillDoctorArgs, SkillInstallArgs, SkillRunArgs, + SkillTargetArg, SkillUninstallArgs, }, commands, state::SessionHandle, @@ -21,10 +21,13 @@ use crate::{ const RALLY_DEFAULT_BIN: &str = "rally"; const RALLY_MANAGED_MARKER: &str = ""; -const CONTEXT_STORE_FILE: &str = "command-context.json"; +const CONTEXT_STORE_FILE: &str = "skill-context.json"; +const LEGACY_CONTEXT_STORE_FILE: &str = "command-context.json"; const DEFAULT_RUN_TIMEOUT_SECS: u64 = 300; +const SKILL_WRAPPER_TEMPLATE: &str = include_str!("../prompts/skill-wrapper.md"); +const SKILL_AGENT_HOWTO_TEMPLATE: &str = include_str!("../prompts/skill-agent-howto.md"); -pub fn install(args: &CommandInstallArgs) -> Result<()> { +pub fn install(args: &SkillInstallArgs) -> Result<()> { let env = AdapterEnvironment::detect()?; let render_ctx = WrapperRenderContext::default(); let outcomes = install_with_environment(args, &env, &render_ctx)?; @@ -48,18 +51,18 @@ pub fn install(args: &CommandInstallArgs) -> Result<()> { Ok(()) } -pub fn run(args: &CommandRunArgs, registry: &WorkflowRegistry) -> Result { +pub fn run(args: &SkillRunArgs, registry: &WorkflowRegistry) -> Result { let workspace = workspace_key()?; let store_path = context_store_path()?; - let mut store = load_context_store(&store_path); + let legacy_store_path = legacy_context_store_path()?; + let mut store = load_context_store(&store_path, Some(&legacy_store_path)); let saved = store.by_workspace.get(&workspace).cloned(); if let Some(cmd) = args.first.as_deref() && let Some(workflow) = registry.resolve_command(cmd) && let Some(run_cmd) = workflow.run_command() { - let workspace_path = - env::current_dir().context("resolving current working directory")?; + let workspace_path = env::current_dir().context("resolving current working directory")?; let cmd_args: Vec = args .second .iter() @@ -87,7 +90,9 @@ pub fn run(args: &CommandRunArgs, registry: &WorkflowRegistry) -> Result { registry, )?; if code == 1 { - println!("No instruction became available within {DEFAULT_RUN_TIMEOUT_SECS}s. Re-run `/rally`."); + println!( + "No instruction became available within {DEFAULT_RUN_TIMEOUT_SECS}s. Re-run `/rally`." + ); } else if code == 2 { println!("session complete; stop working"); } @@ -168,7 +173,24 @@ pub fn run(args: &CommandRunArgs, registry: &WorkflowRegistry) -> Result { Ok(code) } -pub fn doctor(args: &CommandDoctorArgs, registry: &WorkflowRegistry) -> Result<()> { +pub fn agent_howto(args: &SkillAgentHowtoArgs) -> Result<()> { + let render_ctx = WrapperRenderContext::default(); + let run_command = build_run_command_with_args(&render_ctx.rally_bin, &args.args); + let rendered = render_template( + SKILL_AGENT_HOWTO_TEMPLATE, + &[ + ("{{RALLY_BIN}}", render_ctx.rally_bin.as_str()), + ("{{RUN_COMMAND}}", run_command.as_str()), + ], + ); + print!("{rendered}"); + if !rendered.ends_with('\n') { + println!(); + } + Ok(()) +} + +pub fn doctor(args: &SkillDoctorArgs, registry: &WorkflowRegistry) -> Result<()> { let env = AdapterEnvironment::detect()?; let render_ctx = WrapperRenderContext::default(); let outcomes = doctor_with_environment(args, &env, &render_ctx)?; @@ -205,7 +227,7 @@ pub fn doctor(args: &CommandDoctorArgs, registry: &WorkflowRegistry) -> Result<( Ok(()) } -pub fn uninstall(args: &CommandUninstallArgs) -> Result<()> { +pub fn uninstall(args: &SkillUninstallArgs) -> Result<()> { let env = AdapterEnvironment::detect()?; let outcomes = uninstall_with_environment(args, &env)?; for outcome in outcomes { @@ -234,18 +256,11 @@ enum HarnessTarget { Droid, Pi, ClaudeCode, - Factory, } impl HarnessTarget { fn all() -> Vec { - vec![ - Self::Codex, - Self::Droid, - Self::Pi, - Self::ClaudeCode, - Self::Factory, - ] + vec![Self::Codex, Self::Droid, Self::Pi, Self::ClaudeCode] } fn slug(self) -> &'static str { @@ -254,7 +269,6 @@ impl HarnessTarget { Self::Droid => "droid", Self::Pi => "pi", Self::ClaudeCode => "claude-code", - Self::Factory => "factory", } } @@ -264,35 +278,38 @@ impl HarnessTarget { Self::Droid => "RALLY_DROID_HOME", Self::Pi => "RALLY_PI_HOME", Self::ClaudeCode => "RALLY_CLAUDE_CODE_HOME", - Self::Factory => "RALLY_FACTORY_HOME", } } fn default_root(self, home: &std::path::Path) -> PathBuf { match self { Self::Codex => home.join(".codex"), - Self::Droid => home.join(".droid"), + Self::Droid => home.join(".factory"), Self::Pi => home.join(".pi"), Self::ClaudeCode => home.join(".claude"), - Self::Factory => home.join(".factory"), } } - fn wrapper_path(self, root: &std::path::Path) -> PathBuf { + fn skill_path(self, root: &std::path::Path) -> PathBuf { match self { - Self::Codex => root.join("prompts").join("rally.md"), - Self::Pi => root.join("agent").join("prompts").join("rally.md"), - _ => root.join("commands").join("rally.md"), + Self::Pi => root + .join("agent") + .join("skills") + .join("rally") + .join("SKILL.md"), + _ => root.join("skills").join("rally").join("SKILL.md"), } } - fn metadata(self) -> &'static str { + /// Legacy wrapper paths that should be cleaned up on install. + fn legacy_paths(self, root: &std::path::Path) -> Vec { match self { - Self::Codex => "codex-slash-command", - Self::Droid => "droid-slash-command", - Self::Pi => "pi-slash-command", - Self::ClaudeCode => "claude-code-slash-command", - Self::Factory => "factory-slash-command", + Self::Codex => vec![root.join("prompts").join("rally.md")], + Self::Pi => vec![ + root.join("commands").join("rally.md"), + root.join("agent").join("prompts").join("rally.md"), + ], + _ => vec![root.join("commands").join("rally.md")], } } } @@ -381,7 +398,6 @@ enum DoctorStatus { WrapperMissing, InstalledManaged, InstalledManagedModified, - InstalledManagedWrongTarget, InstalledUnmanaged, } @@ -393,7 +409,6 @@ impl fmt::Display for DoctorStatus { Self::WrapperMissing => "wrapper-missing", Self::InstalledManaged => "installed-managed", Self::InstalledManagedModified => "installed-managed-modified", - Self::InstalledManagedWrongTarget => "installed-managed-wrong-target", Self::InstalledUnmanaged => "installed-unmanaged", }; f.write_str(label) @@ -442,7 +457,6 @@ struct UninstallOutcome { #[derive(Clone, Debug, Eq, PartialEq)] enum ManagedOwnership { ManagedForTarget, - ManagedForOtherTarget(String), NotManaged, } @@ -526,79 +540,76 @@ impl TargetAdapter for DotDirAdapter { ); } - Ok(self.target.wrapper_path(&root)) + Ok(self.target.skill_path(&root)) } fn render_wrapper(&self, ctx: &WrapperRenderContext) -> String { - let protocol = format!( - r#"IMPORTANT: You MUST run bash commands to participate in this session. Do NOT try to do the work without running rally commands first. - -FIRST, run this bash command immediately with the user's arguments: - -```bash -{rally} command run $ARGUMENTS -``` - -This will print your instruction. Then follow this loop: + render_template( + SKILL_WRAPPER_TEMPLATE, + &[("{{RALLY_BIN}}", ctx.rally_bin.as_str())], + ) + } +} -1. READ the instruction printed by the command above. -2. DO exactly what it says — write files, run tests, git commit, etc. -3. RUN the follow-up commands listed at the end of your instruction (checkpoint, review, etc). Run them ALL, in order. -4. RUN `{rally} command run` again with no arguments to get your next instruction. -5. REPEAT from step 1. Do NOT stop until the output says "session complete; stop working". +fn adapter_for(target: HarnessTarget) -> Box { + Box::new(DotDirAdapter::new(target)) +} -If the command prints "Waiting for ..." — keep waiting. Another agent needs to act first. -If the command exits with code 2 or prints "session complete" — you are done. Stop."#, - rally = ctx.rally_bin - ); +fn render_template(template: &str, replacements: &[(&str, &str)]) -> String { + let mut rendered = template.to_string(); + for (pattern, value) in replacements { + rendered = rendered.replace(pattern, value); + } + rendered +} - match self.target { - HarnessTarget::Codex => format!( - "{protocol}\n\n[//]: # (rally-managed: true)\n[//]: # (rally-target: {target})\n", - protocol = protocol, - target = self.target - ), - HarnessTarget::Pi => format!( - "---\ndescription: \"Run rally workflow\"\n---\n\n\n\n\n{protocol}\n", - target = self.target, - meta = self.target.metadata(), - protocol = protocol - ), - _ => format!( - "\n\n\n\n{protocol}\n", - target = self.target, - meta = self.target.metadata(), - protocol = protocol - ), - } +fn build_run_command_with_args(rally_bin: &str, args: &[String]) -> String { + let mut command = format!("{rally_bin} skill run"); + if !args.is_empty() { + let escaped = args + .iter() + .map(|token| shell_escape(token)) + .collect::>() + .join(" "); + command.push(' '); + command.push_str(&escaped); } + command } -fn adapter_for(target: HarnessTarget) -> Box { - Box::new(DotDirAdapter::new(target)) +fn shell_escape(token: &str) -> String { + if token.is_empty() { + return "''".to_string(); + } + if token + .bytes() + .all(|byte| matches!(byte, b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'_' | b'-' | b'.' | b'/' | b':' | b'=' | b'@')) + { + return token.to_string(); + } + format!("'{}'", token.replace('\'', "'\"'\"'")) } -fn selected_targets(target: CommandTargetArg) -> Vec { +fn selected_targets(target: SkillTargetArg) -> Vec { match target { - CommandTargetArg::All => HarnessTarget::all(), - CommandTargetArg::Codex => vec![HarnessTarget::Codex], - CommandTargetArg::Droid => vec![HarnessTarget::Droid], - CommandTargetArg::Pi => vec![HarnessTarget::Pi], - CommandTargetArg::ClaudeCode => vec![HarnessTarget::ClaudeCode], - CommandTargetArg::Factory => vec![HarnessTarget::Factory], + SkillTargetArg::All => HarnessTarget::all(), + SkillTargetArg::Codex => vec![HarnessTarget::Codex], + SkillTargetArg::Droid => vec![HarnessTarget::Droid], + SkillTargetArg::Pi => vec![HarnessTarget::Pi], + SkillTargetArg::ClaudeCode => vec![HarnessTarget::ClaudeCode], } } fn install_with_environment( - args: &CommandInstallArgs, + args: &SkillInstallArgs, env: &AdapterEnvironment, render_ctx: &WrapperRenderContext, ) -> Result> { let mut outcomes = Vec::new(); - let allow_missing = matches!(args.target, CommandTargetArg::All); + let allow_missing = matches!(args.target, SkillTargetArg::All); for target in selected_targets(args.target) { let root = env.root_for(target)?; - let wrapper_path = target.wrapper_path(&root); + let skill_path = target.skill_path(&root); let action = if !root.exists() { if allow_missing { InstallAction::SkipEnvironmentMissing @@ -632,6 +643,11 @@ fn install_with_environment( classify_install_action(&artifact.path, &artifact.content, target, args.force)?; if !args.dry_run { apply_install_action(&artifact, action)?; + for legacy in target.legacy_paths(&root) { + if legacy.exists() && legacy != artifact.path { + let _ = fs::remove_file(&legacy); + } + } } outcomes.push(InstallOutcome { target: artifact.target, @@ -646,7 +662,7 @@ fn install_with_environment( } outcomes.push(InstallOutcome { target, - path: wrapper_path, + path: skill_path, action, }); } @@ -672,17 +688,6 @@ fn classify_install_action( match managed_ownership(¤t, target) { ManagedOwnership::ManagedForTarget => Ok(InstallAction::UpdateManaged), - ManagedOwnership::ManagedForOtherTarget(other_target) => { - if force { - Ok(InstallAction::OverwriteForced) - } else { - bail!( - "refusing to overwrite Rally-managed file for target '{}' at {}. Re-run with --force to replace it", - other_target, - path.display() - ) - } - } ManagedOwnership::NotManaged => { if force { Ok(InstallAction::OverwriteForced) @@ -712,86 +717,63 @@ fn apply_install_action(artifact: &WrapperArtifact, action: InstallAction) -> Re } } -fn managed_ownership(content: &[u8], target: HarnessTarget) -> ManagedOwnership { - let Some(parsed_target) = parse_managed_target(content) else { - return ManagedOwnership::NotManaged; - }; - if parsed_target == target.slug() { +fn managed_ownership(content: &[u8], _target: HarnessTarget) -> ManagedOwnership { + if is_rally_managed(content) { ManagedOwnership::ManagedForTarget } else { - ManagedOwnership::ManagedForOtherTarget(parsed_target) + ManagedOwnership::NotManaged } } -fn parse_managed_target(content: &[u8]) -> Option { - let text = std::str::from_utf8(content).ok()?; - - // Skip leading YAML frontmatter (Pi prompt templates use ---\n...\n---) - let body = if text.starts_with("---\n") || text.starts_with("---\r\n") { - text.find("\n---") - .and_then(|i| text.get(i + 4..)) - .unwrap_or(text) - } else { - text +fn is_rally_managed(content: &[u8]) -> bool { + let Some(text) = std::str::from_utf8(content).ok() else { + return false; }; - // HTML comment format (Claude, Pi, Droid, Factory): - // - // - let mut lines = body.lines().map(|l| l.trim()).filter(|l| !l.is_empty()); - let first = lines.next()?; - if first == RALLY_MANAGED_MARKER { - let target_line = lines.next()?; - let prefix = ""; - if target_line.starts_with(prefix) && target_line.ends_with(suffix) { - let target = &target_line[prefix.len()..target_line.len() - suffix.len()]; - if !target.trim().is_empty() { - return Some(target.trim().to_string()); - } + // SKILL.md format: check YAML frontmatter for rally-managed: true + if text.starts_with("---\n") || text.starts_with("---\r\n") { + if let Some(fm_end) = text.find("\n---") { + let frontmatter = &text[..fm_end]; + return frontmatter + .lines() + .any(|l| l.trim() == "rally-managed: true"); } - return None; } - // Markdown comment format (Codex): - // [//]: # (rally-managed: true) - // [//]: # (rally-target: ) - let managed_md = "[//]: # (rally-managed: true)"; - let target_prefix = "[//]: # (rally-target: "; - for line in text.lines() { - let trimmed = line.trim(); - if trimmed == managed_md { - continue; - } - if trimmed.starts_with(target_prefix) && trimmed.ends_with(')') { - let inner = &trimmed[target_prefix.len()..trimmed.len() - 1]; - if !inner.trim().is_empty() { - return Some(inner.trim().to_string()); - } - } + // Legacy HTML comment format + if text.lines().any(|l| l.trim() == RALLY_MANAGED_MARKER) { + return true; } - None + // Legacy markdown comment format (Codex) + if text + .lines() + .any(|l| l.trim() == "[//]: # (rally-managed: true)") + { + return true; + } + + false } fn doctor_with_environment( - args: &CommandDoctorArgs, + args: &SkillDoctorArgs, env: &AdapterEnvironment, render_ctx: &WrapperRenderContext, ) -> Result> { let mut outcomes = Vec::new(); for target in selected_targets(args.target) { let root = env.root_for(target)?; - let wrapper = target.wrapper_path(&root); + let skill = target.skill_path(&root); let status = if !root.exists() { DoctorStatus::EnvironmentMissing } else if !root.is_dir() { DoctorStatus::InvalidEnvironment - } else if !wrapper.exists() { + } else if !skill.exists() { DoctorStatus::WrapperMissing } else { - let bytes = fs::read(&wrapper) - .with_context(|| format!("reading wrapper {}", wrapper.display()))?; + let bytes = + fs::read(&skill).with_context(|| format!("reading skill {}", skill.display()))?; match managed_ownership(&bytes, target) { ManagedOwnership::ManagedForTarget => { let expected = adapter_for(target).render_wrapper(render_ctx); @@ -801,16 +783,13 @@ fn doctor_with_environment( DoctorStatus::InstalledManagedModified } } - ManagedOwnership::ManagedForOtherTarget(_) => { - DoctorStatus::InstalledManagedWrongTarget - } ManagedOwnership::NotManaged => DoctorStatus::InstalledUnmanaged, } }; outcomes.push(DoctorOutcome { target, root, - wrapper, + wrapper: skill, status, }); } @@ -818,40 +797,43 @@ fn doctor_with_environment( } fn uninstall_with_environment( - args: &CommandUninstallArgs, + args: &SkillUninstallArgs, env: &AdapterEnvironment, ) -> Result> { let mut outcomes = Vec::new(); for target in selected_targets(args.target) { let root = env.root_for(target)?; - let wrapper = target.wrapper_path(&root); + let skill = target.skill_path(&root); let action = if !root.exists() { UninstallAction::SkippedEnvironmentMissing } else if !root.is_dir() { UninstallAction::SkippedInvalidEnvironment - } else if !wrapper.exists() { + } else if !skill.exists() { UninstallAction::SkippedWrapperMissing } else { - let bytes = fs::read(&wrapper) - .with_context(|| format!("reading wrapper {}", wrapper.display()))?; + let bytes = + fs::read(&skill).with_context(|| format!("reading skill {}", skill.display()))?; match managed_ownership(&bytes, target) { ManagedOwnership::ManagedForTarget => { if args.dry_run { UninstallAction::WouldRemoveManaged } else { - fs::remove_file(&wrapper) - .with_context(|| format!("removing wrapper {}", wrapper.display()))?; + fs::remove_file(&skill) + .with_context(|| format!("removing skill {}", skill.display()))?; + for legacy in target.legacy_paths(&root) { + if legacy.exists() { + let _ = fs::remove_file(&legacy); + } + } UninstallAction::RemovedManaged } } - ManagedOwnership::ManagedForOtherTarget(_) | ManagedOwnership::NotManaged => { - UninstallAction::SkippedUnmanaged - } + ManagedOwnership::NotManaged => UninstallAction::SkippedUnmanaged, } }; outcomes.push(UninstallOutcome { target, - wrapper, + wrapper: skill, action, }); } @@ -895,7 +877,7 @@ pub(crate) fn normalize_agent_selector(raw: &str) -> String { } fn resolve_run_context( - args: &CommandRunArgs, + args: &SkillRunArgs, workflow_selector: Option, role_selector: Option, invite: Option, @@ -1031,7 +1013,24 @@ fn context_store_path() -> Result { Ok(home.join(".rally").join(CONTEXT_STORE_FILE)) } -fn load_context_store(path: &Path) -> RunContextStore { +fn legacy_context_store_path() -> Result { + let home = dirs::home_dir().ok_or_else(|| anyhow!("unable to resolve home directory"))?; + Ok(home.join(".rally").join(LEGACY_CONTEXT_STORE_FILE)) +} + +fn load_context_store(path: &Path, legacy_path: Option<&Path>) -> RunContextStore { + if path.exists() { + return load_context_store_from_path(path); + } + if let Some(legacy) = legacy_path + && legacy.exists() + { + return load_context_store_from_path(legacy); + } + RunContextStore::default() +} + +fn load_context_store_from_path(path: &Path) -> RunContextStore { if !path.exists() { return RunContextStore::default(); } @@ -1094,14 +1093,11 @@ mod tests { #[test] fn all_target_expands_to_all_adapters() { - let names = selected_targets(CommandTargetArg::All) + let names = selected_targets(SkillTargetArg::All) .into_iter() .map(|target| target.to_string()) .collect::>(); - assert_eq!( - names, - vec!["codex", "droid", "pi", "claude-code", "factory"] - ); + assert_eq!(names, vec!["codex", "droid", "pi", "claude-code"]); } #[test] @@ -1119,20 +1115,21 @@ mod tests { #[test] fn discover_install_path_uses_default_dot_directory_layout() -> Result<()> { let home = unique_dir("rally-command-adapter-layout"); - let root = home.join(".droid"); + let root = home.join(".factory"); fs::create_dir_all(&root).with_context(|| format!("creating {}", root.display()))?; let adapter = DotDirAdapter::new(HarnessTarget::Droid); let path = adapter.discover_install_path(&env_with_home(&home))?; - assert_eq!(path, root.join("commands").join("rally.md")); + assert_eq!(path, root.join("skills").join("rally").join("SKILL.md")); Ok(()) } #[test] fn wrapper_rendering_delegates_to_command_run() { - let adapter = DotDirAdapter::new(HarnessTarget::Factory); + let adapter = DotDirAdapter::new(HarnessTarget::Droid); let rendered = adapter.render_wrapper(&WrapperRenderContext::default()); - assert!(rendered.contains("rally command run $ARGUMENTS")); - assert!(rendered.contains("rally-target: factory")); + assert!(rendered.contains("rally skill agent-howto $ARGUMENTS")); + assert!(rendered.contains("rally-managed: true")); + assert!(rendered.starts_with("---\nname: rally\n")); } fn prepare_target_root(home: &Path, target: HarnessTarget) -> Result { @@ -1141,20 +1138,20 @@ mod tests { Ok(root) } - fn install_args(target: CommandTargetArg, force: bool, dry_run: bool) -> CommandInstallArgs { - CommandInstallArgs { + fn install_args(target: SkillTargetArg, force: bool, dry_run: bool) -> SkillInstallArgs { + SkillInstallArgs { target, force, dry_run, } } - fn doctor_args(target: CommandTargetArg, dry_run: bool) -> CommandDoctorArgs { - CommandDoctorArgs { target, dry_run } + fn doctor_args(target: SkillTargetArg, dry_run: bool) -> SkillDoctorArgs { + SkillDoctorArgs { target, dry_run } } - fn uninstall_args(target: CommandTargetArg, dry_run: bool) -> CommandUninstallArgs { - CommandUninstallArgs { target, dry_run } + fn uninstall_args(target: SkillTargetArg, dry_run: bool) -> SkillUninstallArgs { + SkillUninstallArgs { target, dry_run } } struct StaticPrompter { @@ -1177,8 +1174,8 @@ mod tests { } } - fn run_args() -> CommandRunArgs { - CommandRunArgs { + fn run_args() -> SkillRunArgs { + SkillRunArgs { first: None, second: None, extra: Vec::new(), @@ -1197,20 +1194,20 @@ mod tests { let render_ctx = WrapperRenderContext::default(); let first = install_with_environment( - &install_args(CommandTargetArg::Codex, false, false), + &install_args(SkillTargetArg::Codex, false, false), &env, &render_ctx, )?; assert_eq!(first.len(), 1); assert_eq!(first[0].action, InstallAction::Create); - let wrapper = root.join("prompts").join("rally.md"); - let content = fs::read_to_string(&wrapper)?; - assert!(content.contains("rally command run $ARGUMENTS")); + let skill = root.join("skills").join("rally").join("SKILL.md"); + let content = fs::read_to_string(&skill)?; + assert!(content.contains("rally skill agent-howto $ARGUMENTS")); assert!(content.contains("rally-managed: true")); let second = install_with_environment( - &install_args(CommandTargetArg::Codex, false, false), + &install_args(SkillTargetArg::Codex, false, false), &env, &render_ctx, )?; @@ -1223,14 +1220,18 @@ mod tests { fn install_refuses_to_overwrite_unknown_file_without_force() -> Result<()> { let home = unique_dir("rally-command-install-no-force"); let root = prepare_target_root(&home, HarnessTarget::Pi)?; - let wrapper = root.join("agent").join("prompts").join("rally.md"); - fs::create_dir_all(wrapper.parent().expect("wrapper parent"))?; - fs::write(&wrapper, "custom wrapper")?; + let skill = root + .join("agent") + .join("skills") + .join("rally") + .join("SKILL.md"); + fs::create_dir_all(skill.parent().expect("skill parent"))?; + fs::write(&skill, "custom wrapper")?; let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); let err = install_with_environment( - &install_args(CommandTargetArg::Pi, false, false), + &install_args(SkillTargetArg::Pi, false, false), &env, &render_ctx, ) @@ -1238,7 +1239,7 @@ mod tests { let msg = err.to_string(); assert!(msg.contains("refusing to overwrite non-Rally file")); assert!(msg.contains("--force")); - assert_eq!(fs::read_to_string(&wrapper)?, "custom wrapper"); + assert_eq!(fs::read_to_string(&skill)?, "custom wrapper"); Ok(()) } @@ -1246,20 +1247,20 @@ mod tests { fn install_overwrites_unknown_file_with_force() -> Result<()> { let home = unique_dir("rally-command-install-force"); let root = prepare_target_root(&home, HarnessTarget::ClaudeCode)?; - let wrapper = root.join("commands").join("rally.md"); - fs::create_dir_all(wrapper.parent().expect("wrapper parent"))?; - fs::write(&wrapper, "custom wrapper")?; + let skill = root.join("skills").join("rally").join("SKILL.md"); + fs::create_dir_all(skill.parent().expect("skill parent"))?; + fs::write(&skill, "custom wrapper")?; let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); let outcome = install_with_environment( - &install_args(CommandTargetArg::ClaudeCode, true, false), + &install_args(SkillTargetArg::ClaudeCode, true, false), &env, &render_ctx, )?; assert_eq!(outcome[0].action, InstallAction::OverwriteForced); - let content = fs::read_to_string(&wrapper)?; - assert!(content.contains(RALLY_MANAGED_MARKER)); + let content = fs::read_to_string(&skill)?; + assert!(content.contains("rally-managed: true")); Ok(()) } @@ -1271,7 +1272,7 @@ mod tests { let render_ctx = WrapperRenderContext::default(); let outcomes = install_with_environment( - &install_args(CommandTargetArg::All, false, false), + &install_args(SkillTargetArg::All, false, false), &env, &render_ctx, )?; @@ -1285,7 +1286,13 @@ mod tests { .expect("droid outcome"); assert_eq!(codex.action, InstallAction::Create); assert_eq!(droid.action, InstallAction::SkipEnvironmentMissing); - assert!(codex_root.join("prompts").join("rally.md").exists()); + assert!( + codex_root + .join("skills") + .join("rally") + .join("SKILL.md") + .exists() + ); Ok(()) } @@ -1293,17 +1300,21 @@ mod tests { fn marker_substring_does_not_make_file_managed() -> Result<()> { let home = unique_dir("rally-command-marker-substring"); let root = prepare_target_root(&home, HarnessTarget::Pi)?; - let wrapper = root.join("agent").join("prompts").join("rally.md"); - fs::create_dir_all(wrapper.parent().expect("wrapper parent"))?; + let skill = root + .join("agent") + .join("skills") + .join("rally") + .join("SKILL.md"); + fs::create_dir_all(skill.parent().expect("skill parent"))?; fs::write( - &wrapper, - "user file\ncontains somewhere\n", + &skill, + "user file\ncontains rally-managed: true somewhere\n", )?; let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); let install_err = install_with_environment( - &install_args(CommandTargetArg::Pi, false, false), + &install_args(SkillTargetArg::Pi, false, false), &env, &render_ctx, ) @@ -1315,26 +1326,26 @@ mod tests { ); let uninstall = - uninstall_with_environment(&uninstall_args(CommandTargetArg::Pi, false), &env)?; + uninstall_with_environment(&uninstall_args(SkillTargetArg::Pi, false), &env)?; assert_eq!(uninstall[0].action, UninstallAction::SkippedUnmanaged); - assert!(wrapper.exists()); + assert!(skill.exists()); Ok(()) } #[test] fn doctor_reports_managed_install_status() -> Result<()> { let home = unique_dir("rally-command-doctor-managed"); - let _root = prepare_target_root(&home, HarnessTarget::Factory)?; + let _root = prepare_target_root(&home, HarnessTarget::Droid)?; let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); install_with_environment( - &install_args(CommandTargetArg::Factory, false, false), + &install_args(SkillTargetArg::Droid, false, false), &env, &render_ctx, )?; let result = doctor_with_environment( - &doctor_args(CommandTargetArg::Factory, false), + &doctor_args(SkillTargetArg::Droid, false), &env, &render_ctx, )?; @@ -1344,23 +1355,23 @@ mod tests { } #[test] - fn doctor_reports_wrong_target_when_header_target_mismatches_path() -> Result<()> { - let home = unique_dir("rally-command-doctor-target-mismatch"); + fn doctor_reports_unmanaged_when_file_is_not_rally() -> Result<()> { + let home = unique_dir("rally-command-doctor-unmanaged"); let root = prepare_target_root(&home, HarnessTarget::Codex)?; - let wrapper = root.join("prompts").join("rally.md"); - fs::create_dir_all(wrapper.parent().expect("wrapper parent"))?; + let skill = root.join("skills").join("rally").join("SKILL.md"); + fs::create_dir_all(skill.parent().expect("skill parent"))?; fs::write( - &wrapper, - "\n\n\n# /rally\n", + &skill, + "---\nname: rally\ndescription: custom rally skill\n---\n# My Rally\n", )?; let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); let result = doctor_with_environment( - &doctor_args(CommandTargetArg::Codex, false), + &doctor_args(SkillTargetArg::Codex, false), &env, &render_ctx, )?; - assert_eq!(result[0].status, DoctorStatus::InstalledManagedWrongTarget); + assert_eq!(result[0].status, DoctorStatus::InstalledUnmanaged); Ok(()) } @@ -1373,16 +1384,16 @@ mod tests { let render_ctx = WrapperRenderContext::default(); install_with_environment( - &install_args(CommandTargetArg::Codex, false, false), + &install_args(SkillTargetArg::Codex, false, false), &env, &render_ctx, )?; - let unmanaged = unmanaged_root.join("commands").join("rally.md"); - fs::create_dir_all(unmanaged.parent().expect("wrapper parent"))?; + let unmanaged = unmanaged_root.join("skills").join("rally").join("SKILL.md"); + fs::create_dir_all(unmanaged.parent().expect("skill parent"))?; fs::write(&unmanaged, "custom content")?; let outcome = - uninstall_with_environment(&uninstall_args(CommandTargetArg::All, false), &env)?; + uninstall_with_environment(&uninstall_args(SkillTargetArg::All, false), &env)?; let codex = outcome .iter() .find(|o| o.target == HarnessTarget::Codex) @@ -1393,7 +1404,13 @@ mod tests { .expect("droid result"); assert_eq!(codex.action, UninstallAction::RemovedManaged); assert_eq!(droid.action, UninstallAction::SkippedUnmanaged); - assert!(!managed_root.join("prompts").join("rally.md").exists()); + assert!( + !managed_root + .join("skills") + .join("rally") + .join("SKILL.md") + .exists() + ); assert_eq!(fs::read_to_string(&unmanaged)?, "custom content"); Ok(()) } @@ -1405,21 +1422,24 @@ mod tests { let env = env_with_home(&home); let render_ctx = WrapperRenderContext::default(); install_with_environment( - &install_args(CommandTargetArg::Pi, false, false), + &install_args(SkillTargetArg::Pi, false, false), &env, &render_ctx, )?; - let wrapper = root.join("agent").join("prompts").join("rally.md"); - let outcome = - uninstall_with_environment(&uninstall_args(CommandTargetArg::Pi, true), &env)?; + let skill = root + .join("agent") + .join("skills") + .join("rally") + .join("SKILL.md"); + let outcome = uninstall_with_environment(&uninstall_args(SkillTargetArg::Pi, true), &env)?; assert_eq!(outcome[0].action, UninstallAction::WouldRemoveManaged); - assert!(wrapper.exists()); + assert!(skill.exists()); Ok(()) } #[test] fn run_resolution_precedence_is_explicit_then_token_then_saved() -> Result<()> { - let args = CommandRunArgs { + let args = SkillRunArgs { session: Some("explicit-session".to_string()), agent: Some("explicit-agent".to_string()), ..run_args() @@ -1450,7 +1470,7 @@ mod tests { #[test] fn run_resolution_uses_invite_when_explicit_missing() -> Result<()> { - let args = CommandRunArgs { + let args = SkillRunArgs { non_interactive: true, ..run_args() }; @@ -1473,7 +1493,7 @@ mod tests { #[test] fn run_resolution_uses_saved_context_before_prompting() -> Result<()> { - let args = CommandRunArgs { + let args = SkillRunArgs { non_interactive: true, ..run_args() }; @@ -1502,7 +1522,7 @@ mod tests { #[test] fn run_resolution_non_interactive_errors_when_unresolved() { - let args = CommandRunArgs { + let args = SkillRunArgs { non_interactive: true, ..run_args() }; @@ -1521,8 +1541,40 @@ mod tests { let path = home.join(".rally").join(CONTEXT_STORE_FILE); fs::create_dir_all(path.parent().expect("context dir"))?; fs::write(&path, "{not-json}")?; - let loaded = load_context_store(&path); + let loaded = load_context_store(&path, None); assert!(loaded.by_workspace.is_empty()); Ok(()) } + + #[test] + fn load_context_store_uses_legacy_path_when_new_path_missing() -> Result<()> { + let home = unique_dir("rally-skill-context-legacy"); + let legacy = home.join(".rally").join(LEGACY_CONTEXT_STORE_FILE); + fs::create_dir_all(legacy.parent().expect("legacy parent"))?; + fs::write( + &legacy, + r#"{"by_workspace":{"/tmp/workspace":{"session":"s","agent":"a"}}}"#, + )?; + + let new_path = home.join(".rally").join(CONTEXT_STORE_FILE); + let loaded = load_context_store(&new_path, Some(&legacy)); + assert_eq!(loaded.by_workspace.len(), 1); + Ok(()) + } + + #[test] + fn build_run_command_with_args_escapes_shell_tokens() { + let rendered = build_run_command_with_args( + "rally", + &[ + "implement".to_string(), + "todos/my todo.md".to_string(), + "review".to_string(), + ], + ); + assert_eq!( + rendered, + "rally skill run implement 'todos/my todo.md' review" + ); + } } diff --git a/src/lib.rs b/src/lib.rs index ff1532e..a58e603 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ pub mod workflow; use anyhow::Result; use clap::Parser; -use cli::{BuildSubcommand, Cli, Command, CommandSubcommand, PlanSubcommand, WorkflowSubcommand}; +use cli::{BuildSubcommand, Cli, Command, PlanSubcommand, SkillSubcommand, WorkflowSubcommand}; pub use workflow::{Workflow, WorkflowId, WorkflowRegistry}; @@ -93,20 +93,24 @@ pub fn run_cli_with_registry(cli: Cli, registry: &WorkflowRegistry) -> Result { + Command::Skill(args) => { let exit_code = match args.command { - CommandSubcommand::Install(inner) => { + SkillSubcommand::Install(inner) => { command_surface::install(&inner)?; 0 } - CommandSubcommand::Run(inner) | CommandSubcommand::Exec(inner) => { + SkillSubcommand::Run(inner) | SkillSubcommand::Exec(inner) => { command_surface::run(&inner, registry)? } - CommandSubcommand::Doctor(inner) => { + SkillSubcommand::AgentHowto(inner) => { + command_surface::agent_howto(&inner)?; + 0 + } + SkillSubcommand::Doctor(inner) => { command_surface::doctor(&inner, registry)?; 0 } - CommandSubcommand::Uninstall(inner) => { + SkillSubcommand::Uninstall(inner) => { command_surface::uninstall(&inner)?; 0 } diff --git a/tests/command_install_run.rs b/tests/command_install_run.rs index aad3c9b..4f5d7bb 100644 --- a/tests/command_install_run.rs +++ b/tests/command_install_run.rs @@ -43,7 +43,7 @@ fn write_todo(path: &Path, title: &str) -> Result<()> { fs::write( path, format!( - "## Spec\n\ncommand run test\n\n## Plan\n\n1. {title}\nAcceptance criteria:\n- done\n" + "## Spec\n\nskill run test\n\n## Plan\n\n1. {title}\nAcceptance criteria:\n- done\n" ), ) .with_context(|| format!("writing {}", path.display())) @@ -67,7 +67,7 @@ fn read_agent_last_seen(home: &Path, session: &str, agent: &str) -> Result Result { - let store = home.join(".rally").join("command-context.json"); + let store = home.join(".rally").join("skill-context.json"); let raw = fs::read_to_string(&store).with_context(|| format!("reading {}", store.display()))?; let parsed: Value = serde_json::from_str(&raw)?; let key = workspace @@ -96,55 +96,62 @@ fn command_install_run_entrypoint_integration() -> Result<()> { let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let codex_root = temp_home.join(".codex"); - let droid_root = temp_home.join(".droid"); + let droid_root = temp_home.join(".factory"); let pi_root = temp_home.join(".pi"); let claude_root = temp_home.join(".claude"); - let factory_root = temp_home.join(".factory"); - for root in [ - &codex_root, - &droid_root, - &pi_root, - &claude_root, - &factory_root, - ] { + for root in [&codex_root, &droid_root, &pi_root, &claude_root] { fs::create_dir_all(root)?; } - run_cli( - &["rally", "command", "install", "--target", "all"], - ®istry, - )?; + run_cli(&["rally", "skill", "install", "--target", "all"], ®istry)?; + assert_eq!( + run_cli( + &[ + "rally", + "skill", + "agent-howto", + "implement", + "todo-one.md", + "implement", + ], + ®istry, + )?, + 0 + ); - for (target, root) in [ + for (_target, root) in [ ("codex", &codex_root), ("droid", &droid_root), ("pi", &pi_root), ("claude-code", &claude_root), - ("factory", &factory_root), ] { - let wrapper = match target { - "codex" => root.join("prompts").join("rally.md"), - "pi" => root.join("agent").join("prompts").join("rally.md"), - _ => root.join("commands").join("rally.md"), + let skill = match _target { + "pi" => root + .join("agent") + .join("skills") + .join("rally") + .join("SKILL.md"), + _ => root.join("skills").join("rally").join("SKILL.md"), }; - let content = fs::read_to_string(&wrapper)?; - assert!(content.contains("rally command run $ARGUMENTS")); - assert!(content.contains(&format!("rally-target: {target}"))); + let content = fs::read_to_string(&skill)?; + assert!(content.contains("rally skill agent-howto $ARGUMENTS")); + assert!(content.contains("rally-managed: true")); } - let codex_wrapper = codex_root.join("prompts").join("rally.md"); - let before = fs::read_to_string(&codex_wrapper)?; + let codex_skill = codex_root.join("skills").join("rally").join("SKILL.md"); + let before = fs::read_to_string(&codex_skill)?; run_cli( - &["rally", "command", "install", "--target", "codex"], + &["rally", "skill", "install", "--target", "codex"], ®istry, )?; - let after = fs::read_to_string(&codex_wrapper)?; + let after = fs::read_to_string(&codex_skill)?; assert_eq!(before, after); - let droid_wrapper = droid_root.join("commands").join("rally.md"); - fs::write(&droid_wrapper, "custom wrapper content")?; + let droid_skill = droid_root.join("skills").join("rally").join("SKILL.md"); + fs::create_dir_all(droid_skill.parent().expect("skill parent"))?; + fs::write(&droid_skill, "custom wrapper content")?; let err = run_cli( - &["rally", "command", "install", "--target", "droid"], + &["rally", "skill", "install", "--target", "droid"], ®istry, ) .expect_err("unmanaged wrapper should require --force"); @@ -154,14 +161,14 @@ fn command_install_run_entrypoint_integration() -> Result<()> { ); run_cli( - &["rally", "command", "doctor", "--target", "all", "--dry-run"], + &["rally", "skill", "doctor", "--target", "all", "--dry-run"], ®istry, )?; run_cli( &[ "rally", - "command", + "skill", "uninstall", "--target", "codex", @@ -169,17 +176,14 @@ fn command_install_run_entrypoint_integration() -> Result<()> { ], ®istry, )?; - assert!(codex_wrapper.exists()); + assert!(codex_skill.exists()); run_cli( - &["rally", "command", "uninstall", "--target", "codex"], + &["rally", "skill", "uninstall", "--target", "codex"], ®istry, )?; - assert!(!codex_wrapper.exists()); - assert_eq!( - fs::read_to_string(&droid_wrapper)?, - "custom wrapper content" - ); + assert!(!codex_skill.exists()); + assert_eq!(fs::read_to_string(&droid_skill)?, "custom wrapper content"); let todo_one = temp_home.join("todo-one.md"); let todo_two = temp_home.join("todo-two.md"); @@ -248,7 +252,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { .unwrap_or_else(|_| manifest_dir.clone()) .display() .to_string(); - let context_store = temp_home.join(".rally").join("command-context.json"); + let context_store = temp_home.join(".rally").join("skill-context.json"); fs::create_dir_all(context_store.parent().expect("context store parent"))?; fs::write( &context_store, @@ -259,7 +263,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { let session_one_seen_before = read_agent_last_seen(&temp_home, &session_one, "implementer")?; assert_eq!( - run_cli(&["rally", "command", "run", "--non-interactive"], ®istry,)?, + run_cli(&["rally", "skill", "run", "--non-interactive"], ®istry,)?, 0 ); let session_one_seen_after = read_agent_last_seen(&temp_home, &session_one, "implementer")?; @@ -269,7 +273,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { run_cli( &[ "rally", - "command", + "skill", "run", &format!("rly:{session_two}:implementer"), "--non-interactive", @@ -287,7 +291,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { run_cli( &[ "rally", - "command", + "skill", "run", "--session", &session_one, @@ -311,7 +315,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { run_cli( &[ "rally", - "command", + "skill", "run", "--session", &session_one, @@ -326,7 +330,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { fs::remove_file(&context_store)?; let mut child = Command::new(env!("CARGO_BIN_EXE_rally")) - .arg("command") + .arg("skill") .arg("run") .env("HOME", &temp_home) .current_dir(&manifest_dir) @@ -334,7 +338,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .context("spawning interactive rally command run")?; + .context("spawning interactive rally skill run")?; { let stdin = child.stdin.as_mut().expect("stdin"); writeln!(stdin, "{session_two}")?; @@ -362,7 +366,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { run_cli( &[ "rally", - "command", + "skill", "run", "--session", &session_two, @@ -375,7 +379,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { 2 ); - // If persisted-context saving fails after `next` succeeds, `command run` must still + // If persisted-context saving fails after `next` succeeds, `skill run` must still // return the `next` exit code instead of failing the command. if context_store.exists() { fs::remove_file(&context_store)?; @@ -385,7 +389,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { run_cli( &[ "rally", - "command", + "skill", "run", "--session", &session_one, @@ -410,7 +414,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { let run_cmd_result = run_cli( &[ "rally", - "command", + "skill", "run", "implement", &run_cmd_todo_rel, @@ -450,7 +454,7 @@ fn command_install_run_entrypoint_integration() -> Result<()> { let run_cmd_result2 = run_cli( &[ "rally", - "command", + "skill", "run", "implement", &run_cmd_todo_rel, diff --git a/tests/e2e-agents.sh b/tests/e2e-agents.sh index 1080737..8d2c575 100755 --- a/tests/e2e-agents.sh +++ b/tests/e2e-agents.sh @@ -10,11 +10,11 @@ echo "Building and installing rally..." cargo install --path . --quiet echo "Installing wrappers..." -cargo run --quiet -- command install --target all --force 2>&1 | sed 's/^/ /' +cargo run --quiet -- skill install --target all --force 2>&1 | sed 's/^/ /' # Sanity check SANITY="$TMPDIR/sanity.txt" -rally command run hello "$SANITY" +rally skill run hello "$SANITY" if ! [ -f "$SANITY" ] || ! grep -q "Hello, World!" "$SANITY"; then echo "FATAL: rally hello broken locally"; exit 1 fi diff --git a/tests/e2e-implement.sh b/tests/e2e-implement.sh index 71050b2..a7f9054 100755 --- a/tests/e2e-implement.sh +++ b/tests/e2e-implement.sh @@ -21,7 +21,7 @@ echo "Logs: $LOGDIR" # ── build & install ────────────────────────────────────────────────── echo "Building and installing rally..." cargo install --path . --quiet -rally command install --target all --force 2>&1 | sed 's/^/ /' +rally skill install --target all --force 2>&1 | sed 's/^/ /' # ── create a unique 1-step todo in workspace ───────────────────────── TS=$(date +%s) diff --git a/tests/e2e_matrix.py b/tests/e2e_matrix.py new file mode 100644 index 0000000..f54b320 --- /dev/null +++ b/tests/e2e_matrix.py @@ -0,0 +1,465 @@ +#!/usr/bin/env python3 +""" +Concurrent e2e test matrix for rally implement protocol. + +Runs all requested (implementer, reviewer) agent pairs concurrently. +Each test creates a unique 1-step todo, launches both agents, polls +the session state, and reports results. + +Usage: + python3 tests/e2e_matrix.py # all available pairs + python3 tests/e2e_matrix.py --solo # solo tests only (--reviewers 0) + python3 tests/e2e_matrix.py --pairs # pair tests only + python3 tests/e2e_matrix.py --impl claude --revw pi # specific pair + python3 tests/e2e_matrix.py --timeout 180 # custom timeout +""" + +import argparse +import json +import os +import shutil +import subprocess +import sys +import tempfile +import textwrap +import time +from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import dataclass, field +from pathlib import Path +from typing import Optional + +WORKSPACE = Path(__file__).resolve().parent.parent +HOME = Path.home() +SESSIONS_DIR = HOME / ".rally" / "sessions" + +AGENTS = { + "claude": { + "launch_stdin": lambda prompt, logfile: [ + "claude", "--dangerously-skip-permissions", "--no-session-persistence" + ], + "prompt_style": "stdin", + "skill_prefix": "/rally", + }, + "codex": { + "launch_exec": lambda prompt, logfile: [ + "codex", "exec", prompt, + "--dangerously-bypass-approvals-and-sandbox", "--ephemeral" + ], + "prompt_style": "exec", + "skill_prefix": "$rally", + }, + "pi": { + "launch_stdin": lambda prompt, logfile: [ + "pi", "--no-session", "--tools", "bash" + ], + "prompt_style": "stdin", + "skill_prefix": "Use the rally skill to:", + }, + "droid": { + "launch_exec": lambda prompt, logfile: [ + "droid", "exec", prompt, "--skip-permissions-unsafe" + ], + "prompt_style": "exec", + "skill_prefix": "/rally", + }, +} + + +@dataclass +class TestResult: + name: str + impl_agent: str + revw_agent: Optional[str] + session: str + phase: str + agents_joined: list + file_content: Optional[str] + duration: float + passed: bool + failure_reason: str = "" + logdir: str = "" + errors: list = field(default_factory=list) + + +def available_agents(): + return [a for a in AGENTS if shutil.which(a)] + + +def read_state(session_name): + state_file = SESSIONS_DIR / session_name / "state.json" + if not state_file.exists(): + return None + try: + return json.loads(state_file.read_text()) + except (json.JSONDecodeError, OSError): + return None + + +def launch_agent(agent_name, prompt, logfile): + cfg = AGENTS[agent_name] + if cfg["prompt_style"] == "stdin": + cmd = cfg["launch_stdin"](prompt, logfile) + proc = subprocess.Popen( + cmd, + stdin=subprocess.PIPE, + stdout=open(logfile, "w"), + stderr=subprocess.STDOUT, + cwd=str(WORKSPACE), + ) + proc.stdin.write(prompt.encode()) + proc.stdin.close() + else: + cmd = cfg["launch_exec"](prompt, logfile) + proc = subprocess.Popen( + cmd, + stdout=open(logfile, "w"), + stderr=subprocess.STDOUT, + cwd=str(WORKSPACE), + ) + return proc + + +def build_prompt(agent_name, role, todo_filename, extra_flags=""): + cfg = AGENTS[agent_name] + prefix = cfg["skill_prefix"] + args = f"implement {todo_filename} {role}" + if extra_flags: + args += f" {extra_flags}" + return f"{prefix} {args}" + + +def run_solo_test(agent_name, timeout=120): + ts = int(time.time() * 1000) + todo_name = f"solo-{agent_name}-{ts}" + todo_file = WORKSPACE / f"{todo_name}.md" + session_name = f"implement-{todo_name}" + outfile = WORKSPACE / f"solo-{agent_name}-{ts}.txt" + + logdir = tempfile.mkdtemp(prefix=f"rally-solo-{agent_name}-") + + todo_file.write_text(textwrap.dedent(f"""\ + ## Spec + Solo test for {agent_name}. + ## Plan + 1. Create {outfile.name} with content "hello {agent_name}" + Acceptance criteria: + - {outfile.name} exists with "hello {agent_name}" + """)) + + start = time.time() + prompt = build_prompt(agent_name, "implementer", todo_file.name, "--reviewers 0") + logfile = os.path.join(logdir, "agent.log") + + proc = launch_agent(agent_name, prompt, logfile) + + # Poll + final_phase = "no-session" + elapsed = 0 + while elapsed < timeout: + time.sleep(5) + elapsed = time.time() - start + state = read_state(session_name) + if state: + final_phase = state.get("phase", "unknown") + # For solo, any phase past registration means rally was used + if final_phase in ("review", "final_review", "done"): + break + + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + + duration = time.time() - start + file_content = outfile.read_text().strip() if outfile.exists() else None + state = read_state(session_name) + agents_joined = list(state.get("agents", {}).keys()) if state else [] + + passed = ( + state is not None # session was created (rally was invoked) + and final_phase in ("implement", "review", "final_review", "done") + and file_content is not None + ) + + errors = [] + if state is None: + errors.append("session never created -- agent didn't run rally") + if file_content is None: + errors.append(f"output file {outfile.name} not created") + elif file_content != f"hello {agent_name}": + errors.append(f"file content mismatch: got '{file_content}', expected 'hello {agent_name}'") + + result = TestResult( + name=f"solo-{agent_name}", + impl_agent=agent_name, + revw_agent=None, + session=session_name, + phase=final_phase, + agents_joined=agents_joined, + file_content=file_content, + duration=duration, + passed=passed, + failure_reason="; ".join(errors) if errors else "", + logdir=logdir, + errors=errors, + ) + + # Cleanup + todo_file.unlink(missing_ok=True) + outfile.unlink(missing_ok=True) + session_dir = SESSIONS_DIR / session_name + if session_dir.exists(): + shutil.rmtree(session_dir, ignore_errors=True) + + return result + + +def run_pair_test(impl_agent, revw_agent, timeout=300): + ts = int(time.time() * 1000) + todo_name = f"pair-{impl_agent}-{revw_agent}-{ts}" + todo_file = WORKSPACE / f"{todo_name}.md" + session_name = f"implement-{todo_name}" + outfile = WORKSPACE / f"pair-{impl_agent}-{revw_agent}-{ts}.txt" + + logdir = tempfile.mkdtemp(prefix=f"rally-pair-{impl_agent}-{revw_agent}-") + + todo_file.write_text(textwrap.dedent(f"""\ + ## Spec + Pair test: {impl_agent} implements, {revw_agent} reviews. + ## Plan + 1. Create {outfile.name} with content "step 1" + Acceptance criteria: + - {outfile.name} exists with "step 1" + 2. Append " and step 2" to {outfile.name} + Acceptance criteria: + - {outfile.name} contains "step 1 and step 2" + """)) + + start = time.time() + impl_prompt = build_prompt(impl_agent, "implementer", todo_file.name) + revw_prompt = build_prompt(revw_agent, "reviewer", todo_file.name) + + impl_log = os.path.join(logdir, "impl.log") + revw_log = os.path.join(logdir, "revw.log") + + impl_proc = launch_agent(impl_agent, impl_prompt, impl_log) + time.sleep(8) + revw_proc = launch_agent(revw_agent, revw_prompt, revw_log) + + # Poll with phase trace + phase_trace = [] + final_phase = "no-session" + elapsed = 0 + while elapsed < timeout: + time.sleep(10) + elapsed = time.time() - start + state = read_state(session_name) + if state: + final_phase = state.get("phase", "unknown") + agents = list(state.get("agents", {}).keys()) + phase_trace.append((int(elapsed), final_phase, agents)) + if final_phase == "done": + break + + for proc in (impl_proc, revw_proc): + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + + duration = time.time() - start + file_content = outfile.read_text().strip() if outfile.exists() else None + state = read_state(session_name) + agents_joined = list(state.get("agents", {}).keys()) if state else [] + + # Check for handoff: phase must go implement -> review -> implement (step 2) + saw_handoff = False + last_phase = None + implement_count = 0 + for _, ph, _ in phase_trace: + if ph == "implement": + implement_count += 1 + if last_phase == "review" and ph == "implement" and implement_count >= 2: + saw_handoff = True + last_phase = ph + + errors = [] + if state is None: + errors.append("session never created") + elif "implementer" not in agents_joined: + errors.append("implementer never joined") + elif "reviewer-1" not in agents_joined: + errors.append("reviewer never joined") + if final_phase != "done": + errors.append(f"phase={final_phase}, expected done") + if file_content is None: + errors.append(f"output file {outfile.name} not created") + elif "step 1 and step 2" not in file_content: + errors.append(f"file content wrong: '{file_content}'") + if not saw_handoff and final_phase == "done": + errors.append("no implement->review->implement handoff detected") + + passed = final_phase == "done" and not errors + + result = TestResult( + name=f"pair-{impl_agent}+{revw_agent}", + impl_agent=impl_agent, + revw_agent=revw_agent, + session=session_name, + phase=final_phase, + agents_joined=agents_joined, + file_content=file_content, + duration=duration, + passed=passed, + failure_reason="; ".join(errors) if errors else "", + logdir=logdir, + errors=errors, + ) + + # Cleanup + todo_file.unlink(missing_ok=True) + outfile.unlink(missing_ok=True) + session_dir = SESSIONS_DIR / session_name + if session_dir.exists(): + shutil.rmtree(session_dir, ignore_errors=True) + + return result + + +def print_results(results): + print() + print("=" * 72) + print(f"{'TEST':<30} {'RESULT':<8} {'PHASE':<14} {'TIME':>6} DETAILS") + print("-" * 72) + for r in results: + status = "\033[32mPASS\033[0m" if r.passed else "\033[31mFAIL\033[0m" + print(f"{r.name:<30} {status:<17} {r.phase:<14} {r.duration:5.0f}s {r.failure_reason[:40]}") + print("-" * 72) + + passed = sum(1 for r in results if r.passed) + total = len(results) + color = "\033[32m" if passed == total else "\033[31m" + print(f"{color}{passed}/{total} passed\033[0m") + + failed = [r for r in results if not r.passed] + if failed: + print() + print("FAILURE DETAILS:") + for r in failed: + print(f"\n {r.name}:") + print(f" Session: {r.session}") + print(f" Phase: {r.phase}") + print(f" Agents: {r.agents_joined}") + print(f" File: {r.file_content!r}") + print(f" Logs: {r.logdir}") + for e in r.errors: + print(f" ERROR: {e}") + log_files = list(Path(r.logdir).glob("*.log")) + for lf in log_files: + content = lf.read_text() + if content.strip(): + tail = "\n".join(content.strip().splitlines()[-10:]) + print(f" --- {lf.name} (last 10 lines) ---") + for line in tail.splitlines(): + print(f" {line}") + + +def main(): + parser = argparse.ArgumentParser(description="Rally e2e test matrix") + parser.add_argument("--solo", action="store_true", help="solo tests only") + parser.add_argument("--pairs", action="store_true", help="pair tests only") + parser.add_argument("--impl", help="specific implementer agent") + parser.add_argument("--revw", help="specific reviewer agent") + parser.add_argument("--timeout", type=int, default=300, help="per-test timeout (default 300)") + parser.add_argument("--solo-timeout", type=int, default=120, help="solo test timeout (default 120)") + parser.add_argument("--jobs", type=int, default=4, help="max concurrent tests (default 4)") + args = parser.parse_args() + + agents = available_agents() + print(f"Available agents: {', '.join(agents)}") + + if len(agents) == 0: + print("No agents found!") + sys.exit(1) + + # Build test list + futures_map = {} + results = [] + + # Specific pair + if args.impl and args.revw: + if args.impl not in agents or args.revw not in agents: + print(f"Agent not available. Have: {agents}") + sys.exit(1) + with ThreadPoolExecutor(max_workers=1) as pool: + f = pool.submit(run_pair_test, args.impl, args.revw, args.timeout) + results.append(f.result()) + print_results(results) + sys.exit(0 if all(r.passed for r in results) else 1) + + def collect(futures_map): + for future in as_completed(futures_map): + test_name = futures_map[future] + try: + result = future.result() + results.append(result) + status = "PASS" if result.passed else "FAIL" + print(f" [{status}] {test_name} ({result.duration:.0f}s) {result.phase}") + except Exception as e: + print(f" [ERROR] {test_name}: {e}") + + # Phase 1: all solo tests concurrently (each agent used once) + if not args.pairs: + print("\n--- Solo tests ---") + with ThreadPoolExecutor(max_workers=len(agents)) as pool: + futures_map = {} + for agent in agents: + f = pool.submit(run_solo_test, agent, args.solo_timeout) + futures_map[f] = f"solo-{agent}" + collect(futures_map) + + # Phase 2: pair tests in waves (no agent used in two concurrent tests) + if not args.solo: + # Schedule pairs so no agent appears twice per wave + pair_candidates = [ + ("claude", "pi"), + ("pi", "claude"), + ("claude", "codex"), + ] + pairs = [(a, b) for a, b in pair_candidates if a in agents and b in agents] + + # Greedy wave scheduling: each wave has no agent overlap + waves = [] + remaining = list(pairs) + while remaining: + wave = [] + used = set() + still_remaining = [] + for impl_a, revw_a in remaining: + if impl_a not in used and revw_a not in used: + wave.append((impl_a, revw_a)) + used.update([impl_a, revw_a]) + else: + still_remaining.append((impl_a, revw_a)) + waves.append(wave) + remaining = still_remaining + + for i, wave in enumerate(waves): + print(f"\n--- Pair wave {i+1}: {', '.join(f'{a}+{b}' for a,b in wave)} ---") + with ThreadPoolExecutor(max_workers=len(wave)) as pool: + futures_map = {} + for impl_a, revw_a in wave: + f = pool.submit(run_pair_test, impl_a, revw_a, args.timeout) + futures_map[f] = f"pair-{impl_a}+{revw_a}" + collect(futures_map) + + # Sort by name for stable output + results.sort(key=lambda r: r.name) + print_results(results) + sys.exit(0 if all(r.passed for r in results) else 1) + + +if __name__ == "__main__": + main() diff --git a/todos/run-cmd-test.md b/todos/run-cmd-test.md index 3ad1687..fd0f588 100644 --- a/todos/run-cmd-test.md +++ b/todos/run-cmd-test.md @@ -1,6 +1,6 @@ ## Spec -command run test +skill run test ## Plan diff --git a/todos/run-commands.md b/todos/run-commands.md index dcf01af..9972fe1 100644 --- a/todos/run-commands.md +++ b/todos/run-commands.md @@ -1,6 +1,6 @@ ## Spec -Rally's `/rally` wrapper is installed across coding agents (Claude Code, Codex, Pi, Droid) and calls `rally command run "$@"`. Today, session creation is a separate out-of-band step — someone must run `rally create implement --name X --todo Y --workspace . --reviewers 2` before agents can use `/rally`. This means the user has to leave each agent, run setup commands, then come back and tell agents how to join. The goal is zero-setup: one slash command per agent and everything else is automatic. +Rally's `/rally` wrapper is installed across coding agents (Claude Code, Codex, Pi, Droid) and calls `rally skill run "$@"`. Today, session creation is a separate out-of-band step — someone must run `rally create implement --name X --todo Y --workspace . --reviewers 2` before agents can use `/rally`. This means the user has to leave each agent, run setup commands, then come back and tell agents how to join. The goal is zero-setup: one slash command per agent and everything else is automatic. Why this is being done: - session creation is manual and error-prone — the user must coordinate session names, todo paths, workspace, and reviewer counts across multiple agents @@ -20,9 +20,9 @@ Exact build target (what will exist when done): - new `run_command()` default method on `Workflow` trait returning `Option<&dyn RunCommand>` - `WorkflowRegistry` gains `resolve_command(cmd)` and `list_commands()` methods - `BuildWorkflow` implements `run_command()` returning an `ImplementRunCommand` that handles `/rally implement [--reviewers N]` -- `rally command run` dispatches to workflow run commands before falling through to existing invite/saved-context resolution +- `rally skill run` dispatches to workflow run commands before falling through to existing invite/saved-context resolution - create-or-join logic: first agent creates session, concurrent agents catch "already exists" and join -- `rally command doctor` shows available workflow commands +- `rally skill doctor` shows available workflow commands - session naming: `implement-{todo-stem}` (deterministic, one session per todo file) Exact approach (technical): @@ -55,16 +55,16 @@ Acceptance criteria: - `BuildWorkflow::run_command()` returns `Some(&ImplementRunCommand)`. - `normalize_agent_selector` is made `pub(crate)` in `command_surface.rs`. -3. Add create-or-join dispatch path in `rally command run` +3. Add create-or-join dispatch path in `rally skill run` Acceptance criteria: - `run()` in `command_surface.rs` checks `args.first` against `registry.resolve_command()` before existing invite/saved-context resolution. - On match: collects `[args.second] + args.extra` as command args, calls `resolve()`, then `create_or_join()`, then `ensure_agent_registered()` + `next_with_registry()`. - `create_or_join()` helper tries `create_with_registry()`; on "already exists" error, falls through to join. - Race-safe: two agents running simultaneously both succeed (one creates, one joins). -- Saves resolved session+agent to `command-context.json` so subsequent bare `/rally` calls resume correctly. +- Saves resolved session+agent to `skill-context.json` so subsequent bare `/rally` calls resume correctly. - Non-matching first args fall through to existing resolution logic unchanged. -4. Enhance `rally command doctor` to show available commands +4. Enhance `rally skill doctor` to show available commands Acceptance criteria: - `doctor()` signature accepts `&WorkflowRegistry`. - After printing per-target installation status, prints available workflow commands with name, description, and usage. @@ -75,11 +75,11 @@ Acceptance criteria: - `ImplementRunCommand::resolve()` unit tests: valid args produce correct session name and `CreateArgs`; missing args produce usage error; bad todo path produces clear error; `--reviewers` parsing works. - `create_or_join()` tests: first-create path succeeds; already-exists path falls through to join. - `resolve_command()` and `list_commands()` tests on `WorkflowRegistry`. -- Integration test: `rally command run implement ` creates session, joins agent, returns instruction. +- Integration test: `rally skill run implement ` creates session, joins agent, returns instruction. 6. Manual QA (user-run) Acceptance criteria: -- User runs `rally command install --target all` and `rally command doctor --target all` and confirms all installed targets show available commands. +- User runs `rally skill install --target all` and `rally skill doctor --target all` and confirms all installed targets show available commands. - User runs `/rally implement todos/create-todo.md implementer` in one agent and `/rally implement todos/create-todo.md reviewer` in another; both succeed without prior session creation. - User confirms `rally sessions` shows the auto-created session and `rally status --session implement-create-todo` shows both agents. - User runs bare `/rally` in same workspace and confirms it resumes the saved session.