diff --git a/docs/superpowers/specs/2026-03-23-codex-app-compatibility-design.md b/docs/superpowers/specs/2026-03-23-codex-app-compatibility-design.md index c3fecc04cc..fbd5e159f8 100644 --- a/docs/superpowers/specs/2026-03-23-codex-app-compatibility-design.md +++ b/docs/superpowers/specs/2026-03-23-codex-app-compatibility-design.md @@ -237,6 +237,18 @@ If a third skill needs the same detection pattern, extract it into a shared `ref 4. Full lifecycle — detection → commit → finishing detection → correct behavior → cleanup 5. **Sandbox fallback in Local thread** — Start a Codex App **Local thread** (workspace-write sandbox). Prompt: "Use the superpowers skill `using-git-worktrees` to set up an isolated workspace for implementing a small change." Pre-check: `git checkout -b test-sandbox-check` should fail with `Operation not permitted`. Expected: the skill detects `GIT_DIR == GIT_COMMON` (normal repo), attempts `git worktree add -b`, hits Seatbelt denial, falls back to Step 0 "already in workspace" behavior — runs setup, baseline tests, reports ready from current directory. Pass: agent recovers gracefully without cryptic error messages. Fail: agent prints raw Seatbelt error, retries, or gives up with confusing output. +### Manual Test Results (2026-03-23) + +| Test | Result | Notes | +|------|--------|-------| +| 1. Detection in Worktree thread (workspace-write) | PASS | GIT_DIR != GIT_COMMON, BRANCH empty | +| 2. Detection in Worktree thread (Full access) | PASS | Same detection; Full access allows `git checkout -b` | +| 3. Finishing skill handoff format | PASS (after fix) | Initially FAILED — Step 1.5 was unreachable because Step 1 (test verification) halted first. Fixed by reordering: environment detection is now Step 1, test verification is Step 2. Path A skips tests entirely. | +| 4. Full lifecycle | PASS | Detection → commit → handoff payload → no cleanup attempt | +| 5. Sandbox fallback in Local thread | N/A | Local thread workspace-write sandbox does not block `git checkout -b`; Seatbelt restriction not triggered in current Codex App version | + +**Fix applied:** `c5d93ac` — moved environment detection before test verification in `finishing-a-development-branch/SKILL.md`. Step numbering changed: old Step 1.5 → new Step 1, old Step 1 → new Step 2, all downstream steps renumbered. + ### Regression - Existing Claude Code skill-triggering tests still pass diff --git a/skills/executing-plans/SKILL.md b/skills/executing-plans/SKILL.md index e67f94c573..4c88352aee 100644 --- a/skills/executing-plans/SKILL.md +++ b/skills/executing-plans/SKILL.md @@ -65,6 +65,6 @@ After all tasks complete and verified: ## Integration **Required workflow skills:** -- **superpowers:using-git-worktrees** - REQUIRED: Set up isolated workspace before starting +- **superpowers:using-git-worktrees** - REQUIRED: Ensures isolated workspace (creates one or verifies existing) - **superpowers:writing-plans** - Creates the plan this skill executes - **superpowers:finishing-a-development-branch** - Complete development after all tasks diff --git a/skills/finishing-a-development-branch/SKILL.md b/skills/finishing-a-development-branch/SKILL.md index c308b43b4b..b8e3661af9 100644 --- a/skills/finishing-a-development-branch/SKILL.md +++ b/skills/finishing-a-development-branch/SKILL.md @@ -15,7 +15,55 @@ Guide completion of development work by presenting clear options and handling ch ## The Process -### Step 1: Verify Tests +### Step 1: Detect Environment + +Run this FIRST, before test verification: + +````bash +GIT_DIR=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) +GIT_COMMON=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) +BRANCH=$(git branch --show-current) +```` + +**Path A — `GIT_DIR` differs from `GIT_COMMON` AND `BRANCH` is empty (externally managed worktree, detached HEAD):** + +This is a restricted sandbox environment (e.g., Codex App worktree thread). The test toolchain may not be available. Do NOT attempt test verification — skip directly to the handoff. + +First, ensure all work is staged and committed (`git add` + `git commit`). If staging/committing fails due to sandbox restrictions, note the uncommitted files in the handoff message. + +Then present this to the user (do NOT present the 4-option menu): + +``` +Implementation complete. +Current HEAD: + +This workspace is externally managed (detached HEAD). +I cannot create branches, push, or open PRs from here. + +⚠ These commits are on a detached HEAD. If you do not create a branch, +they may be lost when this workspace is cleaned up. + +If your host application provides these controls: +- "Create branch" — to name a branch, then commit/push/PR +- "Hand off to local" — to move changes to your local checkout + +Suggested branch name: +Suggested commit message: +``` + +Branch name: use ticket ID if available (e.g., `pri-823/codex-compat`), otherwise slugify the first 5 words of the plan title, otherwise omit. Avoid sensitive content in branch names. + +Skip to Step 6 (cleanup is a no-op — see guard below). + +**Path B — `GIT_DIR` differs from `GIT_COMMON` AND `BRANCH` exists (externally managed worktree, named branch):** + +Proceed to Step 2 (verify tests, then 4-option menu). + +**Path C — `GIT_DIR` equals `GIT_COMMON` (normal environment):** + +Proceed to Step 2 (verify tests, then 4-option menu). + +### Step 2: Verify Tests **Before presenting options, verify tests pass:** @@ -33,11 +81,11 @@ Tests failing ( failures). Must fix before completing: Cannot proceed with merge/PR until tests pass. ``` -Stop. Don't proceed to Step 2. +Stop. Don't proceed to Step 3. -**If tests pass:** Continue to Step 2. +**If tests pass:** Continue to Step 3. -### Step 2: Determine Base Branch +### Step 3: Determine Base Branch ```bash # Try common base branches @@ -46,7 +94,7 @@ git merge-base HEAD main 2>/dev/null || git merge-base HEAD master 2>/dev/null Or ask: "This branch split from main - is that correct?" -### Step 3: Present Options +### Step 4: Present Options Present exactly these 4 options: @@ -63,7 +111,7 @@ Which option? **Don't add explanation** - keep options concise. -### Step 4: Execute Choice +### Step 5: Execute Choice #### Option 1: Merge Locally @@ -84,7 +132,7 @@ git merge git branch -d ``` -Then: Cleanup worktree (Step 5) +Then: Cleanup worktree (Step 6) #### Option 2: Push and Create PR @@ -103,7 +151,7 @@ EOF )" ``` -Then: Cleanup worktree (Step 5) +Then: Cleanup worktree (Step 6) #### Option 3: Keep As-Is @@ -131,11 +179,20 @@ git checkout git branch -D ``` -Then: Cleanup worktree (Step 5) +Then: Cleanup worktree (Step 6) + +### Step 6: Cleanup Worktree + +**First, check if worktree is externally managed:** + +````bash +GIT_DIR=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) +GIT_COMMON=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) +```` -### Step 5: Cleanup Worktree +If `GIT_DIR` differs from `GIT_COMMON`: skip worktree removal — the host environment owns this workspace. -**For Options 1, 2, 4:** +**Otherwise, for Options 1 and 4:** Check if in worktree: ```bash diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 5150b18632..10a1c3cc61 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -265,7 +265,7 @@ Done! ## Integration **Required workflow skills:** -- **superpowers:using-git-worktrees** - REQUIRED: Set up isolated workspace before starting +- **superpowers:using-git-worktrees** - REQUIRED: Ensures isolated workspace (creates one or verifies existing) - **superpowers:writing-plans** - Creates the plan this skill executes - **superpowers:requesting-code-review** - Code review template for reviewer subagents - **superpowers:finishing-a-development-branch** - Complete development after all tasks diff --git a/skills/using-git-worktrees/SKILL.md b/skills/using-git-worktrees/SKILL.md index e153843cd1..99f2bdebf0 100644 --- a/skills/using-git-worktrees/SKILL.md +++ b/skills/using-git-worktrees/SKILL.md @@ -13,6 +13,30 @@ Git worktrees create isolated workspaces sharing the same repository, allowing w **Announce at start:** "I'm using the using-git-worktrees skill to set up an isolated workspace." +## Step 0: Check if Already in an Isolated Workspace + +Before creating a worktree, check if one already exists: + +````bash +GIT_DIR=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) +GIT_COMMON=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) +BRANCH=$(git branch --show-current) +```` + +**If `GIT_DIR` differs from `GIT_COMMON`:** You are already inside a linked worktree (created by the Codex App, Claude Code's Agent tool, a previous skill run, or the user). Do NOT create another worktree. Instead: + +1. Run project setup (auto-detect package manager as in "Run Project Setup" below) +2. Verify clean baseline (run tests as in "Verify Clean Baseline" below) +3. Report with branch state: + - On a branch: "Already in an isolated workspace at `` on branch ``. Tests passing. Ready to implement." + - Detached HEAD: "Already in an isolated workspace at `` (detached HEAD, externally managed). Tests passing. Note: branch creation needed at finish time. Ready to implement." + +After reporting, STOP. Do not continue to Directory Selection or Creation Steps. + +**If `GIT_DIR` equals `GIT_COMMON`:** Proceed with the full worktree creation flow below. + +**Sandbox fallback:** If you proceed to Creation Steps but `git worktree add -b` fails with a permission error (e.g., "Operation not permitted"), treat this as a late-detected restricted environment. Fall back to the behavior above — run setup and baseline tests in the current directory, report accordingly, and STOP. + ## Directory Selection Process Follow this priority order: @@ -209,9 +233,9 @@ Ready to implement auth feature ## Integration **Called by:** -- **brainstorming** (Phase 4) - REQUIRED when design is approved and implementation follows -- **subagent-driven-development** - REQUIRED before executing any tasks -- **executing-plans** - REQUIRED before executing any tasks +- **brainstorming** - REQUIRED: Ensures isolated workspace (creates one or verifies existing) +- **subagent-driven-development** - REQUIRED: Ensures isolated workspace (creates one or verifies existing) +- **executing-plans** - REQUIRED: Ensures isolated workspace (creates one or verifies existing) - Any skill needing isolated workspace **Pairs with:** diff --git a/tests/codex-app-compat/test-environment-detection.sh b/tests/codex-app-compat/test-environment-detection.sh new file mode 100755 index 0000000000..daee039817 --- /dev/null +++ b/tests/codex-app-compat/test-environment-detection.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Test environment detection logic from PRI-823 +# Tests the git-dir vs git-common-dir comparison used by +# using-git-worktrees Step 0 and finishing-a-development-branch Step 1.5 + +PASS=0 +FAIL=0 +TEMP_DIR=$(mktemp -d) +trap 'rm -rf "$TEMP_DIR"' EXIT + +log_pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +log_fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +# Helper: run detection and return "linked" or "normal" +detect_worktree() { + local git_dir git_common + git_dir=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) + git_common=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) + if [ "$git_dir" != "$git_common" ]; then + echo "linked" + else + echo "normal" + fi +} + +echo "=== Test 1: Normal repo detection ===" +cd "$TEMP_DIR" +git init test-repo > /dev/null 2>&1 +cd test-repo +git commit --allow-empty -m "init" > /dev/null 2>&1 +result=$(detect_worktree) +if [ "$result" = "normal" ]; then + log_pass "Normal repo detected as normal" +else + log_fail "Normal repo detected as '$result' (expected 'normal')" +fi + +echo "=== Test 2: Linked worktree detection ===" +git worktree add "$TEMP_DIR/test-wt" -b test-branch > /dev/null 2>&1 +cd "$TEMP_DIR/test-wt" +result=$(detect_worktree) +if [ "$result" = "linked" ]; then + log_pass "Linked worktree detected as linked" +else + log_fail "Linked worktree detected as '$result' (expected 'linked')" +fi + +echo "=== Test 3: Detached HEAD detection ===" +git checkout --detach HEAD > /dev/null 2>&1 +branch=$(git branch --show-current) +if [ -z "$branch" ]; then + log_pass "Detached HEAD: branch is empty" +else + log_fail "Detached HEAD: branch is '$branch' (expected empty)" +fi + +echo "=== Test 4: Linked worktree + detached HEAD (Codex App simulation) ===" +result=$(detect_worktree) +branch=$(git branch --show-current) +if [ "$result" = "linked" ] && [ -z "$branch" ]; then + log_pass "Codex App simulation: linked + detached HEAD" +else + log_fail "Codex App simulation: result='$result', branch='$branch'" +fi + +echo "=== Test 5: Cleanup guard — linked worktree should NOT remove ===" +cd "$TEMP_DIR/test-wt" +result=$(detect_worktree) +if [ "$result" = "linked" ]; then + log_pass "Cleanup guard: linked worktree correctly detected (would skip removal)" +else + log_fail "Cleanup guard: expected 'linked', got '$result'" +fi + +echo "=== Test 6: Cleanup guard — main repo SHOULD remove ===" +cd "$TEMP_DIR/test-repo" +result=$(detect_worktree) +if [ "$result" = "normal" ]; then + log_pass "Cleanup guard: main repo correctly detected (would proceed with removal)" +else + log_fail "Cleanup guard: expected 'normal', got '$result'" +fi + +# Cleanup worktree before temp dir removal +cd "$TEMP_DIR/test-repo" +git worktree remove "$TEMP_DIR/test-wt" > /dev/null 2>&1 || true + +echo "" +echo "=== Results: $PASS passed, $FAIL failed ===" +if [ "$FAIL" -gt 0 ]; then + exit 1 +fi