fix: restore auto-worktree triggering; remove consent ask (PRI-1147)#1167
Closed
arittr wants to merge 48 commits into
Closed
fix: restore auto-worktree triggering; remove consent ask (PRI-1147)#1167arittr wants to merge 48 commits into
arittr wants to merge 48 commits into
Conversation
Design for making using-git-worktrees, finishing-a-development-branch, and subagent-driven-development skills work in the Codex App's sandboxed worktree environment. Read-only environment detection via git-dir vs git-common-dir comparison, ~48 lines across 4 files, zero breaking changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix three Important issues from spec review: - Clarify Step 1.5 placement relative to existing Steps 2/3 - Re-derive environment state at cleanup time instead of relying on earlier skill output - Acknowledge pre-existing Step 5 cleanup inconsistency Also: precise step references, exact codex-tools.md content, clearer Integration section update instructions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add commit SHA + data loss warning to handoff payload (HIGH) - Add explicit commit step before handoff (HIGH) - Remove misleading "mark as externally managed" from Path B - Add executing-plans 1-line edit (was missing) - Add branch name derivation rules - Add conditional UI language for non-App environments - Add sandbox fallback for permission errors - Add STOP directive after Step 0 reporting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks covering: environment detection in using-git-worktrees, Step 1.5 + cleanup guard in finishing-a-development-branch, Integration line updates, codex-tools.md docs, automated tests, and final verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The subagent review loop (dispatching a fresh agent to review plans/specs) doubled execution time (~25 min overhead) without measurably improving plan quality. Regression testing across 5 versions (v3.6.0 through v5.0.4) with 5 trials each showed identical plan sizes, task counts, and quality scores regardless of whether the review loop ran. Changes: - writing-plans: Replace subagent Plan Review Loop with inline Self-Review checklist (spec coverage, placeholder scan, type consistency) - writing-plans: Add explicit "No Placeholders" section listing plan failures (TBD, vague descriptions, undefined references, "similar to Task N") - brainstorming: Replace subagent Spec Review Loop with inline Spec Self-Review (placeholder scan, internal consistency, scope check, ambiguity check) - Both skills now use "look at it with fresh eyes" framing Testing: 5 trials with the new skill show self-review catches 3-5 real bugs per run (spawn positions, API mismatches, seed bugs, grid indexing) in ~30s instead of ~25 min. Remaining defects are comparable to the subagent approach. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…view" This reverts commit bf8f757.
…eview" This reverts commit b045fa3.
Metadata files (.server-info, .events, .server.pid, .server.log,
.server-stopped) were stored in the same directory served over HTTP,
making them accessible via the /files/ route. They now live in a .meta/
subdirectory that is not web-accessible.
Also fixes a stale test assertion ("Waiting for Claude" → "Waiting for
the agent").
Reported-By: 吉田仁
This reverts commit ab500da.
The session directory now contains two peers: content/ (HTML served to
the browser) and state/ (events, server-info, pid, log). Previously
all files shared a single directory, making server state and user
interaction data accessible over the /files/ HTTP route.
Also fixes stale test assertion ("Waiting for Claude" → "Waiting for
the agent").
Reported-By: 吉田仁
ownerAlive() treated EPERM (permission denied) the same as ESRCH (process not found), causing the server to self-terminate within 60s whenever the owner process ran as a different user. This affected WSL (owner is a Windows process), Tailscale SSH, and any cross-user scenario. The fix: `return e.code === 'EPERM'` — if we get permission denied, the process is alive; we just can't signal it. Tested on Linux via Tailscale SSH with a root-owned grandparent PID: - Server survives past the 60s lifecycle check (EPERM = alive) - Server still shuts down when owner genuinely dies (ESRCH = dead) Fixes #879
Two bugs caused the brainstorm server to self-terminate within 60s: 1. ownerAlive() treated EPERM (permission denied) as "process dead". When the owner PID belongs to a different user (Tailscale SSH, system daemons), process.kill(pid, 0) throws EPERM — but the process IS alive. Fixed: return e.code === 'EPERM'. 2. On WSL, the grandparent PID resolves to a short-lived subprocess that exits before the first 60s lifecycle check. The PID is genuinely dead (ESRCH), so the EPERM fix alone doesn't help. Fixed: validate the owner PID at server startup — if it's already dead, it was a bad resolution, so disable monitoring and rely on the 30-minute idle timeout. This also removes the Windows/MSYS2-specific OWNER_PID="" carve-out from start-server.sh, since the server now handles invalid PIDs generically at startup regardless of platform. Tested on Linux (magic-kingdom) via Tailscale SSH: - Root-owned owner PID (EPERM): server survives ✓ - Dead owner PID at startup (WSL sim): monitoring disabled, survives ✓ - Valid owner that dies: server shuts down within 60s ✓ Fixes #879
…ction Copilot CLI v1.0.11 reads `additionalContext` from sessionStart hook output, but the session-start script only emits the Claude Code-specific nested format. Add COPILOT_CLI env var detection so Copilot CLI gets the SDK-standard top-level `additionalContext` while Claude Code continues getting `hookSpecificOutput`. Based on PR #910 by @culinablaz.
- Add references/copilot-tools.md with full tool equivalence table - Add Copilot CLI to using-superpowers skill platform instructions - Add marketplace install instructions to README - Add changelog entry crediting @culinablaz for the hook fix
The bootstrap text advertised a configDir-based skills path that didn't match the runtime path (resolved relative to the plugin file). Tests used yet another hardcoded path and referenced a nonexistent lib/ dir. - Remove misleading skills path from bootstrap text; the agent should use the native skill tool, not read files by path - Fix test setup to create a consistent layout matching the plugin's ../../skills resolution - Export SUPERPOWERS_SKILLS_DIR from setup.sh so tests use a single source of truth - Add regression test that bootstrap doesn't advertise the old path - Remove broken cp of nonexistent lib/ directory Fixes #847
…sage Move bootstrap injection from experimental.chat.system.transform to experimental.chat.messages.transform, prepending to the first user message instead of adding a system message. This avoids two issues: - System messages repeated every turn inflate token usage (#750) - Multiple system messages break Qwen and other models (#894) Tested on OpenCode 1.3.2 with Claude Sonnet 4.5 — brainstorming skill fires correctly on "Let's make a React to do list" prompt.
Design for detect-and-defer worktree support. Superpowers defers to native harness worktree systems when available, falls back to manual git worktree creation when not. Covers Phases 0-2: detection, consent, native tool preference, finishing state detection, and three bug fixes (#940, #999, #238). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix Bug #999 order: merge → verify → remove worktree → delete branch (avoids losing work if merge fails after worktree removal) - Add submodule guard to Step 0 detection (GIT_DIR != GIT_COMMON is also true in submodules) - Preserve global path (~/.config/superpowers/worktrees/) in detection for backward compatibility, just stop offering it to new users - Add step numbering note and implementation notes section - Expand provenance heuristic to cover global path and manual creation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Step 1a is the load-bearing assumption, not just a risk — if it fails, the entire design needs rework. TDD validation must be first impl task. - #1009 resolution depends on Step 1a working, stated explicitly - #574 honestly deferred, not "partially addressed" - Add hooks symlink to Step 1b (PR #965 idea, prevents silent hook loss) - Add stale worktree pruning to Step 5 (PR #1072 idea, one-line self-heal) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks: TDD gate for Step 1a, using-git-worktrees rewrite, finishing-a-development-branch rewrite, integration updates, end-to-end validation. Task 1 is a hard gate — if native tool preference fails RED/GREEN, stop and redesign. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gate test for Step 1a — validates agents prefer EnterWorktree over git worktree add on Claude Code. Must pass before skill rewrite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Step 0: GIT_DIR != GIT_COMMON detection (skip if already isolated) Step 0 consent: opt-in prompt before creating worktree (#991) Step 1a: native tool preference (short, first, declarative) Step 1b: git worktree fallback with hooks symlink and legacy path compat Submodule guard prevents false detection Platform-neutral instruction file references (#1049) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…RI-974) Step 2: environment detection (GIT_DIR != GIT_COMMON) before presenting menu Detached HEAD: reduced 3-option menu (no merge from detached HEAD) Provenance-based cleanup: .worktrees/ = ours, anything else = hands off Bug #940: Option 2 no longer cleans up worktree Bug #999: merge -> verify -> remove worktree -> delete branch Bug #238: cd to main repo root before git worktree remove Stale worktree pruning after removal (git worktree prune) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
using-git-worktrees: submodule guard now says "treat as normal repo" instead of "proceed to Step 1" (preserves consent flow) using-git-worktrees: directory priority summaries include global legacy finishing-a-development-branch: move git branch -d after Step 6 cleanup to make Bug #999 ordering unambiguous (merge -> worktree remove -> branch delete) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove REQUIRED language from executing-plans and subagent-driven-development. Consent and detection now live inside using-git-worktrees itself. Fix stale 'created by brainstorming' claim in writing-plans. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…RI-974) The creation skill supports both .worktrees/ and worktrees/ directories, but the finishing skill's cleanup only checked .worktrees/. Worktrees under the non-hidden path would be orphaned on merge or discard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… (PRI-974)
Step 1a failed at 2/6 with the spec's original abstract text ("use your
native tool"). Three REFACTOR iterations found what works (50/50 runs):
1. Explicit tool naming — "do you have EnterWorktree, WorktreeCreate..."
transforms interpretation into factual toolkit check
2. Consent bridge — "user's consent is your authorization" directly
addresses EnterWorktree's "ONLY when user explicitly asks" guardrail
3. Red Flag entry naming the specific anti-pattern
File split was tested but proven unnecessary — the fix is the Step 1a
text quality, not physical separation of git commands. Control test
with full 240-line skill (all git commands visible) passed 20/20.
Test script updated: supports batch runs (./test.sh green 20), "all"
phase, and checks absence of git worktree add (reliable signal) rather
than presence of EnterWorktree text (agent sometimes omits tool name).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Step 1a's original "deliberately short, abstract" design was disproven by TDD (2/6 pass rate). Spec now documents the validated approach: explicit tool naming + consent bridge + red flag (50/50 pass rate). - Design Principles: updated to reflect explicit naming over abstraction - Step 1a: replaced abstract text with validated approach, added design note explaining the TDD revision and why file splitting was unnecessary - Risks: Step 1a risk marked RESOLVED with cross-platform validation table and residual risk note about upstream tool description dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Research confirmed Claude Code is currently the only harness with an agent-callable mid-session worktree tool. All others either create worktrees before the agent starts (Codex App, Gemini, Cursor) or have no native support (Codex CLI, OpenCode). Table now shows: what was actually tested (Claude Code 50/50, Codex CLI 6/6), what was simulated (Codex App 1/1), and what's untested (Gemini, Cursor, OpenCode). Step 1a is forward-compatible for when other harnesses add agent-callable tools. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tested on Gemini CLI (gemini -p) and Cursor Agent (cursor-agent -p): - Gemini: Step 0 detection 1/1, Step 1b fallback 1/1 - Cursor: Step 0 detection 1/1, Step 1b fallback 1/1 Both correctly identified no native agent-callable worktree tool, fell through to git worktree add, and performed safety verification. Both correctly detected existing worktrees and skipped creation. 5 of 6 harnesses now tested. Only OpenCode untested (no CLI access). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Git worktrees inherit hooks from the main repo automatically via $GIT_COMMON_DIR — this has been the case since git 2.5 (2015). The symlink step was based on an incorrect premise from PR #965 and also fails in practice (.git is a file in worktrees, not a dir). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Consent prompt: drop "(y/n)" and add escape valve for users who have already declared their worktree preference in global or project agent instruction files. - Directory selection: reorder to put declared user preference ahead of observed filesystem state, and reframe the default as "if no other guidance available". - Sandbox fallback: require explicitly informing the user that the sandbox blocked creation, not just "report accordingly". - writing-plans: fully qualify the superpowers:using-git-worktrees reference. - Plan doc: mirror the consent-prompt change. Step 1a native-tool framing and the helper-scripts suggestion are still outstanding — the first needs a benchmark re-run before softer phrasing can be adopted without regressing compliance; the second is exploratory and will get a thread reply. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address obra's comment on explicit step numbers / prescriptive tone. Drops "STOP HERE if available", the "If YES:" gate, and the "even if / even if / NO EXCEPTIONS" reinforcement paragraph. Keeps the specific tool-name anchors (EnterWorktree, WorktreeCreate, /worktree, --worktree), which the original TDD data showed are load-bearing. A/B verified against drill harness on the 3 creation/consent scenarios (consent-flow, creation-from-main, creation-from-main-spec-aware): baseline explicit wording scored 12/12 criteria, softened wording also scored 12/12. The "agent used the most appropriate tool" criterion passed in all 3 softened runs — agents still picked EnterWorktree via ToolSearch without the imperative framing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Jesse flagged that the verbose CLAUDE.md/AGENTS.md/GEMINI.md/.cursorrules enumeration (a) chews tokens, (b) confuses models that anchor on exact strings, and (c) is repeated DRY-violatingly across 3+ locations. Replace with abstract "your instructions" framing in four spots: - skills/using-git-worktrees/SKILL.md Step 0 → Step 1 transition - skills/using-git-worktrees/SKILL.md Step 1b Directory Selection - docs/superpowers/plans/2026-04-06-worktree-rototill.md (both mirror locations) Same intent, harness-agnostic phrasing, ~half the tokens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: replace hardcoded /Users/jesse with generic placeholders (#858)
adjust worktree handling and defer to harness tools when avail (PRI-974)
PR #1121's Step 0 consent prompt was intended as a bridge to EnterWorktree's built-in "ONLY when user explicitly asks" guardrail, but in context-diluted real-world sessions agents rationalize their way out of asking AND fail to create a worktree — falling back to a plain feature branch with reasoning like "EnterWorktree is restricted to explicit worktree requests only, which this isn't." Changes: - Delete Step 0 consent ask paragraph; replace with "skill invocation IS the request for isolation" bridge language - Restore imperative Step 1a framing ("STOP HERE if available", "No exceptions") that 998c40b softened during PR #1121 review - Add Red Flag entries naming the specific anti-patterns: stopping to ask when skill is already invoked; falling back to a plain feature branch because the native tool feels "restricted to explicit requests" Preserves the valuable parts of PRI-974: Step 0 detection (GIT_DIR != GIT_COMMON), submodule guard, native-tool preference, cleanup fixes. Only walks back the consent-ask construct. Drill companion changes ship in a separate PR in the drill repo under the same ticket. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem are you trying to solve?
Worktrees are almost never being triggered automatically anymore — agents starting isolated feature work rationalize their way out of creating one and settle for a plain feature branch. Transcript from a loaded real-world session (multiple skills + MCPs, non-trivial CLAUDE.md, uncommitted noise), immediately after
using-git-worktreesStep 0 detection:The agent skipped Step 0's consent prompt, paraphrased the native tool's own "ONLY when user explicitly asks" guardrail as authoritative over the skill, and invented a "close enough" escape hatch. Users who previously got automatic isolation now have to manually ask every session — regression from pre-worktree-worktree-rototill behavior.
What does this PR change?
Removes the Step 0 consent prompt from
skills/using-git-worktrees/SKILL.mdand replaces it with explicit "skill invocation IS the request for isolation" bridge language. Restores imperative Step 1a framing ("STOP HERE if available", "No exceptions") that commits 98263ce and 998c40b softened during worktree-worktree-rototill PR review. Adds two Red Flag entries naming the anti-patterns from the transcript.Is this change appropriate for the core library?
Yes.
using-git-worktreesis a core-library skill used by every user who starts isolated feature work viaexecuting-plansorsubagent-driven-development. This PR modifies existing core behavior — no new skill, no domain-specific functionality, no third-party integration.What alternatives did you consider?
Does this PR contain multiple unrelated changes?
No. Single file, single concern.
Existing PRs
executing-plans,subagent-driven-development), not about a user-facing consent prompt inside the skill. Removing the in-skill prompt does not re-make worktrees mandatory — the opt-in concept lives at the caller layer.Environment tested
claude-opus-4-6Evaluation
superpowers:using-git-worktrees, ran Step 0, and produced the transcript in the Problem section.EnterWorktreeimmediately without stopping to ask; in the detection scenario it correctly skipped creation. Grep across all session logs found none of the anti-pattern phrases the PR targets ("restricted to explicit", "fall back to a feature branch", "work in place instead").Rigor
superpowers:writing-skillsand completed adversarial pressure testing (paste results below)Unchecked-box disclosure: I ran 4 scenarios at 1 run each, not a full
writing-skillspressure-test batch. The Red Flag entries being added name anti-patterns observed directly in the quoted transcript — not speculative. Happy to run a 20-runs-per-scenario sweep if reviewers want heavier numbers before merging.Human review
Drew reviewed the investigation, the root-cause reframe, the alternatives, and the patch strategy before I cut the branch. Drew has not yet reviewed the final committed diff — that review is happening on this PR.
🤖 Generated with Claude Code