diff --git a/.claude/agents/marketplace-package-installer.md b/.claude/agents/marketplace-package-installer.md index bd6bef34b..0f5174621 100644 --- a/.claude/agents/marketplace-package-installer.md +++ b/.claude/agents/marketplace-package-installer.md @@ -21,6 +21,8 @@ This agent handles package installation from marketplace registries, including s - Copy package artifacts (commands, skills, agents, scripts) - Set correct file permissions (especially for scripts) - Update registry.yaml with installed agents +- Run a package's optional `install.py` hooks (`prepare`/`complete`/`cleanup`) around the copy and uninstall steps +- Pass package-specific `--set KEY=VALUE` args through to those hooks when a package's hook requires them - Verify installation success - Provide detailed installation feedback @@ -43,6 +45,7 @@ Use this agent when the user wants to: - `registry`: Specific registry to use (default: auto-detect) - `force`: Overwrite existing files (default: false) - `no_expand`: Skip token expansion (default: false) +- `set_args`: Extra `KEY=VALUE` pairs (repeatable) forwarded to the package's `install.py` hooks as `options["args"]`, only needed when a package's hook reports a missing required value ## Outputs @@ -132,11 +135,15 @@ result = install_marketplace_package( **Installation steps:** 1. Validate package and scope 2. Create destination directories -3. Copy artifacts (commands, skills, agents, scripts) -4. Set executable permissions on scripts -5. Perform token expansion (if enabled) -6. Update ~/.claude/agents/registry.yaml -7. Verify all files copied successfully +3. If the package ships an `install.py`, run its `prepare(source_path, destination_path, options)` hook (per install target: `.claude`, and `.codex` when both are installed) — a failing `prepare()` stops the install for that target before anything is copied +4. Copy artifacts (commands, skills, agents, scripts) +5. Set executable permissions on scripts +6. Perform token expansion (if enabled) +7. Update ~/.claude/agents/registry.yaml +8. If the package ships an `install.py`, run its `complete(source_path, destination_path, options)` hook — this is where a package renders repo-specific output (e.g. via `sc-compose render`) or does its own version-to-version cleanup; a failure here is also reported and stops that target +9. Verify all files copied successfully + +`options` passed to every hook: `global`, `local`, `user`, `project`, `codex`, `force`, `expand`, plus `args` (the `set_args` dict, empty unless the caller supplied `--set`). A hook's result is `{"result": "success"}` or `{"result": "fail", "message": ""}` — surface that `message` directly to the user/caller rather than a generic failure. ### 5. Verify Installation @@ -251,6 +258,26 @@ Troubleshooting: 4. Wait and retry if registry server is down ``` +### install.py Hook Failure + +A package's `prepare()`, `complete()`, or (on uninstall) `cleanup()` hook can +fail with `{"result": "fail", "message": ""}`. Relay that +`message` verbatim — it's written to carry both the reason and the fix. If it +names a missing value, retry the install with the matching `--set KEY=VALUE`: + +``` +Error: install.py complete() failed: sc-compose render failed for +commands/sc-git-worktree.md: template requires REPO_NAME +-- suggested fix: pass --set REPO_NAME= or run from inside a git repo + +Troubleshooting: +1. Follow the fix instructions in the message above +2. Retry with the suggested flag, e.g.: + /marketplace install --local --set REPO_NAME=my-project +3. If the hook itself raised an unhandled exception, treat it as a bug in + the package (not a user-fixable input problem) and report it +``` + ## Integration with CLI This agent uses the skill integration module: diff --git a/.claude/skills/marketplace/README.md b/.claude/skills/marketplace/README.md index 6937e30f6..89c5905e6 100644 --- a/.claude/skills/marketplace/README.md +++ b/.claude/skills/marketplace/README.md @@ -193,10 +193,20 @@ When you view package details, you'll see: 1. **Validation**: Checks package exists and is accessible 2. **Dependency Check**: Verifies required dependencies -3. **File Copy**: Installs commands, skills, agents, and scripts -4. **Configuration**: Updates registry and config files -5. **Verification**: Confirms all files installed correctly -6. **Feedback**: Shows installation summary +3. **Prepare Hook**: Runs the package's `install.py` `prepare()`, if it ships one (before anything is copied) +4. **File Copy**: Installs commands, skills, agents, and scripts +5. **Configuration**: Updates registry and config files +6. **Complete Hook**: Runs the package's `install.py` `complete()`, if it ships one +7. **Verification**: Confirms all files installed correctly +8. **Feedback**: Shows installation summary + +Most packages have no `install.py` and these hook steps are simply skipped. +A package that does ship one may need extra info sc-install has no generic +way to know; if a hook fails for that reason, it reports a message naming +what's missing, and you retry with the named flag, e.g.: +```bash +/marketplace install some-package --local --set TARGET_ENV=staging +``` ## Registry Management diff --git a/.claude/skills/marketplace/SKILL.md b/.claude/skills/marketplace/SKILL.md index 9c74ae4fd..c27f3ade1 100644 --- a/.claude/skills/marketplace/SKILL.md +++ b/.claude/skills/marketplace/SKILL.md @@ -21,6 +21,7 @@ Discover and manage Synaptic Canvas marketplace packages through natural convers - **Scope Control**: Install globally (--global) or locally (--local) - **Version Management**: Track installed package versions - **Dependency Resolution**: View and manage package dependencies +- **Install-Time Hooks**: Runs a package's optional `install.py` `prepare()`/`complete()` hooks around the copy step, and `cleanup()` on uninstall, if the package ships one - **Installation Verification**: Confirm successful installation with file checks ### Registry Management @@ -165,6 +166,7 @@ Unified command interface for marketplace operations. - `--local` - Install to ./.claude-local (project-level) - `--force` - Overwrite existing files - `--registry ` - Use specific registry +- `--set KEY=VALUE` - Repeatable; extra info for a package's optional `install.py` hooks (see Troubleshooting below) ## Examples @@ -251,6 +253,7 @@ See [TROUBLESHOOTING.md](./TROUBLESHOOTING.md) for common issues and solutions. - **Package not found**: Verify package name and registry configuration - **Installation fails**: Check write permissions and disk space - **Command not found**: Ensure `sc-install` is in PATH +- **`install.py` hook failed**: the error message names the reason and the fix; if it names a missing value, retry with `--set KEY=VALUE` ## Use Cases diff --git a/.claude/skills/marketplace/TROUBLESHOOTING.md b/.claude/skills/marketplace/TROUBLESHOOTING.md index 77961dd47..25b1d9330 100644 --- a/.claude/skills/marketplace/TROUBLESHOOTING.md +++ b/.claude/skills/marketplace/TROUBLESHOOTING.md @@ -511,6 +511,63 @@ echo "cache_enabled: true" >> ~/.claude/config.yaml --- +### Issue 9: "install.py hook failed" or "missing required value" + +**Symptoms:** +- Error: "install.py complete() failed: -- suggested fix: pass --set KEY=VALUE ..." +- Installation aborts after files were validated but before/after the copy step +- Works for `--global`/`--user` but fails for `--local`/`--project` (or vice versa) + +**Possible Causes:** +1. The package ships an optional `install.py` (`prepare()`/`complete()`/`cleanup()` + hooks) and one of them needs information `sc-install` has no generic way to + know (e.g. a target environment name, a repo-specific value) +2. The required `--set KEY=VALUE` wasn't passed +3. The hook itself is buggy (raises an exception, or returns something other + than `{"result": "success"}` / `{"result": "fail", "message": "..."}`) + +**Solutions:** + +**Solution 9A: Read the Failure Message** +The hook's `message` is required to combine *why it failed* and *what to do +about it* in one string. Most of the time the fix is right there: +``` +Error: install.py complete() failed: sc-compose render failed for +commands/sc-example.md: template requires TARGET_ENV -- suggested fix: +pass --set TARGET_ENV= +``` + +**Solution 9B: Retry with `--set`** +```bash +# Add the named key/value and retry +/marketplace install some-package --local --set TARGET_ENV=staging + +# --set is repeatable for hooks that need more than one value +sc-install install some-package --local --set TARGET_ENV=staging --set REGION=us-east-1 +``` + +**Solution 9C: Uninstall Cleanup Also Runs Hooks** +`sc-install uninstall` runs the package's `cleanup()` hook (if it has one) +after removing the manifest artifacts. If uninstall reports a hook failure, +the same `--set KEY=VALUE` pattern applies: +```bash +sc-install uninstall some-package --dest ~/.claude --set TARGET_ENV=staging +``` + +**Solution 9D: If the Message Doesn't Name a Fix** +That's a bug in the package's `install.py`, not a missing input on your end - +every hook result must be `{"result": "success"}` or `{"result": "fail", +"message": ""}`. Report it against the package +(see "Reporting Issues" below) rather than retrying blindly. + +**Prevention:** +- Read the package's own README before installing if it documents required + `--set` keys +- See `src/sc_cli/README.md` in the repository for the full `install.py` + hook contract + +--- + ## Diagnostic Commands ### Check System Health diff --git a/.claude/skills/marketplace/USE-CASES.md b/.claude/skills/marketplace/USE-CASES.md index 7a70aa341..9e2c044ad 100644 --- a/.claude/skills/marketplace/USE-CASES.md +++ b/.claude/skills/marketplace/USE-CASES.md @@ -509,6 +509,63 @@ Issue resolved ✓ --- +## Use Case 7b: Install a Package That Requires Hook-Based Customization + +**Scenario**: A package ships an optional `install.py` with `prepare()`/ +`complete()`/`cleanup()` hooks, and one of them needs a value `sc-install` +has no generic way to know (for example, a target environment name). + +**Goal**: Successfully install the package by supplying the missing value +via `--set KEY=VALUE`, without needing to understand the package's internals. + +### Steps + +1. **Attempt a normal install:** + ``` + /marketplace install some-package --local + ``` + +2. **The hook fails with a message naming what's missing:** + ``` + Error: install.py complete() failed: sc-compose render failed for + commands/sc-example.md: template requires TARGET_ENV -- suggested fix: + pass --set TARGET_ENV= + ``` + +3. **Retry with the named value:** + ``` + /marketplace install some-package --local --set TARGET_ENV=staging + ``` + +4. **`--set` is repeatable if more than one value is required:** + ``` + sc-install install some-package --local --set TARGET_ENV=staging --set REGION=us-east-1 + ``` + +5. **The same pattern applies to uninstall**, since `cleanup()` hooks + receive `--set` the same way: + ``` + sc-install uninstall some-package --dest ~/.claude --set TARGET_ENV=staging + ``` + +### Expected Outcome + +- Installation succeeds once the required value is supplied +- No need to read the package's `install.py` source to know what to pass - + the failure message names it +- Understanding that `sc-install` itself never interprets `--set` values; + only the package's own hook does + +### Variations + +- **Package needs no hook values**: most packages have no `install.py` at + all, or one that derives everything it needs automatically (e.g. + `sc-git-worktree` infers the repo name); these steps are simply skipped +- **Hook fails without naming a fix**: that's a bug in the package, not a + missing input - see Use Case 7 (Troubleshoot Package Issues) + +--- + ## Advanced Use Cases ### Use Case 8: Create Custom Registry for Team Packages diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56ebb796e..b3d970f69 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -266,6 +266,18 @@ requires: - Optionally specify version constraints - Document installation instructions in README +### install.py (Tier 3: install-time hooks) + +Not a manifest field - a package that needs more than token substitution or +runtime-dependency checks may ship `packages//install.py`, discovered by +convention (sc-install checks for the file's presence, nothing declares it +in manifest.yaml). It defines any of `prepare()`, `complete()`, `cleanup()`, +run around the artifact copy step and around uninstall, and can require +extra `--set KEY=VALUE` info from the caller by failing with an actionable +message when it's missing. Full contract, the result shape, and the +idempotency/cleanup requirements every `install.py` must meet: see +`src/sc_cli/README.md`. + ## Version Management ### Three-Layer Versioning System diff --git a/docs/MARKETPLACE-INFRASTRUCTURE.md b/docs/MARKETPLACE-INFRASTRUCTURE.md index bdf256f68..4e1794e96 100644 --- a/docs/MARKETPLACE-INFRASTRUCTURE.md +++ b/docs/MARKETPLACE-INFRASTRUCTURE.md @@ -847,6 +847,11 @@ requires: - python3 >= 3.10 ``` +Not a manifest field, but a package may also ship `install.py` at its root +(Tier 3, discovered by presence rather than declared in manifest.yaml) with +`prepare()`/`complete()`/`cleanup()` hooks run by sc-install around the copy +and uninstall steps. See `src/sc_cli/README.md` for the full contract. + ### Package Installation Flow When a user installs a package: @@ -859,9 +864,11 @@ When a user installs a package: b. Finds package entry c. Fetches manifest.yaml from package path d. Downloads all artifacts listed in manifest - e. Performs token substitution (Tier 1) - f. Validates dependencies (Tier 2) - g. Copies artifacts to ~/.claude/ or ./.claude/ + e. Runs the package's install.py prepare() hook, if present (Tier 3) + f. Performs token substitution (Tier 1) + g. Validates dependencies (Tier 2) + h. Copies artifacts to ~/.claude/ or ./.claude/ + i. Runs the package's install.py complete() hook, if present (Tier 3) 3. Result: .claude/ diff --git a/packages/sc-git-worktree/README.md b/packages/sc-git-worktree/README.md index 440e9efea..e092782ec 100644 --- a/packages/sc-git-worktree/README.md +++ b/packages/sc-git-worktree/README.md @@ -37,8 +37,8 @@ Create, scan, clean up, and abort worktrees using predictable paths and safe def - `/sc-git-worktree --abort ` Defaults -- Worktree base: `../{{REPO_NAME}}-worktrees/` -- Tracking file: `../{{REPO_NAME}}-worktrees/worktree-tracking.jsonl` +- Worktree base: `../-worktrees/` where `` is derived at runtime from `basename $(git rev-parse --show-toplevel)`. +- Tracking file: `../-worktrees/worktree-tracking.jsonl` Safety - Never delete unmerged branches without explicit approval @@ -71,11 +71,11 @@ See [DESIGN.md](DESIGN.md) for detailed requirements including: ## Troubleshooting - "Path exists": Ensure `../-worktrees/` is not present (or choose a different branch) - "Dirty worktree": Commit or stash before cleanup/abort -- Token expansion: `{{REPO_NAME}}` is auto-detected from the repo toplevel (via git) +- `` is derived at runtime by the agent (`basename $(git rev-parse --show-toplevel)`), not substituted at install time — this keeps global/user installs safe (see issue #112). ## Components - Command: `commands/sc-git-worktree.md` -- Skill: `skills/sc-managing-worktrees/SKILL.md` +- Skill: `skills/sc-git-worktree/SKILL.md` - Agents: `sc-worktree-create`, `sc-worktree-create-stacked`, `sc-worktree-scan`, `sc-worktree-cleanup`, `sc-worktree-abort`, `sc-worktree-update` - Stack layers: `skills/sc-git-worktree/references/stack-layers.md` (pairs with the `sc-gh-stack` package) diff --git a/packages/sc-git-worktree/agents/sc-git-worktree-update.md b/packages/sc-git-worktree/agents/sc-git-worktree-update.md index bef1ac5bb..5d337a42a 100644 --- a/packages/sc-git-worktree/agents/sc-git-worktree-update.md +++ b/packages/sc-git-worktree/agents/sc-git-worktree-update.md @@ -23,7 +23,7 @@ Safely update protected branches (main, develop, master) in their worktrees by p ## Inputs (required unless noted) - branch: protected branch name to update (optional; if omitted, update all protected branches that have worktrees) - path: worktree path (default `/`) -- worktree_base (optional): defaults to `../{{REPO_NAME}}-worktrees` +- worktree_base (optional): defaults to `../-worktrees` where `` is derived at runtime from `basename $(git rev-parse --show-toplevel)` - protected_branches: list of protected branch names (required for validation) - tracking_enabled: true/false (default true) - tracking_path (optional): defaults to `/worktree-tracking.jsonl` when tracking is enabled diff --git a/packages/sc-git-worktree/agents/sc-git-worktree-update.md.local.j2 b/packages/sc-git-worktree/agents/sc-git-worktree-update.md.local.j2 new file mode 100644 index 000000000..248f1f5d6 --- /dev/null +++ b/packages/sc-git-worktree/agents/sc-git-worktree-update.md.local.j2 @@ -0,0 +1,181 @@ +--- +name: sc-worktree-update +version: 0.14.0 +description: Update a protected branch in its worktree by pulling latest changes. Handle merge conflicts by returning control to main agent for user coordination. +model: haiku +color: blue +--- + +# Worktree Update Agent + +## Invocation + +This agent is invoked via the Claude Task tool by a skill or command. Do not invoke directly. + +## Input Protocol + +Read inputs from `` (JSON object). If omitted, treat as `{}`. + +## Purpose + +Safely update protected branches (main, develop, master) in their worktrees by pulling latest changes from remote. Return control to caller if merge conflicts occur. + +## Inputs (required unless noted) +- branch: protected branch name to update (optional; if omitted, update all protected branches that have worktrees) +- path: worktree path (default `/`) +- worktree_base (optional): defaults to `../{{ REPO_NAME }}-worktrees` +- protected_branches: list of protected branch names (required for validation) +- tracking_enabled: true/false (default true) +- tracking_path (optional): defaults to `/worktree-tracking.jsonl` when tracking is enabled +- cache_protected_branches (optional): defaults to `true`. When `false`, do not write `.sc/shared-settings.yaml`. + +## Rules +- **Only operates on protected branches** - error if requested branch not in protected_branches list. If branch is omitted, iterate all protected branches with existing worktrees. +- Never proceed if worktree is dirty (uncommitted changes) +- Never create or delete branches - only update existing ones +- On merge conflicts, return detailed error for caller to coordinate resolution +- If tracking enabled, update last_checked timestamp on successful pull + +## Execution + +Run the update script once with the input JSON: + +```bash +python3 .claude/scripts/worktree_update.py '' +``` + +The script handles validation, protected branch resolution, update logic, and tracking updates. + +## Output Format + +Return fenced JSON with minimal envelope: + +### Success (clean pull) + +````markdown +```json +{ + "success": true, + "data": { + "action": "update", + "branch": "main", + "path": "../repo-worktrees/main", + "commits_pulled": 5, + "old_commit": "abc1234", + "new_commit": "def5678", + "tracking_update": "last_checked updated" + }, + "error": null +} +``` +```` + +### Success (already up to date) + +````markdown +```json +{ + "success": true, + "data": { + "action": "update", + "branch": "main", + "path": "../repo-worktrees/main", + "commits_pulled": 0, + "old_commit": "abc1234", + "new_commit": "abc1234", + "message": "already up to date", + "tracking_update": "last_checked updated" + }, + "error": null +} +``` +```` + +### Error (merge conflicts) + +````markdown +```json +{ + "success": false, + "data": null, + "error": { + "code": "merge.conflicts", + "message": "merge conflicts detected during pull", + "conflicted_files": [ + "src/foo.cs", + "src/bar.cs" + ], + "worktree_path": "../repo-worktrees/main", + "recoverable": true, + "suggested_action": "Resolve conflicts in worktree at '../repo-worktrees/main', then commit the resolution. Run 'git status' to see conflict details." + } +} +``` +```` + +### Error (not a protected branch) + +````markdown +```json +{ + "success": false, + "data": null, + "error": { + "code": "branch.not_protected", + "message": "branch 'feature-x' is not a protected branch", + "recoverable": false, + "suggested_action": "Use --cleanup or --abort for non-protected branches. --update is only for protected branches like main, develop, master." + } +} +``` +```` + +### Error (dirty worktree) + +````markdown +```json +{ + "success": false, + "data": null, + "error": { + "code": "worktree.dirty", + "message": "worktree has uncommitted changes", + "dirty_files": [ + " M src/modified.cs", + "?? src/untracked.txt" + ], + "recoverable": true, + "suggested_action": "Commit or stash changes in worktree before updating" + } +} +``` +```` + +## Output Protocol + +Wrap the script output in `` tags with a fenced JSON block. Do not add prose outside the tags. + +## Constraints + +- Do NOT proceed if branch is not in protected_branches list +- Do NOT proceed if worktree is dirty +- Do NOT run manual git commands; use the script only +### Success (multi-branch aggregate) + +````markdown +```json +{ + "success": true, + "data": { + "action": "update", + "results": { + "main": {"commits_pulled": 3, "status": "updated"}, + "develop": {"commits_pulled": 0, "status": "up_to_date"} + }, + "conflicts": {}, + "tracking_update": "last_checked updated" + }, + "error": null +} +``` +```` diff --git a/packages/sc-git-worktree/commands/sc-git-worktree.md b/packages/sc-git-worktree/commands/sc-git-worktree.md index 374124e07..70c471296 100644 --- a/packages/sc-git-worktree/commands/sc-git-worktree.md +++ b/packages/sc-git-worktree/commands/sc-git-worktree.md @@ -53,8 +53,8 @@ Use this command to manage worktrees following the repo's layout and tracking ru Defaults: - Repo root: current directory. -- Worktree base: `../{{REPO_NAME}}-worktrees/`. -- Tracking file: `../{{REPO_NAME}}-worktrees/worktree-tracking.jsonl` (disable or override if tracking is not used). +- Worktree base: `../-worktrees/` where `` is derived at runtime from `basename $(git rev-parse --show-toplevel)`. +- Tracking file: `../-worktrees/worktree-tracking.jsonl` (disable or override if tracking is not used). ## Protected Branches Configuration diff --git a/packages/sc-git-worktree/commands/sc-git-worktree.md.local.j2 b/packages/sc-git-worktree/commands/sc-git-worktree.md.local.j2 new file mode 100644 index 000000000..9c91358ae --- /dev/null +++ b/packages/sc-git-worktree/commands/sc-git-worktree.md.local.j2 @@ -0,0 +1,127 @@ +--- +name: sc-git-worktree +description: Manage git worktrees for this repo (create, list/status, update, cleanup, abort) while enforcing the repo's worktree/tracking rules and protected branch safeguards. +version: 0.14.0 +options: + - name: --list + description: List worktrees and show status/notes. + - name: --status + description: Alias for --list (show worktree status and tracking sync). + - name: --create + args: + - name: branch + description: Branch name to create/use for the worktree. + - name: base + description: Base branch to start from (e.g., master, develop, release/x.y, hotfix/...). + description: Create a worktree (and branch if needed) using the mandated layout and update tracking. + - name: --create-stacked + args: + - name: branch + description: New gh-stack layer name (must not exist yet). + - name: parent + description: Layer to cut from - the current stack top from /sc-gh-stack-view, or the trunk for the bottom layer, or a mid-stack layer when inserting. + - name: trunk + description: Branch the stack's bottom PR targets (e.g., develop, integrate/phase-x). + description: Cut a new stack layer worktree from the parent's PUSHED head (--no-track), validate the cut, record the parent SHA, and return a stack_handoff for the layer's writer. Add `--above ` when inserting under an existing layer. + - name: --above + args: + - name: layer + description: With --create-stacked only - the layer currently stacked directly on , when inserting mid-stack. + description: Modifier for --create-stacked; marks the cut as a mid-stack insert and adds the insert steps to the handoff. + - name: --update + args: + - name: branch + description: Protected branch name to update (e.g., main, develop). + description: Pull latest changes for protected branches in their worktrees. If a branch is specified, update only that branch; if omitted, update all protected branches. Handle merge conflicts interactively by notifying user and coordinating resolution. + - name: --cleanup + args: + - name: branch + description: Branch/worktree name to clean up (post-merge or finished work). + description: Remove a worktree; for non-protected branches, delete local and remote branch by default if merged/no unique commits (only keep if user opts out); for protected branches, only remove worktree and preserve branch; update tracking. + - name: --abort + args: + - name: branch + description: Branch/worktree name to abandon (discard work). + description: Abandon a worktree (delete worktree, optionally delete branch for non-protected branches) with explicit approval if dirty. Protected branches are never deleted. + - name: --help + description: Show available options and guidance. +--- + +# /sc-git-worktree command + +Use this command to manage worktrees following the repo's layout and tracking rules. You MUST invoke the appropriate subagent via the Task tool; do not run manual git commands in the primary session. + +Defaults: +- Repo root: current directory. +- Worktree base: `../{{ REPO_NAME }}-worktrees/`. +- Tracking file: `../{{ REPO_NAME }}-worktrees/worktree-tracking.jsonl` (disable or override if tracking is not used). + +## Protected Branches Configuration + +Protected branches (main, develop, master) require special handling to prevent accidental deletion. Configure using: + +```yaml +git: + protected_branches: + - "main" + - "develop" + - "master" +``` + +**Protected Branch Rules:** +- Cleanup/abort operations NEVER delete protected branches (local or remote) +- Protected branches can only be removed from worktrees, never deleted +- Use `--update` to safely pull changes for protected branches in worktrees +- Protected branches are read from `.sc/shared-settings.yaml` (`git.protected_branches`) +- If not configured, protected branches are auto-detected from git-flow and cached to `.sc/shared-settings.yaml` +- **Required**: Operations fail if protected branches cannot be determined + +If run with no options or `--help`: print a concise list of options (no git status) and prompt with a numbered choice for list/status, create, create-stacked, cleanup, or abort; then gather required inputs. + +## Behavior + +## Task Tool Invocation (Required) + +Use the Task tool with `` and consume `` from the subagent response. No manual git commands in the primary session. + +### Template + +```xml + +$SUBAGENT +$DESCRIPTION +Run $SUBAGENT with this input: + + +```json +$INPUT_JSON +``` + + + +``` + +### --list / --status +MUST invoke `sc-worktree-scan` and render its `` summary and recommendations. + +### --create +MUST invoke `sc-worktree-create` with `branch`, `base`, `purpose`, `owner`, and optional tracking inputs. Render the `` summary. + +### --create-stacked +MUST invoke `sc-worktree-create-stacked` with `branch`, `base` (= ``), `stack: {"trunk": "", "above": "" | null}`, `purpose`, `owner`, and optional tracking inputs. Never route a stack layer to `sc-worktree-create`: it branches from the local ref. + +Before invoking, when `sc-gh-stack` is installed, run `/sc-gh-stack-view` so `` is the pushed top (or, for an insert, so `--above` names the real layer above the parent). Without it, the script still verifies the cut from git. + +Render the `` summary **and the full `stack_handoff` block verbatim**. Hand `stack_handoff.writer` unchanged to the agent that will work in the worktree, and `stack_handoff.stack_writer` to the stack writer (the one agent that opens PRs and runs gh stack write commands; never the layer's writer). See `skills/sc-git-worktree/references/stack-layers.md`. + +### --update +MUST invoke `sc-worktree-update` for protected branches only. Render conflicts or success from ``. + +### --cleanup +MUST invoke `sc-worktree-cleanup`. Batch cleanup reconciles JSONL first, captures untracked local worktrees, then cleans tracked worktrees only. Render the `` summary. + +### --abort +MUST invoke `sc-worktree-abort` and render the `` summary. + +### --help +Show options and remind about base branches, paths, tracking toggles, and dirty-worktree safeguards. Keep output concise (no tool traces). diff --git a/packages/sc-git-worktree/manifest.yaml b/packages/sc-git-worktree/manifest.yaml index f0fca621d..5e571e917 100644 --- a/packages/sc-git-worktree/manifest.yaml +++ b/packages/sc-git-worktree/manifest.yaml @@ -36,12 +36,6 @@ artifacts: - scripts/worktree_abort.py - scripts/worktree_update.py -# Token substitution (Tier 1 package) -variables: - REPO_NAME: - auto: git-repo-basename - description: Repository name used for default worktree and tracking paths - # Installation policy/metadata install: scope: local-only # this package may only be installed in a repo's .claude (not global) diff --git a/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md b/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md index 4d61a90a2..f3fee69be 100644 --- a/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md +++ b/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md @@ -1,5 +1,5 @@ --- -name: sc-managing-worktrees +name: sc-git-worktree description: Create, manage, scan, update, and clean up git worktrees for parallel development with protected branch safeguards. Use when working on multiple branches simultaneously, isolating experiments, updating protected branches (main/develop), @@ -60,7 +60,7 @@ Details and the handoff contract: `references/stack-layers.md`. The stack model, ## Standards and Paths - Repo root: current directory. -- Default worktree base: `../{{REPO_NAME}}-worktrees`. +- Default worktree base: `../-worktrees` where `` is the basename of the repo root directory, derived at runtime via `basename $(git rev-parse --show-toplevel)` (e.g. repo `my-project` → `../my-project-worktrees`). - Worktrees live in `/`. - Tracking file (if used): `/worktree-tracking.jsonl` must be updated on create/scan/cleanup/abandon. Allow a toggle to disable tracking for repos that don't use it. - Naming: worktree directory = branch name; branch naming follows repo policy. diff --git a/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md.local.j2 b/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md.local.j2 new file mode 100644 index 000000000..9df092568 --- /dev/null +++ b/packages/sc-git-worktree/skills/sc-git-worktree/SKILL.md.local.j2 @@ -0,0 +1,96 @@ +--- +name: sc-git-worktree +description: + Create, manage, scan, update, and clean up git worktrees for parallel development with protected branch safeguards. + Use when working on multiple branches simultaneously, isolating experiments, updating protected branches (main/develop), + cutting a gh-stack layer as a worktree (top or mid-stack insert), or when user mentions "worktree", "parallel branches", + "feature isolation", "branch cleanup", "worktree status", "update main/develop", "stack layer worktree", or "stacked worktree". +version: 0.14.0 +entry_point: /sc-git-worktree +--- + +# Managing Git Worktrees + +Use this skill to manage worktrees with a standard structure and tracking. Use the `/sc-git-worktree` command to invoke this skill. + +## Agent Delegation (Required) + +This skill delegates all execution to specialized agents via the **Task tool** (no manual git commands in the primary session). +Always pass inputs via `` and render `` from the subagent response. + +**Task tool template:** + +```xml + +$SUBAGENT +$DESCRIPTION +Run $SUBAGENT with this input: + + +```json +$INPUT_JSON +``` + + + +``` + +| Operation | Agent | Returns | +|-----------|-------|---------| +| Create | `sc-worktree-create` | JSON: success, path, branch, tracking_entry | +| Create stack layer | `sc-worktree-create-stacked` | JSON: success, path, branch, stack, stack_handoff | +| Scan | `sc-worktree-scan` | JSON: success, worktrees list, recommendations | +| Cleanup | `sc-worktree-cleanup` | JSON: success, branch_deleted, tracking_update | +| Abort | `sc-worktree-abort` | JSON: success, worktree_removed, tracking_update | +| Update | `sc-worktree-update` | JSON: success, commits_pulled, conflicts (if any) | + +To invoke an agent, use the Task tool with the agent prompt and pass parameters exactly as documented in the agent Inputs section. + +**Routing rule:** a worktree that is a gh-stack layer (the user says "stack", "layer", "on top of ", "insert under", or the branch will be linked with `gh stack`) goes to `sc-worktree-create-stacked`. Everything else goes to `sc-worktree-create`. + +## Stack Layers (gh-stack) + +A stack layer is a worktree whose branch will be a PR in a `gh stack`. The plain create is wrong for it: it branches from the **local** base ref (possibly stale or another writer's unpushed state) and, from a remote ref, would leave the parent as upstream. `sc-worktree-create-stacked` cuts from `origin/` with `--no-track`, refuses bad cuts before touching anything (parent not pushed, already landed, insert target not stacked on the parent), records the parent SHA in tracking, and returns a `stack_handoff` block. + +- `stack_handoff.writer` goes verbatim to the agent that will work in the worktree (WIP commit, first push with `-u`, at most one rebase at task start, no edits to lower layers, no gh stack write commands). `stack_handoff.stack_writer` goes to the stack writer only (PR with base = parent, `gh stack checkout` in the new worktree, link on top, or the insert sequence: merge-forward by every layer above, unstack, `gh pr edit --base`, full relink). +- Cleanup refuses to delete a branch that live layers sit on unless git shows a merge-commit landing in the trunk (`STACK.HAS_CHILDREN`; batch cleanup reports `stack_blocked`, and never sweeps a fresh layer with no commits). Abort always refuses. Both read the tracking file. +- Scan reports `stack_parent_advanced` (layer no longer contains the parent's head) and `stack_parent_landed` (PR should now target the trunk) on layer rows. + +Details and the handoff contract: `references/stack-layers.md`. The stack model, recipes and the view tool live in the `sc-gh-stack` package (`/sc-gh-stack`, `/sc-gh-stack-view`); this skill only makes the worktree side of that model safe. + +## Standards and Paths +- Repo root: current directory. +- Default worktree base: `../{{ REPO_NAME }}-worktrees`. +- Worktrees live in `/`. +- Tracking file (if used): `/worktree-tracking.jsonl` must be updated on create/scan/cleanup/abandon. Allow a toggle to disable tracking for repos that don't use it. +- Naming: worktree directory = branch name; branch naming follows repo policy. +- Branch protections/hooks: no direct commits to protected branches. + +## Protected Branch Configuration + +Protected branches (main, develop, master, etc.) require special handling to prevent accidental deletion: + +```yaml +# .sc/shared-settings.yaml +git: + protected_branches: + - "main" + - "develop" + - "master" +``` + +**Protected Branch Rules:** +- Protected branches are read from `.sc/shared-settings.yaml` (`git.protected_branches`) +- If missing, protected branches are auto-detected from git-flow and cached to `.sc/shared-settings.yaml` +- **Cleanup/abort agents NEVER delete protected branches** (local or remote) +- Protected branches can only be removed from worktrees, never deleted +- Use `--update` to safely pull changes for protected branches in worktrees + +## Safety and reminders +- **NEVER delete protected branches** (main, develop, master) under any circumstances. +- Protected branches can only be removed from worktrees; the branch itself must always be preserved. +- Never delete branches or force-remove worktrees without explicit approval. +- Never clean/abandon a worktree with uncommitted changes unless explicitly approved. +- Keep tracking JSONL in sync on every operation when enabled. +- Respect branch protections and hooks; no direct commits to protected branches. +- Use background agents for worktree operations; keep the main context focused on decisions and summaries. diff --git a/scripts/validate-manifest-artifacts.py b/scripts/validate-manifest-artifacts.py index 0e7f58451..d553d8656 100755 --- a/scripts/validate-manifest-artifacts.py +++ b/scripts/validate-manifest-artifacts.py @@ -225,6 +225,11 @@ def get_disk_files(package_path: Path) -> list[str]: continue if file_path.is_file(): + # .local.j2 siblings are rendered in place of their plain + # manifest artifact (see src/sc_cli/README.md); they are not + # themselves manifest entries. + if file_path.name.endswith(".local.j2"): + continue # Get path relative to package root rel_path = file_path.relative_to(package_path) # Normalize to forward slashes for cross-platform compatibility diff --git a/src/sc_cli/README.md b/src/sc_cli/README.md new file mode 100644 index 000000000..2950b10e1 --- /dev/null +++ b/src/sc_cli/README.md @@ -0,0 +1,246 @@ +# sc-install + +`sc-install` (implementation: `install.py`, invoked via `tools/sc-install.py` or +`python3 -m sc_cli.sc_install`) installs and uninstalls Synaptic Canvas +packages from `packages/*/` into a `.claude` and/or `.codex` directory. This +document is the normative reference for what a package (`packages//`) +must provide, and what `sc-install` guarantees in return. + +## CLI + +``` +sc-install list [--registry NAME] [--all-registries] [--search QUERY] +sc-install info [--registry NAME] +sc-install search [--registry NAME] +sc-install install --dest [--force] [--no-expand] [--set K=V ...] +sc-install install --global [--claude|--codex] [--force] [--no-expand] [--set K=V ...] +sc-install install --local [--claude|--codex] [--force] [--no-expand] [--set K=V ...] +sc-install install --user [--claude|--codex] [--force] [--no-expand] [--set K=V ...] +sc-install install --project [--claude|--codex] [--force] [--no-expand] [--set K=V ...] +sc-install uninstall --dest [--set K=V ...] +sc-install registry add [--path ] +sc-install registry list +sc-install registry remove +``` + +- `--global`/`--user` resolve to `~/`; `--local`/`--project` resolve to `./`. + Exactly one of `--global`/`--local`/`--user`/`--project`/`--dest` is + required for `install`. +- `--claude`/`--codex` select which target(s) get installed under the chosen + scope: symmetric flags, either alone means only that target, neither means + both `.claude` and `.codex` are installed. An explicit `--dest` is always + `.claude`-only (there is no sibling `.codex` to mirror into). +- `--force` overwrites existing files instead of skipping them. +- `--no-expand` disables the legacy `{{TOKEN}}` expansion (see below). +- `--set KEY=VALUE` is repeatable and forwards arbitrary key/value pairs to a + package's `install.py` hooks (see below); `sc-install` itself never + interprets these values, only a package's own hook does. + +## `manifest.yaml` + +Every package must have `packages//manifest.yaml`: + +```yaml +name: sc-example # defaults to the directory name if omitted +version: 0.1.0 +description: > + One or more sentences describing the package. +author: your-name +license: MIT +tags: [tag-one, tag-two] + +# Files to install, relative to the package root. Every key is optional; +# omit a category entirely if the package ships none of it. +artifacts: + commands: + - commands/sc-example.md + skills: + - skills/sc-example/SKILL.md + agents: + - agents/sc-example-do-thing.md + scripts: + - scripts/example_helper.py # chmod +x'd automatically on install + assets: + - assets/example-template.txt + plugin: + - plugin.json # or any other manifest-adjacent artifact + +# Optional installation policy +install: + scope: local-only # or omit; local-only packages refuse --global/--user + +# Optional install-time flags a package documents for its own consumers +# (informational only - sc-install does not read or enforce this section; +# a package's install.py reads its own required values out of options["args"] +# instead, see below) +options: + some-flag: + type: boolean + default: false + description: What this flag changes about behavior at runtime. + +# Legacy token-expansion variable declaration (superseded by install.py + +# .j2 rendering for anything new - see "Repo-specific rendering" below). +# Only relevant if a shipped artifact still contains a literal {{REPO_NAME}} +# token and --no-expand is not passed. +variables: + REPO_NAME: + auto: git-repo-basename +``` + +Codex installs (`--codex`) only ever receive `skills`, `scripts`, and +`assets` - Codex has no equivalent of commands, agents, or +`agents/registry.yaml`. + +### Registry (`agents/registry.yaml`) + +For non-Codex targets, every installed file under `agents/` or `skills/` is +recorded in `/agents/registry.yaml` after the copy step. An +`agents/*.md` file's frontmatter `version` (if present) must match the +package's `manifest.yaml` version, or the install fails; this catches an +artifact that was hand-edited without bumping its own version. + +## Repo-specific rendering: `.local.j2` and `install.py` + +A package that needs to bake repo-specific values (like the consuming repo's +name) into an installed artifact has two options: + +1. **`.local.j2` sibling files** (existing mechanism, any artifact category): + ship `.local.j2` next to `` and it is rendered via + `sc-compose` and used *instead of* the plain `` for any install that + isn't `--global`/`--user` and isn't `--codex`. `sc-compose` is + auto-installed on first use. This is a generic, no-code mechanism - good + for simple single-variable substitution. +2. **`install.py` hooks** (below): full control over what happens before and + after the copy step, for anything `.local.j2` doesn't cover (multiple + templates, conditional logic, non-trivial failure messages, cleanup). + +Both mechanisms may be used by the same package; they don't interact with +each other. + +## `install.py` hooks + +A package root may optionally contain `packages//install.py` defining +any of three module-level functions. None are required; a package with no +`install.py`, or one missing some of the functions, behaves as if the absent +ones were no-ops. + +```python +def prepare(source_path: str, destination_path: str, options: dict) -> dict: + """Runs once per install target, immediately before the artifact copy + step for that target. Must not write to destination_path - nothing has + been copied there yet for this install run.""" + +def complete(source_path: str, destination_path: str, options: dict) -> dict: + """Runs once per install target, immediately after the artifact copy + step and registry update finish. This is where repo-specific rendering, + and any conditional delete-if-present cleanup of prior-version output + (see INVENTORY below), belongs.""" + +def cleanup(source_path: str, destination_path: str, options: dict) -> dict: + """Runs once per target during `sc-install uninstall`, after the + standard manifest-artifact removal. Deletes anything prepare()/ + complete() created that isn't itself a plain manifest artifact (so the + standard removal step wouldn't already have deleted it).""" +``` + +- `source_path` is the package's root directory (`packages/`). +- `destination_path` is the resolved directory for *this* install target + (e.g. `~/.claude`, `./.codex`, or an explicit `--dest`) - the same + directory every prior install/uninstall of this package used, so a hook + can inspect what's already on disk there. +- `options` is always: + ```python + { + "global": bool, "local": bool, "user": bool, "project": bool, + "codex": bool, # True iff this call is for the .codex target + "force": bool, "expand": bool, + "args": {"KEY": "VALUE", ...}, # from repeatable --set KEY=VALUE + } + ``` + `args` is empty unless the caller passed `--set`. A package that needs + information `sc-install` has no generic way to know (e.g. a target + environment name) reads it from `options["args"]` and fails with a message + telling the caller which `--set` to add if it's missing - see the result + contract below. +- Both `.claude` and `.codex` targets get their own full `prepare()` → copy → + `complete()` cycle when both are installed (`options["codex"]` tells the + hook which one it's in). + +### Result contract + +Every hook function must return one of exactly two shapes: + +```python +{"result": "success"} +{"result": "fail", "message": ""} +``` + +`message` must combine *why it failed* and *what to do about it* in one +string - there is no separate reason/suggestion field. Anything else - +raising an exception, returning a non-dict, returning a dict without a +`"result"` key, or reporting `"fail"` without a `message` - is itself an +`sc-install` error (the hook is buggy, not the caller's input), and +`sc-install` reports a descriptive error and aborts that target with a +non-zero exit code. A failing hook stops the install/uninstall for that +target only; other targets already processed are not rolled back. + +### Requirements for `install.py` authors + +These are enforced by convention and (for the INVENTORY rule) by CI, not by +`sc-install` itself - a non-compliant hook can still be written, but every +hook shipped in this repo is expected to meet these: + +1. **Idempotent.** Running `prepare()`/`complete()`/`cleanup()` twice against + the same `destination_path` (re-running install, e.g. with `--force`; + running uninstall twice) must produce the same on-disk result as running + it once - never accumulate duplicate or corrupted state. +2. **Self-cleaning across versions, via a cumulative `INVENTORY`.** A + package with an `install.py` should keep a module-level `INVENTORY` list + naming every destination-relative path it has ever produced across all + released versions. Entries are only ever *added*, never removed, even + once a path stops being current. A CI validator checks each `INVENTORY` + entry against the package's current source tree: an entry whose source + file no longer exists (this version dropped it) fails CI unless + `complete()` contains a matching conditional delete for that path. + `prepare()` must never contain such a delete (see rule 1: nothing should + touch `destination_path` before the copy step). Put a comment directly + above `INVENTORY` telling the next editor exactly this, so removing an + entry without adding the corresponding cleanup line is caught early + rather than at CI: + + ```python + # Every path this package has ever installed, across all versions. + # Only append - never remove an entry. If a version stops shipping a + # path listed here, add a matching `if (Path(destination_path) / path + # ).exists(): ...unlink()` line to complete() before removing that + # artifact from manifest.yaml, or CI will fail. + INVENTORY = [ + "commands/sc-example.md", + ] + ``` + +### Example: `packages/sc-git-worktree/*.local.j2` + +`sc-git-worktree` is the package this generic mechanism was built for: its +manifest docs describe a `worktree_base` default of `../-worktrees`, +and that repo name should be baked in for a local install rather than left as +a runtime-derivation instruction. It ships three `.local.j2` siblings - +`commands/sc-git-worktree.md.local.j2`, `skills/sc-git-worktree/SKILL.md.local.j2`, +and `agents/sc-git-worktree-update.md.local.j2` - each identical to its plain +`.md` counterpart except that the `basename $(git rev-parse --show-toplevel)` +runtime-derivation sentence is replaced with the literal `{{ REPO_NAME }}` +value. For `--local`/`--project` installs where `repo_name` is non-empty, +`install_one()` renders the `.local.j2` and writes that instead of the plain +file; for `--global`/`--user` installs, `--codex` targets, or any install +where no repo is detected, the plain `.md` (with the runtime-derivation +instructions) is copied unchanged. `sc-git-worktree` has no `install.py` - +this substitution needs nothing beyond the generic mechanism. + +## Uninstall + +`sc-install uninstall --dest ` removes every path in the +package's current `manifest.yaml` artifacts from ``, then runs the +package's `cleanup()` hook (if any) with the same `options` shape as +install (scope flags are all `False` since `uninstall` doesn't reconstruct +them; `args` still comes from `--set`). diff --git a/src/sc_cli/install.py b/src/sc_cli/install.py index b95d28a07..672282b2f 100644 --- a/src/sc_cli/install.py +++ b/src/sc_cli/install.py @@ -5,11 +5,11 @@ Commands: list info - install --dest [--force] [--no-expand] - install --global [--force] [--no-expand] - install --local [--force] [--no-expand] - install --user [--force] [--no-expand] - install --project [--force] [--no-expand] + install --dest [--force] [--no-expand] [--set K=V ...] + install --global [--claude|--codex] [--force] [--no-expand] [--set K=V ...] + install --local [--claude|--codex] [--force] [--no-expand] [--set K=V ...] + install --user [--claude|--codex] [--force] [--no-expand] [--set K=V ...] + install --project [--claude|--codex] [--force] [--no-expand] [--set K=V ...] uninstall --dest registry add [--path ] registry list @@ -33,6 +33,15 @@ - Uses YAML if PyYAML is installed; otherwise falls back to a simple line parser compatible with the existing manifest patterns. - Token expansion: replaces {{REPO_NAME}} when variables.REPO_NAME.auto == git-repo-basename +- Local-install templating: any artifact with a sibling .local.j2 file (any + category: commands, skills, agents, scripts, assets, plugin) is rendered via + sc-compose (auto-installed on demand) instead of copied verbatim, for any + install that isn't --global/--user, and never for the .codex target (Codex + has no install-time repo-specific customization). +- --claude/--codex select which target(s) get installed under the chosen + scope (--global/--local/--user/--project): symmetric flags, either alone + means only that target, neither means both .claude and .codex are + installed. An explicit --dest is always .claude-only. - Scripts are made executable on install (artifacts under scripts/*) - Config file manages marketplace registries with metadata (url, path, status, added_date) - Phase 1: Basic registry commands (add, list, remove) and config persistence @@ -862,6 +871,149 @@ def cmd_info(pkg: str, registry: Optional[str] = None) -> int: return 0 +def _get_render_template(): + """Return sc_compose.render_template, auto-installing sc-compose if missing. + + Only called when a package actually ships a `.local.j2` artifact, so the + dependency is pulled in lazily/scoped rather than being a hard requirement + of sc-install itself. + """ + try: + from sc_compose import render_template # type: ignore + return render_template + except ImportError: + pass + try: + subprocess.run([sys.executable, "-m", "pip", "install", "sc-compose"], check=True) + except subprocess.CalledProcessError as ex: + raise RuntimeError(f"failed to install required dependency 'sc-compose': {ex}") from ex + try: + from sc_compose import render_template # type: ignore + return render_template + except ImportError as ex: + raise RuntimeError("sc-compose installed but 'render_template' is not importable") from ex + + +def _parse_hook_args(raw: List[str]) -> Dict[str, str]: + """Parse repeatable --set KEY=VALUE into a dict for options['args']. + + A malformed entry (no '=') is a caller error, so it's reported like any + other bad argument rather than silently dropped. + """ + parsed: Dict[str, str] = {} + for item in raw: + if "=" not in item: + raise SystemExit(f"--set expects KEY=VALUE, got: {item!r}") + key, value = item.split("=", 1) + key = key.strip() + if not key: + raise SystemExit(f"--set expects KEY=VALUE, got: {item!r}") + parsed[key] = value + return parsed + + +def _load_install_hook(pkg_dir: Path): + """Load a package's optional install.py hook, or None if it ships none. + + A package root may contain an install.py defining prepare(source_path, + destination_path, options), complete(source_path, destination_path, + options), and/or cleanup(source_path, destination_path, options). + prepare() runs right before the artifact copy step, complete() right + after it finishes (including the registry update) - both once per + install target. cleanup() runs once per target during `sc-install + uninstall`, after the standard manifest-artifact removal. Any of the + three may do target-specific work sc-install's generic copy/.local.j2 + mechanism doesn't cover, e.g. shelling out to `sc-compose render` with + values only the package knows, or deleting hook-created files that + aren't manifest artifacts sc-install would otherwise know to remove. + + Requirements for install.py authors (enforced by convention, not by + sc-install - a non-compliant hook can still be written, but every hook + that ships in this repo is expected to meet these): + + - Idempotent: running prepare()/complete()/cleanup() twice against the + same destination_path (same or re-run install, e.g. after --force; + uninstall run twice) must produce the same on-disk result as running + it once, not accumulate duplicate or corrupted state. + - Self-cleaning across versions, via a cumulative INVENTORY: a package + that ships an install.py should keep a module-level `INVENTORY` list + in install.py naming every relative artifact/output path it has ever + produced, across all released versions (entries are only ever added, + never removed, even once a path stops being current). A validator + checks each INVENTORY entry against the package's current source + tree; an entry whose source file no longer exists (i.e. this version + dropped it) fails CI unless complete() contains a matching + "delete-if-present" line for that destination-relative path (prepare() + runs before the copy step and must not touch the destination at all - + the conditional delete belongs in complete(), alongside the rest of + the post-copy cleanup work) - the + docstring/comment right above INVENTORY should say as much, so a + designer (human or agent) editing the list is pointed straight at + what to add. destination_path itself already holds the prior + release's files (each install runs against the same on-disk + location the last one used), so no separate "previous version" data + needs to be passed in - the check is source-tree-vs-INVENTORY, not + destination-vs-something. + """ + hook_path = pkg_dir / "install.py" + if not hook_path.is_file(): + return None + import importlib.util + + spec = importlib.util.spec_from_file_location(f"sc_install_hook_{pkg_dir.name}", hook_path) + if spec is None or spec.loader is None: + raise RuntimeError(f"Cannot load install hook: {hook_path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _run_install_hook( + hook, + fn_name: str, + source_path: Path, + destination_path: Path, + options: Dict[str, Any], +) -> Optional[str]: + """Run an optional prepare()/complete() hook. + + Returns None if the hook is absent or reports success. On failure, + returns a single agent-facing error string. The hook's return value must + be {"result": "success"} or {"result": "fail", "message": ""} - a hook that raises, or that fails + without a message carrying both the reason and the fix, is itself an + sc-install error, since callers need a reason and a fix, not a stack + trace. + """ + fn = getattr(hook, fn_name, None) if hook is not None else None + if fn is None: + return None + try: + result = fn(str(source_path), str(destination_path), dict(options)) + except Exception as ex: + return ( + f"install.py {fn_name}() raised {type(ex).__name__}: {ex} " + "-- suggested fix: install.py hooks must catch their own errors and " + "return {'result': 'fail', 'message': ''}" + ) + if not isinstance(result, dict) or "result" not in result: + return ( + f"install.py {fn_name}() returned a malformed result " + f"(expected {{'result': 'success'|'fail', ...}}): {result!r} " + "-- suggested fix: return the standard result shape" + ) + if result["result"] == "success": + return None + message = result.get("message") + if not message: + return ( + f"install.py {fn_name}() failed without a message " + f"(got {result!r}) -- suggested fix: every failure must set " + "'message' to the fail reason and instructions to fix it" + ) + return f"install.py {fn_name}() failed: {message}" + + def _git_repo_basename(dest_dir: Path) -> str: try: # Determine toplevel from parent of dest (.claude lives under repo) @@ -881,8 +1033,15 @@ def _git_repo_basename(dest_dir: Path) -> str: return "" -def _iter_artifacts(m: Manifest) -> Iterable[str]: - order = ["commands", "skills", "agents", "scripts", "assets", "plugin"] +def _iter_artifacts(m: Manifest, *, codex: bool = False) -> Iterable[str]: + # Codex has no equivalent of Claude Code's slash-commands, subagents, or + # plugin manifests, and no registry.yaml, so only skills/scripts/assets + # are meaningful there. + order = ( + ["skills", "scripts", "assets"] + if codex + else ["commands", "skills", "agents", "scripts", "assets", "plugin"] + ) for key in order: for item in m.artifacts.get(key, []): yield item @@ -952,21 +1111,26 @@ def _parse_frontmatter_simple(md_path: Path) -> Dict[str, Any]: return out -def _resolve_install_dest( +def _resolve_install_scope( global_flag: bool = False, local_flag: bool = False, user_flag: bool = False, project_flag: bool = False, dest: Optional[str] = None, ) -> Optional[Path]: - """Resolve installation destination from --global/--local/--user/--project/--dest flags. + """Resolve the install *scope* (global/user/local/project/dest) to a base path. + + Phase 1: Support for --global, --local, --user, and --project flags. - Phase 1: Support for --global, --local, --user, and --project flags + For --global/--user/--local/--project this is the *parent* directory that + a `.claude` and/or `.codex` subdirectory is installed into (see + `_resolve_install_targets`). For --dest it is the exact, single directory + to install into (dest is always claude-only; there is no sibling `.codex` + to mirror into, since the path was explicitly chosen by the caller). Returns: - Path to .claude directory, or None if invalid combination + Path, or None if invalid combination (an error has already been reported). """ - # Count how many flags are set flags_set = sum([global_flag, local_flag, user_flag, project_flag, dest is not None]) if flags_set == 0: @@ -978,10 +1142,10 @@ def _resolve_install_dest( return None if global_flag or user_flag: - return Path.home() / ".claude" + return Path.home() if local_flag or project_flag: - return Path.cwd() / ".claude" + return Path.cwd() # dest flag if dest: @@ -993,6 +1157,25 @@ def _resolve_install_dest( return None +def _resolve_install_targets( + claude_flag: bool, codex_flag: bool, *, dest: Optional[str] = None +) -> List[str]: + """Resolve which of "claude"/"codex" to install, mirroring --global/--local + symmetry: --claude or --codex alone installs only that target; with + neither given, both install. An explicit --dest is always claude-only, + since it names one exact directory rather than a `.claude`/`.codex`-suffixed + base. + """ + if dest: + return ["claude"] + targets = [] + if claude_flag: + targets.append("claude") + if codex_flag: + targets.append("codex") + return targets or ["claude", "codex"] + + def _parse_skill_metadata(skill_md_path: Path) -> Dict[str, Any]: """Parse skill metadata from SKILL.md frontmatter. @@ -1159,10 +1342,13 @@ def cmd_install( local_flag: bool = False, user_flag: bool = False, project_flag: bool = False, + claude_flag: bool = False, + codex_flag: bool = False, registry: Optional[str] = None, + hook_args: Optional[Dict[str, str]] = None, ) -> int: - """Install a package to a .claude directory. - + """Install a package to a .claude and/or .codex directory. + Phase 3 Enhancement: Remote Registry Support - Support --registry flag to install from remote registry - Prefer local packages (backward compatible) @@ -1170,14 +1356,24 @@ def cmd_install( Args: pkg: Package name - dest: Explicit destination path + dest: Explicit destination path (always .claude-only; no .codex mirror) force: Overwrite existing files expand: Perform token expansion - global_flag: Install to ~/.claude - local_flag: Install to ./.claude + global_flag: Install under ~/ (i.e. ~/.claude and/or ~/.codex) + local_flag: Install under ./ (i.e. ./.claude and/or ./.codex) user_flag: Alias for --global project_flag: Alias for --local + claude_flag: Install the .claude target (default: both, if neither + --claude nor --codex is given) + codex_flag: Install the .codex target (skills/scripts/assets only; + no commands, agents, or registry.yaml) registry: Optional registry name to install from + hook_args: Extra key=value pairs (from repeatable --set) passed through + to the package's install.py hooks as options["args"]. Lets a + package require info sc-install has no generic way to know (e.g. + a target environment name); its prepare()/complete() can fail + with a message telling the caller which --set to add, and the + agent driving sc-install retries with it supplied. """ # Check local package first (backward compatible) pkg_dir = PACKAGES_DIR / pkg @@ -1219,71 +1415,141 @@ def cmd_install( error(f"Package not found: {pkg}") return 1 - # Resolve destination (Phase 1: Support --global/--local/--user/--project) - dest_path = _resolve_install_dest(global_flag, local_flag, user_flag, project_flag, dest) - if dest_path is None: + if dest and codex_flag: + error("--dest does not support --codex (an explicit --dest path is always .claude-only)") return 1 - # track installed artifact files relative to dest_path - installed_artifacts: List[str] = [] - dest_path.mkdir(parents=True, exist_ok=True) + # Resolve scope (Phase 1: --global/--local/--user/--project/--dest) and + # targets (--claude/--codex, symmetric: either alone means only that one, + # neither means both). + base_path = _resolve_install_scope(global_flag, local_flag, user_flag, project_flag, dest) + if base_path is None: + return 1 + targets = _resolve_install_targets(claude_flag, codex_flag, dest=dest) manifest = _parse_manifest(pkg_dir) - repo_name = "" - if expand and manifest.variables.get("REPO_NAME", {}).get("auto") == "git-repo-basename": - repo_name = _git_repo_basename(dest_path) - - info(f"Installing {pkg} to {dest_path}") - if repo_name: - info(f"REPO_NAME={repo_name}") - - def install_one(rel_file: str) -> None: - src = (pkg_dir / rel_file).resolve() - dst = (dest_path / rel_file).resolve() - if not src.exists(): - warn(f"Source not found: {src}") - return - if dst.exists() and not force: - warn(f"Skip (exists): {dst}") - return - dst.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(src, dst) - # executable for scripts/* - if rel_file.startswith("scripts/"): - _ensure_executable(dst) - # track agents and skills for registry - if rel_file.startswith("agents/") or rel_file.startswith("skills/"): - # store relative to .claude (dest_path) - installed_artifacts.append(rel_file) - # token expansion - if expand and repo_name: - try: - text = dst.read_text(encoding="utf-8", errors="ignore") - text = text.replace("{{REPO_NAME}}", repo_name) - dst.write_text(text, encoding="utf-8") - except Exception: - # Ignore binary/non-text failures - pass - info(f"Installed: {rel_file}") + # Legacy generic {{TOKEN}} naive-replace mechanism (still supported, unrelated + # to .local.j2): only active when the manifest explicitly declares it. + declares_repo_name_var = manifest.variables.get("REPO_NAME", {}).get("auto") == "git-repo-basename" + + # .local.j2 templating (sc-compose): available for any artifact, in any + # category, for any install that isn't --global/--user, and never for + # codex (codex has no install-time repo-specific customization). + allow_local_templates = not (global_flag or user_flag) + + # Optional package-supplied install.py: prepare() runs right before the + # artifact copy step, complete() right after it (registry update + # included), once per install target. Lets a package do target-specific + # work sc-install's generic copy/.local.j2 mechanism doesn't cover. + install_hook = _load_install_hook(pkg_dir) + + def install_target(dest_path: Path, is_codex: bool) -> int: + dest_path.mkdir(parents=True, exist_ok=True) + installed_artifacts: List[str] = [] + use_local_templates = allow_local_templates and not is_codex + + repo_name = "" + if use_local_templates or (expand and declares_repo_name_var): + repo_name = _git_repo_basename(dest_path) + + hook_options = { + "global": global_flag, + "local": local_flag, + "user": user_flag, + "project": project_flag, + "codex": is_codex, + "force": force, + "expand": expand, + "args": dict(hook_args or {}), + } - for rel in _iter_artifacts(manifest): - install_one(rel) + hook_error = _run_install_hook(install_hook, "prepare", pkg_dir, dest_path, hook_options) + if hook_error: + error(hook_error) + return 1 - # Update registry.yaml (agents and skills) - rc = _update_registry( - dest_path, - installed_artifacts, - package_version=manifest.version or None, - ) - if rc != 0: - return rc + info(f"Installing {pkg} to {dest_path}") + if repo_name: + info(f"REPO_NAME={repo_name}") + + def install_one(rel_file: str) -> bool: + local_template = pkg_dir / f"{rel_file}.local.j2" + use_template = use_local_templates and bool(repo_name) and local_template.exists() + src = (local_template if use_template else (pkg_dir / rel_file)).resolve() + dst = (dest_path / rel_file).resolve() + if not src.exists(): + warn(f"Source not found: {src}") + return True + if dst.exists() and not force: + warn(f"Skip (exists): {dst}") + return True + dst.parent.mkdir(parents=True, exist_ok=True) + + if use_template: + try: + render_template = _get_render_template() + except RuntimeError as ex: + error(f"Cannot install {rel_file}: {ex}") + return False + text = local_template.read_text(encoding="utf-8", errors="ignore") + rendered = render_template(text, {"REPO_NAME": repo_name}) + dst.write_text(rendered, encoding="utf-8") + else: + shutil.copy2(src, dst) + # legacy token expansion + if expand and declares_repo_name_var and repo_name: + try: + text = dst.read_text(encoding="utf-8", errors="ignore") + text = text.replace("{{REPO_NAME}}", repo_name) + dst.write_text(text, encoding="utf-8") + except Exception: + # Ignore binary/non-text failures + pass + + # executable for scripts/* + if rel_file.startswith("scripts/"): + _ensure_executable(dst) + # track agents and skills for registry + if rel_file.startswith("agents/") or rel_file.startswith("skills/"): + # store relative to .claude (dest_path) + installed_artifacts.append(rel_file) + info(f"Installed: {rel_file}") + return True + + for rel in _iter_artifacts(manifest, codex=is_codex): + if not install_one(rel): + return 1 + + # Codex has no registry.yaml concept (no agents/subagent roster to track) + if not is_codex: + rc = _update_registry( + dest_path, + installed_artifacts, + package_version=manifest.version or None, + ) + if rc != 0: + return rc + + hook_error = _run_install_hook(install_hook, "complete", pkg_dir, dest_path, hook_options) + if hook_error: + error(hook_error) + return 1 + + info(f"Done installing {pkg} to {dest_path}") + return 0 + + for target in targets: + is_codex = target == "codex" + target_dest = base_path if dest else (base_path / f".{target}") + rc = install_target(target_dest, is_codex) + if rc != 0: + return rc - info(f"Done installing {pkg}") return 0 -def cmd_uninstall(pkg: str, dest: str) -> int: +def cmd_uninstall(pkg: str, dest: str, *, hook_args: Optional[Dict[str, str]] = None) -> int: pkg_dir = PACKAGES_DIR / pkg if not pkg_dir.is_dir(): error(f"Package not found: {pkg}") @@ -1303,6 +1569,26 @@ def cmd_uninstall(pkg: str, dest: str) -> int: info(f"Removed: {rel}") except Exception: warn(f"Could not remove: {rel}") + + # Optional install.py cleanup(): removes hook-created files (e.g. + # rendered .local.j2/.j2 output) that aren't manifest artifacts, so + # sc-install's own artifact-list loop above wouldn't know to remove them. + install_hook = _load_install_hook(pkg_dir) + hook_options: Dict[str, Any] = { + "global": False, + "local": False, + "user": False, + "project": False, + "codex": False, + "force": False, + "expand": False, + "args": dict(hook_args or {}), + } + hook_error = _run_install_hook(install_hook, "cleanup", pkg_dir, dest_path, hook_options) + if hook_error: + error(hook_error) + return 1 + info(f"Done uninstalling {pkg}") return 0 @@ -1337,13 +1623,35 @@ def build_parser() -> argparse.ArgumentParser: dest_group.add_argument("--user", dest="user_flag", action="store_true") dest_group.add_argument("--local", dest="local_flag", action="store_true") dest_group.add_argument("--project", dest="project_flag", action="store_true") + # Target flags: independent of scope, symmetric with each other. Neither + # given means install both; either alone means only that one. + p_install.add_argument("--claude", dest="claude_flag", action="store_true") + p_install.add_argument("--codex", dest="codex_flag", action="store_true") p_install.add_argument("--force", action="store_true") p_install.add_argument("--no-expand", action="store_true") p_install.add_argument("--registry", help="Install from remote registry") + p_install.add_argument( + "--set", + dest="hook_args", + action="append", + default=[], + metavar="KEY=VALUE", + help="Extra key=value passed to the package's install.py hooks " + "(options['args']); repeatable. Only a package's own hook interprets these.", + ) p_uninstall = sub.add_parser("uninstall") p_uninstall.add_argument("package") p_uninstall.add_argument("--dest", required=True) + p_uninstall.add_argument( + "--set", + dest="hook_args", + action="append", + default=[], + metavar="KEY=VALUE", + help="Extra key=value passed to the package's install.py cleanup() hook " + "(options['args']); repeatable.", + ) # Phase 1: Registry commands p_registry = sub.add_parser("registry") @@ -1392,11 +1700,18 @@ def main(argv: Optional[list[str]] = None) -> int: local_flag=getattr(args, 'local_flag', False), user_flag=getattr(args, 'user_flag', False), project_flag=getattr(args, 'project_flag', False), + claude_flag=getattr(args, 'claude_flag', False), + codex_flag=getattr(args, 'codex_flag', False), registry=getattr(args, 'registry', None), + hook_args=_parse_hook_args(getattr(args, 'hook_args', []) or []), ) if args.cmd == "uninstall": - return cmd_uninstall(args.package, args.dest) + return cmd_uninstall( + args.package, + args.dest, + hook_args=_parse_hook_args(getattr(args, 'hook_args', []) or []), + ) if args.cmd == "registry": if args.registry_cmd == "add": diff --git a/tests/scripts/test_validate_manifest_artifacts.py b/tests/scripts/test_validate_manifest_artifacts.py index 3e22f4b69..6d4dea9dc 100644 --- a/tests/scripts/test_validate_manifest_artifacts.py +++ b/tests/scripts/test_validate_manifest_artifacts.py @@ -327,6 +327,20 @@ def test_get_disk_files_ignores_directories(temp_dir): assert "commands/subdir" not in files +def test_get_disk_files_excludes_local_j2_siblings(temp_dir): + """Test that .local.j2 sibling files are excluded from disk files.""" + package_dir = temp_dir / "local-j2-package" + package_dir.mkdir() + + (package_dir / "commands").mkdir() + (package_dir / "commands" / "cmd.md").write_text("# Command") + (package_dir / "commands" / "cmd.md.local.j2").write_text("# Command {{ REPO_NAME }}") + + files = get_disk_files(package_dir) + assert files == ["commands/cmd.md"] + assert "commands/cmd.md.local.j2" not in files + + # ============================================================================ # validate_script_file Tests # ============================================================================ diff --git a/tests/test_sc_cli_scope_aliases.py b/tests/test_sc_cli_scope_aliases.py index 34c10665f..ce4ab1199 100644 --- a/tests/test_sc_cli_scope_aliases.py +++ b/tests/test_sc_cli_scope_aliases.py @@ -4,14 +4,14 @@ from sc_cli import skill_integration -def test_resolve_install_dest_supports_user_and_project(tmp_path, monkeypatch): +def test_resolve_install_scope_supports_user_and_project(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) monkeypatch.setattr(sc_install.Path, "home", lambda: tmp_path / "home") - assert sc_install._resolve_install_dest(local_flag=True) == tmp_path / ".claude" - assert sc_install._resolve_install_dest(project_flag=True) == tmp_path / ".claude" - assert sc_install._resolve_install_dest(global_flag=True) == tmp_path / "home" / ".claude" - assert sc_install._resolve_install_dest(user_flag=True) == tmp_path / "home" / ".claude" + assert sc_install._resolve_install_scope(local_flag=True) == tmp_path + assert sc_install._resolve_install_scope(project_flag=True) == tmp_path + assert sc_install._resolve_install_scope(global_flag=True) == tmp_path / "home" + assert sc_install._resolve_install_scope(user_flag=True) == tmp_path / "home" def test_install_marketplace_scope_aliases(tmp_path, monkeypatch): diff --git a/tests/test_sc_install.py b/tests/test_sc_install.py index 401845519..0c7fea2c2 100644 --- a/tests/test_sc_install.py +++ b/tests/test_sc_install.py @@ -5,6 +5,7 @@ from pathlib import Path from sc_cli import sc_install +from sc_cli import install as _install_mod def _init_git_repo(path: Path) -> None: subprocess.run(["git", "init", "-q", str(path)], check=True) @@ -46,14 +47,36 @@ def test_install_and_uninstall_delay_tasks(tmp_path: Path): assert not (dest / "agents/sc-delay-once.md").exists() -def test_token_expansion_repo_name(tmp_path: Path): +def test_token_expansion_repo_name(tmp_path: Path, monkeypatch): + """Test the {{REPO_NAME}} token-expansion mechanism in isolation via a + synthetic package, since no shipped package consumes it anymore (see + GitHub issue #112).""" + pkg_root = tmp_path / "pkgs" + pkg_dir = pkg_root / "fake-repo-name-pkg" + (pkg_dir / "commands").mkdir(parents=True) + (pkg_dir / "manifest.yaml").write_text( + "name: fake-repo-name-pkg\n" + "version: 0.1.0\n" + "variables:\n" + " REPO_NAME:\n" + " auto: git-repo-basename\n" + "artifacts:\n" + " commands:\n" + " - commands/cmd.md\n", + encoding="utf-8", + ) + (pkg_dir / "commands" / "cmd.md").write_text( + "base: ../{{REPO_NAME}}-worktrees\n", encoding="utf-8" + ) + monkeypatch.setattr(_install_mod, "PACKAGES_DIR", pkg_root) + repo = tmp_path / "myrepo" repo.mkdir() _init_git_repo(repo) dest = repo / ".claude" - rc = sc_install.main(["install", "sc-git-worktree", "--dest", str(dest)]) + rc = sc_install.main(["install", "fake-repo-name-pkg", "--dest", str(dest)]) assert rc == 0 - f = dest / "commands/sc-git-worktree.md" + f = dest / "commands/cmd.md" assert f.exists() content = f.read_text(encoding="utf-8") assert f"../{repo.name}-worktrees" in content diff --git a/tests/test_sc_install_phase1_2.py b/tests/test_sc_install_phase1_2.py index 9b8e522f1..43420a445 100644 --- a/tests/test_sc_install_phase1_2.py +++ b/tests/test_sc_install_phase1_2.py @@ -82,6 +82,65 @@ def git_repo(tmp_path): return repo_dir +@pytest.fixture +def repo_name_token_pkg(tmp_path, monkeypatch): + """Synthetic package with a manifest.yaml declaring the REPO_NAME auto variable. + + Used to test the {{REPO_NAME}} token-expansion mechanism in isolation, since + no shipped package consumes it anymore (see GitHub issue #112). + """ + pkg_root = tmp_path / "pkgs" + pkg_dir = pkg_root / "fake-repo-name-pkg" + (pkg_dir / "commands").mkdir(parents=True) + (pkg_dir / "manifest.yaml").write_text( + "name: fake-repo-name-pkg\n" + "version: 0.1.0\n" + "variables:\n" + " REPO_NAME:\n" + " auto: git-repo-basename\n" + "artifacts:\n" + " commands:\n" + " - commands/cmd.md\n", + encoding="utf-8", + ) + (pkg_dir / "commands" / "cmd.md").write_text( + "base: ../{{REPO_NAME}}-worktrees\n", encoding="utf-8" + ) + monkeypatch.setattr(sc_install, "PACKAGES_DIR", pkg_root) + return "fake-repo-name-pkg" + + +@pytest.fixture +def local_template_pkg(tmp_path, monkeypatch): + """Synthetic package shipping a `.local.j2` sibling under `scripts/`. + + Deliberately not under commands/ or agents/, to prove `.local.j2` detection + is generic across every artifact category. Also used to prove --codex + skips .local.j2 templating exactly like --global. + """ + pkg_root = tmp_path / "pkgs" + pkg_dir = pkg_root / "fake-local-template-pkg" + (pkg_dir / "scripts").mkdir(parents=True) + (pkg_dir / "manifest.yaml").write_text( + "name: fake-local-template-pkg\n" + "version: 0.1.0\n" + "artifacts:\n" + " scripts:\n" + " - scripts/run.sh\n", + encoding="utf-8", + ) + # Plain shipped fallback (used for global/user installs). + (pkg_dir / "scripts" / "run.sh").write_text( + "#!/bin/sh\necho generic\n", encoding="utf-8" + ) + # Local-install-only template (sc-compose Jinja-style syntax). + (pkg_dir / "scripts" / "run.sh.local.j2").write_text( + "#!/bin/sh\necho {{ REPO_NAME }}\n", encoding="utf-8" + ) + monkeypatch.setattr(sc_install, "PACKAGES_DIR", pkg_root) + return "fake-local-template-pkg" + + # ============================================================================== # TEST GROUP 1: Global and Local Flags (12 tests) # ============================================================================== @@ -197,6 +256,77 @@ def test_install_multiple_flags_error(self, temp_home, capsys): assert "mutually exclusive" in err.lower() or "not allowed" in err.lower() +class TestCodexFlag: + """Test --codex installation target (skills/scripts/assets only, no + commands/agents/registry.yaml). --claude/--codex are symmetric target + flags under a scope (--global/--local/--user/--project): either alone + installs only that target; neither installs both.""" + + def test_install_codex_flag_creates_dir(self, temp_home, capsys): + """Test that --global --codex creates ~/.codex directory.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global", "--codex"]) + assert rc == 0 + assert (temp_home / ".codex").exists() + assert (temp_home / ".codex" / "scripts").exists() + + def test_install_codex_flag_uses_home_directory(self, temp_home, capsys): + """Test that --global --codex installs to ~/.codex, not ~/.claude.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global", "--codex"]) + assert rc == 0 + assert not (temp_home / ".claude").exists() + + out = capsys.readouterr().out + assert str(temp_home / ".codex") in out + + def test_install_codex_skips_commands_and_agents(self, temp_home, capsys): + """Test that --codex installs only skills/scripts, not commands/agents.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global", "--codex"]) + assert rc == 0 + assert (temp_home / ".codex" / "skills").exists() + assert (temp_home / ".codex" / "scripts").exists() + assert not (temp_home / ".codex" / "commands").exists() + assert not (temp_home / ".codex" / "agents").exists() + + def test_install_codex_skips_registry(self, temp_home, capsys): + """Test that --codex does not write agents/registry.yaml.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global", "--codex"]) + assert rc == 0 + assert not (temp_home / ".codex" / "agents" / "registry.yaml").exists() + + def test_install_no_target_flags_installs_both(self, temp_home, capsys): + """With neither --claude nor --codex given, both targets are installed.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global"]) + assert rc == 0 + assert (temp_home / ".claude" / "agents" / "sc-delay-once.md").exists() + assert (temp_home / ".codex" / "scripts").exists() + assert not (temp_home / ".codex" / "commands").exists() + + def test_install_claude_flag_installs_only_claude(self, temp_home, capsys): + """--claude alone installs only .claude, symmetric with --codex alone.""" + rc = sc_install.main(["install", "sc-delay-tasks", "--global", "--claude"]) + assert rc == 0 + assert (temp_home / ".claude" / "agents" / "sc-delay-once.md").exists() + assert not (temp_home / ".codex").exists() + + def test_install_codex_and_dest_conflict_error(self, temp_home, capsys): + """--dest is always .claude-only; combining it with --codex is an error.""" + dest = temp_home / "custom" / ".claude" + rc = sc_install.main(["install", "sc-delay-tasks", "--codex", "--dest", str(dest)]) + assert rc == 1 + err = capsys.readouterr().err + assert "--codex" in err + + def test_install_codex_skips_local_j2_template(self, temp_home, local_template_pkg): + """--codex must never consult `.local.j2` siblings, exactly like --global.""" + rc = sc_install.main(["install", local_template_pkg, "--global", "--codex"]) + assert rc == 0 + + script = temp_home / ".codex" / "scripts" / "run.sh" + assert script.exists() + content = script.read_text(encoding="utf-8") + assert content == "#!/bin/sh\necho generic\n" + + # ============================================================================== # TEST GROUP 2: Registry Commands (18 tests) # ============================================================================== @@ -665,31 +795,71 @@ def test_install_local_maintains_executable_bit(self, temp_cwd): script = temp_cwd / ".claude" / "scripts" / "sc-delay-run.py" assert os.access(script, os.X_OK) - def test_install_global_expands_repo_name_token(self, temp_home, git_repo): + def test_install_global_expands_repo_name_token(self, temp_home, git_repo, repo_name_token_pkg): """Test that --global expands {{REPO_NAME}} token.""" # Install to git repo dest = git_repo / ".claude" - rc = sc_install.main(["install", "sc-git-worktree", "--dest", str(dest)]) + rc = sc_install.main(["install", repo_name_token_pkg, "--dest", str(dest)]) assert rc == 0 - cmd_file = dest / "commands" / "sc-git-worktree.md" + cmd_file = dest / "commands" / "cmd.md" content = cmd_file.read_text(encoding="utf-8") assert "{{REPO_NAME}}" not in content assert "repo-worktrees" in content - def test_install_local_expands_repo_name_token(self, temp_cwd, git_repo, monkeypatch): + def test_install_local_expands_repo_name_token(self, temp_cwd, git_repo, repo_name_token_pkg, monkeypatch): """Test that --local expands {{REPO_NAME}} token.""" # Change to git repo directory monkeypatch.chdir(git_repo) - rc = sc_install.main(["install", "sc-git-worktree", "--local"]) + rc = sc_install.main(["install", repo_name_token_pkg, "--local"]) assert rc == 0 - cmd_file = git_repo / ".claude" / "commands" / "sc-git-worktree.md" + cmd_file = git_repo / ".claude" / "commands" / "cmd.md" content = cmd_file.read_text(encoding="utf-8") assert "{{REPO_NAME}}" not in content assert "repo-worktrees" in content + def test_install_local_renders_local_j2_template_for_any_category( + self, temp_cwd, git_repo, local_template_pkg, monkeypatch + ): + """`.local.j2` is detected/rendered for a scripts/ artifact, not just commands/agents.""" + monkeypatch.chdir(git_repo) + + rc = sc_install.main(["install", local_template_pkg, "--local"]) + assert rc == 0 + + script = git_repo / ".claude" / "scripts" / "run.sh" + assert script.exists() + content = script.read_text(encoding="utf-8") + assert "{{ REPO_NAME }}" not in content + assert git_repo.name in content + # Rendering must never leak the .local.j2 suffix into the destination. + assert not (git_repo / ".claude" / "scripts" / "run.sh.local.j2").exists() + + def test_install_local_skips_local_j2_template_when_no_repo_found( + self, temp_cwd, local_template_pkg + ): + """--local outside a git repo must fall back to the plain file, not + render `.local.j2` with an empty REPO_NAME.""" + rc = sc_install.main(["install", local_template_pkg, "--local"]) + assert rc == 0 + + script = temp_cwd / ".claude" / "scripts" / "run.sh" + assert script.exists() + content = script.read_text(encoding="utf-8") + assert content == "#!/bin/sh\necho generic\n" + + def test_install_global_skips_local_j2_template(self, temp_home, local_template_pkg): + """--global must never consult `.local.j2` siblings; it installs the plain file.""" + rc = sc_install.main(["install", local_template_pkg, "--global"]) + assert rc == 0 + + script = temp_home / ".claude" / "scripts" / "run.sh" + assert script.exists() + content = script.read_text(encoding="utf-8") + assert content == "#!/bin/sh\necho generic\n" + def test_install_global_updates_agent_registry(self, temp_home): """Test that --global updates agent registry.""" rc = sc_install.main(["install", "sc-delay-tasks", "--global"])