Skip to content

feat(worktree): ship /dev multi-repo workspace orchestration (FR-001..FR-024)#1

Merged
NeverMore93 merged 9 commits intomainfrom
001-workspace-worktree-orchestration
Apr 22, 2026
Merged

feat(worktree): ship /dev multi-repo workspace orchestration (FR-001..FR-024)#1
NeverMore93 merged 9 commits intomainfrom
001-workspace-worktree-orchestration

Conversation

@NeverMore93
Copy link
Copy Markdown
Owner

@NeverMore93 NeverMore93 commented Apr 19, 2026

PR Description — 001-workspace-worktree-orchestration

Copy-paste target for the GitHub PR body. Reflects session 2026-04-13 through 2026-04-19.


Title

feat(worktree): ship /dev multi-repo workspace orchestration (FR-001..FR-024)

Summary

Introduces /dev <name> as a first-class slash command for creating or reconciling a multi-repo development workspace. Running /dev prd_1 auto-detects git repos in the current directory, creates mirrored worktrees under <cwd>/../worktrees/prd_1/<repo>/ on a date-stamped branch per repo, runs per-repo sync + postCreate hooks, and forks the current OpenCode session — all in one step, fully headless.

Both a slash command surface (via .opencode/commands/dev.md, auto-created by the plugin) and an AI tool (worktree_workspace_create) exist and are MVP-equivalent — the slash command delegates to the tool via $ARGUMENTS substitution.

Motivation

Users working on PRDs that span multiple repos had to git worktree add by hand for each repo, resolve branch naming by convention, and fork sessions manually. The end-to-end setup took minutes and was error-prone. /dev prd_1 collapses this to one command with consistent naming, partial-success semantics, and reconciliation on re-run.

This PR completes the spec-driven workflow tracked in specs/001-workspace-worktree-orchestration/ — 24 functional requirements, 3 user stories, and 3 clarification sessions (2026-04-13, 04-14, 04-17). The final spec state has been analyzed for consistency (/speckit.analyze twice) and code has been aligned with independent code review (pr-review-toolkit:code-reviewer).

Change highlights

New capability — FR-024 auto-created slash command

  • src/plugin/worktree/dev-command.ts (NEW) — ensureDevCommand() idempotently creates .opencode/commands/dev.md on plugin activation. Uses OpenCode's native custom-command convention (verified against packages/opencode/src/config/command.ts:29 in the upstream OpenCode source).
  • src/plugin/worktree.ts — wires ensureDevCommand(directory, log) into the plugin entry alongside the existing loadWorktreeConfig bootstrap.

Multi-repo workspace orchestration — previously tracked as T004..T023

  • src/plugin/worktree/workspace.ts — name validation (FR-002 regex), namespace conflict check against OpenCode built-in commands + existing .opencode/commands/<name>.md, auto-detect direct-child git repos, target path resolution with nesting rejection, per-path mutex.
  • src/plugin/worktree/workspace-create.ts — branch-collision pre-check with FR-009 dual-path (whole-reject vs per-repo failed), reconcile planning with FR-022 health check + FR-007 orphan handling + FR-008 ghost prune, parallel worktree creation with FR-023 partial-rollback semantics, response assembly matching FR-019 schema.
  • src/plugin/worktree/workspace-session.ts — single workspace-level session fork/reuse (FR-013/FR-014), always headless per FR-018.
  • src/plugin/worktree/state.ts — extended with workspace_associations + workspace_members tables per FR-021 (per-project DB only, no global registry).

Single-repo headless mode — US3

  • worktree_create tool gains optional repoPath + headless parameters for SDK-driven callers; legacy behaviour preserved when both are unset (FR-020).

Spec drift fixes applied 2026-04-19

Four drifts identified via literal-Grep audit against the 2026-04-17 spec and fixed:

  1. FR-005: removed the iteration-2 /- substitution rule (was unauthorised, caused silent collision between feature/login and feature-login).
  2. FR-005: changed detached-HEAD SHA length from 8 to 12 hex chars (--short=12) for collision resistance.
  3. FR-009: checkBranchCollisions refactored from Result<void, CollisionError[]> to PreCheckOutcome { collisions, preCheckFailures }, giving the caller both (a) whole-command reject on confirmed branch collision and (b) per-repo status="failed" on pre-check exception.
  4. T008 / H8: added checkWorkspaceNameAvailable — rejects workspace names that collide with OpenCode built-in commands (init, review) or an existing .opencode/commands/<name>.md file (excluding our own FR-024 dev.md).

Bonus fixes

  • CollisionError.push used the wrong field key (name instead of the declared repoName); fixed for type consistency and correct rendering in rejection messages.
  • Stale JSDoc in planRepoWorktrees still described the removed /- rule and 8-char SHA; updated to reflect the 2026-04-17 spec.

Testing

  • Static: all four new / modified source entry points pass bun build --target=bun --external '*' with zero errors (full dependency chain resolves internally; external deps validated upstream in OCX monorepo).
  • Code review: pr-review-toolkit:code-reviewer verified each of FR-005, FR-009, FR-019, FR-022, FR-024 against the implementation file:line with no unresolved issues after the stale-docstring fix.
  • Smoke tests: full 10-scenario manual runbook at specs/001-workspace-worktree-orchestration/context/go-live-runbook.md — operator runs before merge.
  • Automated tests: none in this facade repository; test harness lives in the OCX monorepo upstream.

Spec alignment & audit trail

  • specs/001-workspace-worktree-orchestration/spec.md — 24 FRs, 3 user stories, 3 clarification sessions documented in-file.
  • specs/001-workspace-worktree-orchestration/plan.md — Constitution Check re-evaluated, risk register updated for FR-009 dual-path, integration flow shows current branch computation.
  • specs/001-workspace-worktree-orchestration/tasks.md — T008 / T011 / T012 markers moved from [需复审] to [已对齐 2026-04-19] with fix notes; T024 marked [x].
  • specs/001-workspace-worktree-orchestration/checklists/ — iteration-2 split into requirements, state, contract, error; iteration-1 archived. 8 of 51 items resolved; remainder are LOW-priority implementation detail.

Known limitations

  1. Hot-reload of .opencode/commands/dev.md — not explicitly verified. Worst case: user restarts OpenCode once after first plugin install, then /dev is recognised for all subsequent runs.
  2. Reserved command list is hard-coded{init, review} matches OpenCode packages/opencode/src/command/index.ts:60-63 as of 2026-04-19. If upstream adds more built-in prompt commands, this list needs a manual update; a future refactor could query client.command.list() dynamically.
  3. No local test harness — per repository contract, full test flow is upstream.

Constitution compliance

Per .specify/memory/constitution.md v1.0.0:

  • I. Facade Fidelity: ✅ new behaviour is additive; no claim in README that's not actually implemented.
  • II. Spec-Driven Change Control: ✅ spec → clarify → checklist → plan → tasks → implement executed in that order; drift fixes applied in the same change that updated plan + tasks + checklists.
  • III. Additive Compatibility: ✅ FR-020 preserves legacy worktree_create and worktree_delete unchanged; T020 retains the interactive terminal-and-fork path.
  • IV. Explicit State Ownership: ✅ workspace_associations + workspace_members carry all workspace metadata; no directory-shape inference.
  • V. Directory Contract Hygiene: ✅ new modules live under src/plugin/worktree/ which has established CLAUDE.md coverage; new context note + runbook live under the feature directory.

Reviewer checklist

  • Smoke test scenarios S1–S10 all pass (see go-live-runbook.md)
  • /dev prd_1 in a test directory creates worktrees/prd_1/<repo> with expected branch names
  • Second /dev prd_1 reports reused for healthy members (no date re-roll)
  • /dev init is rejected with clear namespace-conflict error
  • Branch with / in name (e.g., HEAD on feature/login) yields dev_feature/login_<name>_<date> — slash preserved
  • Detached HEAD yields 12-char SHA in branch name, not 8
  • Legacy worktree_create behaviour unchanged
  • bun build (with external deps installed) compiles cleanly
  • Upstream OCX backport filed (if applicable)

Summary by CodeRabbit

  • New Features

    • Headless multi-repo workspace creation/reconcile: new workspace tool + /dev , returns sessionId and per-repo statuses; worktree creation supports headless SDK flows and explicit repoPath.
  • Documentation

    • Large additions: governance, templates, specs, plans, tasks, checklists, runbook, README updates describing workspace behavior and policies.
  • Tools & Workflows

    • New spec-driven agent/CLI workflows for analyze, checklist, clarify, plan, implement, tasks→issues and prereq helpers.
  • Chores

    • Added .idea to .gitignore.

wentaoyuan added 2 commits April 19, 2026 21:13
…t guidance

Introduces the full SpecKit-driven design artefact set for multi-repo
workspace orchestration (feature 001-workspace-worktree-orchestration).
Captures three clarification sessions (2026-04-13, 04-14, 04-17), the
24-FR spec, the execution plan with Constitution Check, the 24-task
breakdown, and four quality checklists.

Adds repository-level governance so agents and reviewers can act from
a stable context:
  - CLAUDE.md hierarchy for per-directory responsibility boundaries
  - AGENTS.md with current runtime truth + resolved design decisions
  - .specify/ SpecKit templates, scripts, stable memory
  - .claude/ commands and agent definitions
  - .agents/ skills including workspace-worktree-context
  - .gitignore entries for local state

Feature artefacts under specs/001-workspace-worktree-orchestration/:
  - spec.md (24 FRs, 3 user stories)
  - plan.md (Constitution PASS, risk register, FR-023 ordering)
  - tasks.md (T001..T024 ordered by phase, MVP-complete)
  - checklists/{requirements,state,contract,error}.md (iteration-2)
  - checklists/archive/ (iteration-1 kept for audit trail)
  - context/{current-state,target-state,go-live-runbook,pr-description}.md
  - cases/{reconcile-cases,repo-path-resolution,workspace-metadata-authority}.md

Implementation follows in the next commit.

Refs: Constitution v1.0.0 Principle II (spec-driven change control);
       spec FR-001..FR-024.
…mmand

Ships the workspace orchestration feature designed in the preceding
spec commit. Users can now run /dev <name> to create or reconcile a
mirrored multi-repo workspace under <cwd>/../worktrees/<name>/ with
one forked OpenCode session — fully headless.

New modules under src/plugin/worktree/:

  dev-command.ts        FR-024: ensureDevCommand idempotently creates
                        .opencode/commands/dev.md; OpenCode's native
                        command scan picks it up so /dev <name> works
                        alongside the worktree_workspace_create tool.

  workspace.ts          FR-002 name validation (regex); FR-003 auto-detect
                        git repos in direct subdirs; FR-004 target path
                        resolution with nesting rejection; FR-016 warnings
                        for unrelated root-level content; FR-017 per-path
                        mutex; T008 namespace conflict check against
                        OpenCode built-in commands (init, review) and
                        existing .opencode/commands/<name>.md files.

  workspace-create.ts   FR-009 dual-path pre-check (confirmed collision
                        = whole reject; per-repo pre-check failure =
                        per-repo status=failed); FR-005 branch naming
                        (local short name preserved as-is; SHA[:12] for
                        detached HEAD); FR-007 orphan handling;
                        FR-008 ghost prune + retry; FR-022 health check;
                        FR-023 strict mutation ordering with partial-
                        rollback semantics; FR-019 response schema.

  workspace-session.ts  FR-013/FR-014 single workspace session
                        fork/reuse/refork; FR-018 always headless.

  config.ts             Shared .opencode/worktree.jsonc schema + loader
                        (extracted from worktree.ts for reuse).

  git.ts                Shell-safe git wrapper, branch name validation,
                        Result<T,E> type (extracted from worktree.ts).

  sync.ts               File copy, symlink, hook execution with timeout
                        (extracted from worktree.ts).

Modified:

  worktree.ts           Wires ensureDevCommand into plugin entry
                        alongside loadWorktreeConfig; adds
                        worktree_workspace_create AI tool; preserves
                        legacy worktree_create/worktree_delete
                        behaviour unchanged (FR-020).

  state.ts              Extends SQLite schema with workspace_associations
                        and workspace_members tables per FR-021
                        (per-project DB only; no global registry).

  README.md             Documents /dev <name> usage and the
                        worktree_workspace_create tool.

Spec drift fixes applied in this commit (found via literal-Grep audit
against the 2026-04-17 spec):

  - FR-005: removed unauthorised iteration-2 "/"→"-" substitution rule
    (baseBranch.replace(/\//g, "-") deleted); now preserves slashes
    verbatim in branch refs.
  - FR-005: detached HEAD SHA length 8 → 12 (--short=8 → --short=12)
    for collision resistance.
  - FR-009: checkBranchCollisions refactored from
    Result<void, CollisionError[]> to PreCheckOutcome with separate
    collisions and preCheckFailures arrays; caller now handles both
    paths distinctly.
  - T008/H8: added checkWorkspaceNameAvailable for namespace conflict
    detection before any worktree mutation or FR-024 auto-create.
  - CollisionError: fixed pre-existing bug where push used the field
    name "name" but the interface declared "repoName"; no behavioural
    change for callers, fixes type consistency.
  - planRepoWorktrees JSDoc: removed stale mention of "/"→"-" rule and
    "abbreviated SHA" (found by pr-review-toolkit:code-reviewer in
    post-implementation review).

Static validation: bun build --target=bun --external '*' succeeds on
worktree.ts, workspace.ts, workspace-create.ts with zero errors
(30ms). Full type/link check runs upstream in OCX monorepo.

Smoke tests: see
specs/001-workspace-worktree-orchestration/context/go-live-runbook.md
for S1..S10 scenarios covering FR-005/009/019/022/023/024 plus
namespace conflict and partial-success paths.

Refs: spec FR-001..FR-024; plan.md Risk Register; tasks T001..T024
(all previously marked [需复审] items moved to [已对齐 2026-04-19]).
@NeverMore93
Copy link
Copy Markdown
Owner Author

@codex review

@NeverMore93
Copy link
Copy Markdown
Owner Author

@claude review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 19, 2026

Warning

Rate limit exceeded

@NeverMore93 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 0 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 0 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40d95155-a256-4758-ac83-0ab393955657

📥 Commits

Reviewing files that changed from the base of the PR and between dd94092 and 42cc8e5.

📒 Files selected for processing (1)
  • src/plugin/worktree/workspace-create.ts

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds repository governance docs and agent skill wrappers; introduces SpecKit templates, PowerShell automation, and multiple CLAUDE command specs; adds a full workspace-level worktree orchestration feature (spec/plan/tasks/context/cases/checklists); refactors plugin into modular worktree modules and implements headless workspace creation tooling and per-project state persistence.

Changes

Cohort / File(s) Summary
Governance & agent boundaries
CLAUDE.md, AGENTS.md, .claude/CLAUDE.md, .agents/CLAUDE.md, .specify/CLAUDE.md, src/CLAUDE.md, src/plugin/CLAUDE.md, src/plugin/.../CLAUDE.md, specs/CLAUDE.md, .agents/skills/.../CLAUDE.md, .specify/memory/.../CLAUDE.md
Add many CLAUDE/AGENTS docs defining responsibility boundaries and allowed/forbidden content across agent-facing, specify, templates, memory, and src subtrees.
Agent skills & wrappers
.agents/skills/README.md, .agents/skills/workspace-worktree-context/SKILL.md, .agents/skills/CLAUDE.md, .specify/memory/skills/*
Introduce repo-local skill wrappers, README, skill map, and guidance for exposing workspace/worktree context to agents without vendoring skill implementations.
SpecKit command specs (Claude commands)
.claude/commands/speckit.*.md (specify,clarify,plan,tasks,implement,checklist,analyze,constitution,taskstoissues)
Add detailed command/workflow specifications for the speckit toolset including prechecks, progressive loading, hook semantics, outputs, and file handling rules.
SpecKit templates & stable memory
.specify/templates/*, .specify/memory/*, .specify/memory/contexts/*, .specify/memory/cases/*, .specify/memory/skills/*
Add spec/plan/tasks/checklist/constitution/agent-file templates, stable memory docs, compatibility pointers, constitution, project memory, skill maps, and recommended workflows.
PowerShell automation & Codex integration
.specify/scripts/powershell/common.ps1, create-new-feature.ps1, setup-plan.ps1, check-prerequisites.ps1, update-agent-context.ps1, .specify/integrations/codex/scripts/install-user-speckit.ps1, .../update-context.ps1
Introduce shared PowerShell helpers for repo/feature discovery, feature creation, plan setup, prerequisite checks, agent-context generation, and Codex skill install/update utilities.
Feature: Workspace worktree orchestration (001)
specs/001-workspace-worktree-orchestration/spec.md, plan.md, tasks.md, context/*, cases/*, checklists/*
Add full feature specification, plan, tasks, contexts (current/target/go-live/pr-description), case records (reconcile, repo-path resolution, metadata authority), and multiple quality checklists.
Plugin refactor — worktree modules
src/plugin/worktree/config.ts, git.ts, sync.ts, state.ts, workspace.ts, workspace-create.ts, workspace-session.ts, dev-command.ts
Extract modular worktree logic: config loading, git/worktree helpers, safe sync/symlink/hooks, pooled per-project SQLite state, repo detection/locking, orchestration/execution, session resolution, and dev-command auto-creation.
Main plugin integration & behavior changes
src/plugin/worktree.ts
Refactor main plugin to import new modules, add worktree_workspace_create tool, support headless and repoPath args for worktree_create, await ensureDevCommand(...), and switch synchronous DB retry sleep to async.
SpecKit templates & agent files
.specify/templates/agent-file-template.md, .specify/templates/*, .agents/skills/.../SKILL.md, .specify/memory/skills/project-skill-map.md
Add agent-file template, various SpecKit templates, repo-local SKILL.md wrappers, and a project skill map linking recommended skill combinations to workflows.
Docs, README & misc updates
README.md, many added AGENTS.md/CLAUDE.md files, .gitignore
Document multi-repo workspace flow, new tool worktree_workspace_create, hook timeout semantics, diagrams, and add /.idea/ to .gitignore.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Plugin as OpenCode Plugin
    participant Validator as Workspace Validator
    participant PreCheck as Collision PreCheck
    participant Executor as Worktree Executor
    participant DB as SQLite (per-repo)
    participant Session as Session Manager

    User->>Plugin: /dev <name> or worktree_workspace_create(name)
    Plugin->>Validator: validate name, detect repos, resolve target path
    Validator-->>Plugin: WorkspaceName, DetectedRepos, targetPath
    Plugin->>DB: load existing workspace members per repo
    DB-->>Plugin: existing members
    Plugin->>PreCheck: plan actions, run branch collision checks
    PreCheck-->>Plugin: collisions or per-repo pre-check failures
    alt collision detected
        Plugin-->>User: Error (branch collision) — abort
    else proceed
        Plugin->>Executor: execute per-repo create/retry/reuse (parallel)
        Executor->>Executor: git worktree add / prune, sync files, run hooks
        Executor-->>Plugin: MemberOutcome[] (created/reused/retried/failed)
        Plugin->>Session: resolveWorkspaceSession (reuse or fork)
        Session-->>Plugin: sessionBinding (forked/reused) or error
        alt fork succeeded
            Plugin->>DB: persist workspace association and members
            DB-->>Plugin: persisted
            Plugin-->>User: WorkspaceCreateResult (sessionId, repos, warnings)
        else fork failed
            Plugin-->>User: Error (worktrees on disk, no DB records)
        end
    end
    Plugin->>Validator: release workspace lock
Loading
sequenceDiagram
    participant Dev
    participant Script as create-new-feature.ps1
    participant Git
    participant FS
    participant Template as Template Resolver

    Dev->>Script: run with feature description
    Script->>Script: compute branch suffix / next feature number
    Script->>Git: git checkout -b <branch>
    alt branch exists
        Git-->>Script: error
        Script-->>Dev: exit with failure
    else created
        Script->>FS: create specs/<branch>/ directory
        Script->>Template: Resolve spec-template
        alt template found
            Template-->>Script: path
            Script->>FS: copy template -> spec.md
        else
            Script->>FS: create empty spec.md
        end
        Script-->>Dev: output branch name, spec path, feature metadata
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐰 I hopped through CLAUDE and templates bright,
I stitched worktrees, checked each branch by light,
Specs, scripts, and hooks in tidy rows,
Sessions fork where reconciliation goes,
A rabbit cheers: build, branch, unite! 🌿

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 001-workspace-worktree-orchestration

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces multi-repo workspace orchestration to the OpenCode worktree plugin, enabling the creation and reconciliation of mirrored worktrees across multiple repositories via a new /dev slash command. The implementation refactors the core plugin into modular components and adds extensive SpecKit workflow assets, document templates, and project memory. Feedback suggests improving cross-platform compatibility for hook execution, increasing the robustness of git error handling by avoiding fragile string matching, and dynamically querying reserved command names to enhance maintainability.

Comment thread src/plugin/worktree/sync.ts Outdated
Comment thread src/plugin/worktree/workspace-create.ts Outdated
Comment thread src/plugin/worktree/workspace.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (21)
.specify/memory/contexts/repository-context.md-8-8 (1)

8-8: ⚠️ Potential issue | 🟡 Minor

Update relative path to AGENTS.md at the repo root.

Line 8 currently points to ../../AGENTS.md, which resolves to .specify/AGENTS.md. Since AGENTS.md is located at the repository root, the correct relative path is ../../../AGENTS.md.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/memory/contexts/repository-context.md at line 8, The relative link
to AGENTS.md is incorrect; update the reference string in
.specify/memory/contexts/repository-context.md from "../../AGENTS.md" to
"../../../AGENTS.md" so it points to the repository root AGENTS.md (locate the
line containing "../../AGENTS.md" and replace it with "../../../AGENTS.md").
.agents/skills/README.md-14-15 (1)

14-15: ⚠️ Potential issue | 🟡 Minor

Replace machine-specific absolute paths with repo-relative links.
Line 14 and Line 15 hardcode E:/..., which breaks for other contributors and CI docs rendering. Use relative paths instead.

Proposed fix
-  - `E:/workspaces/opencode-worktree/.specify/memory/skills/project-skill-map.md`
-  - `E:/workspaces/opencode-worktree/.agents/skills/workspace-worktree-context/SKILL.md`
+  - `.specify/memory/skills/project-skill-map.md`
+  - `.agents/skills/workspace-worktree-context/SKILL.md`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/skills/README.md around lines 14 - 15, The README contains
machine-specific absolute paths
("E:/workspaces/opencode-worktree/.specify/memory/skills/project-skill-map.md"
and
"E:/workspaces/opencode-worktree/.agents/skills/workspace-worktree-context/SKILL.md");
replace them with repo-relative paths (for example
".specify/memory/skills/project-skill-map.md" and
".agents/skills/workspace-worktree-context/SKILL.md") and use Markdown relative
links so they resolve for other contributors and CI.
specs/001-workspace-worktree-orchestration/checklists/archive/2026-04-15-requirements-iteration-1.md-6-6 (1)

6-6: ⚠️ Potential issue | 🟡 Minor

Fix the archived checklist’s spec link.

From checklists/archive/, ../spec.md points to checklists/spec.md; the feature spec is two levels up.

🛠️ Proposed fix
-**Feature**: [spec.md](../spec.md)
+**Feature**: [spec.md](../../spec.md)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@specs/001-workspace-worktree-orchestration/checklists/archive/2026-04-15-requirements-iteration-1.md`
at line 6, The markdown link under the heading "**Feature**:
[spec.md](../spec.md)" is pointing one directory too shallow; update the link
target to "../../spec.md" so the feature spec resolves correctly from the
archived checklist (replace the existing "../spec.md" URI in the
"[spec.md](../spec.md)" link with "../../spec.md").
.specify/memory/constitution.md-7-11 (1)

7-11: ⚠️ Potential issue | 🟡 Minor

Replace local absolute paths with repo-relative paths.

The sync report commits machine-local E:/workspaces/... paths. Use repo-relative paths so the governance document is portable and does not embed local state. Based on learnings, generated build outputs, vendored dependencies, secrets, or local machine state must not be committed to this repository.

🛠️ Proposed fix
-- ✅ `E:/workspaces/opencode-worktree/.specify/templates/spec-template.md` remains compatible with explicit requirements, assumptions, and edge-case capture.
-- ✅ `E:/workspaces/opencode-worktree/.specify/templates/plan-template.md` remains compatible with Constitution Check usage.
-- ✅ `E:/workspaces/opencode-worktree/.specify/templates/tasks-template.md` remains compatible with doc and cross-cutting task requirements.
-- ✅ `E:/workspaces/opencode-worktree/AGENTS.md` updated in the same change to reflect current-vs-target behavior and canonical context files.
+- ✅ `.specify/templates/spec-template.md` remains compatible with explicit requirements, assumptions, and edge-case capture.
+- ✅ `.specify/templates/plan-template.md` remains compatible with Constitution Check usage.
+- ✅ `.specify/templates/tasks-template.md` remains compatible with doc and cross-cutting task requirements.
+- ✅ `AGENTS.md` updated in the same change to reflect current-vs-target behavior and canonical context files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/memory/constitution.md around lines 7 - 11, Update
.specify/memory/constitution.md to remove machine-local absolute paths (e.g.,
"E:/workspaces/opencode-worktree/...") and replace them with repo-relative paths
that point to the same files (for example ".specify/templates/spec-template.md",
".specify/templates/plan-template.md", ".specify/templates/tasks-template.md",
and "AGENTS.md"); search the document for any occurrences of the "E:/" drive
prefix and substitute repo-relative references, ensuring the sync-report lines
and any explanatory text no longer contain local drive letters or
machine-specific state and keep the documented targets exactly matching the
filenames used in the repository.
specs/001-workspace-worktree-orchestration/checklists/requirements.md-34-38 (1)

34-38: ⚠️ Potential issue | 🟡 Minor

Refresh the stale slash-deferral note.

Lines 28-29 resolve CHK012/CHK013 and state both slash and tool surfaces are MVP, but Line 37 still calls slash deferral the highest-impact open issue. This will mislead checklist consumers.

🛠️ Proposed fix
-- Slash deferral (CHK012, CHK013) is the highest-impact open issue this iteration; resolution likely requires a spec edit pass before `/speckit.plan`.
+- Slash deferral (CHK012, CHK013) was resolved on 2026-04-17 by adding `.opencode/commands/dev.md` delegation to the AI tool.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/checklists/requirements.md` around
lines 34 - 38, Update the "Slash deferral (CHK012, CHK013) is the highest-impact
open issue" note to reflect that CHK012 and CHK013 are resolved and no longer
the highest-impact open issue; specifically remove or replace the phrase about
"highest-impact open issue this iteration" and either point to the current
highest-impact open issue or simply state that slash/tool surfaces are now MVP
and that remaining open items are tracked elsewhere (e.g., /speckit.plan);
ensure you reference CHK012 and CHK013 as resolved and keep the cross-reference
to state.md/contract.md/error.md intact.
.specify/memory/project-memory.md-33-42 (1)

33-42: ⚠️ Potential issue | 🟡 Minor

Remove the .specify/memory/cases feature-case pointer.

Line 38 points feature-local case context back into .specify/memory/, while this document already says cases belong under specs/<feature>/cases/. Keep only the specs/001-workspace-worktree-orchestration/cases/... reference. Based on learnings, put feature-local cases under specs/<feature>/cases/, not under .specify/memory/.

🛠️ Proposed fix
   - `.specify/memory/active-feature-context.md`
   - `.specify/memory/contexts/repository-context.md`
   - `.specify/memory/contexts/current-vs-target-state.md`
-  - `.specify/memory/cases/001-workspace-worktree-orchestration.md`
   - `specs/001-workspace-worktree-orchestration/spec.md`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/memory/project-memory.md around lines 33 - 42, Remove the
feature-local case pointer that references
`.specify/memory/cases/001-workspace-worktree-orchestration.md` from the list
and keep only the canonical cases entry under
`specs/001-workspace-worktree-orchestration/cases/reconcile-cases.md`; update
the bullet list in project-memory.md so it no longer includes
`.specify/memory/cases/...` and ensures cases are referenced exclusively via
`specs/<feature>/cases/...` (e.g.,
`specs/001-workspace-worktree-orchestration/cases/reconcile-cases.md`).
AGENTS.md-212-214 (1)

212-214: ⚠️ Potential issue | 🟡 Minor

Heads-up: update-agent-context.ps1 can overwrite this curated file if invoked with -AgentType opencode|codex|amp|kiro-cli|bob|pi.

The guard in lines 212–214 is a human rule, but the PowerShell helper added in this PR ($AGENTS_FILE = ... AGENTS.md) treats AGENTS.md as a regenerable target for six agent keys. If someone wires speckit.plan to use one of those keys, the curated content in this file will be mutated by the script. Consider either (a) documenting a SpecKit-level guard, or (b) making the script skip in-place rewrites when AGENTS.md lacks its generated marker.

Based on learnings: "Do not auto-regenerate AGENTS.md or root CLAUDE.md from generic SpecKit agent-context template without explicit review — treat as curated guidance, not disposable generated summaries."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AGENTS.md` around lines 212 - 214, The PowerShell helper
update-agent-context.ps1 currently treats AGENTS.md (via $AGENTS_FILE) as a
regenerable target and can overwrite curated guidance; change the script so it
first checks AGENTS.md for a generated-marker (or the absence/presence of the
human-rule guard lines) and skip in-place rewrites when the marker is missing,
or add an explicit CLI flag to force overwrite; update-agent-context.ps1 should
log a non-destructive skip and return non-zero when it detects a curated file,
and also update speckit.plan handling to respect the same allowlist/guard
behavior so only files with the generated marker are updated automatically.
specs/001-workspace-worktree-orchestration/checklists/contract.md-25-25 (1)

25-25: ⚠️ Potential issue | 🟡 Minor

Minor: CHK010 wording references SHA[:8] but spec Part 3 moved to SHA[:12].

Per the PR's spec-drift fixes (detached-HEAD SHA length → 12) and AGENTS.md lines 62–66, detached-HEAD base-branch naming now uses SHA[:12]. Consider updating CHK010 to ask about SHA[:12] so reviewers validate the actual contract, not the superseded length.

📝 Proposed wording tweak
-- [ ] CHK010 Is FR-005's SHA[:8] (8 hex chars) sufficient for detached-HEAD branch naming in monorepos with millions of commits — does the spec acknowledge birthday-paradox collision risk or specify a fallback? [Coverage, Gap, Spec §FR-005]
+- [ ] CHK010 Is FR-005's SHA[:12] (12 hex chars) sufficient for detached-HEAD branch naming in monorepos with millions of commits — does the spec acknowledge birthday-paradox collision risk or specify a fallback? [Coverage, Gap, Spec §FR-005]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/checklists/contract.md` at line
25, CHK010 currently asks about SHA[:8] but the spec and AGENTS.md now use
SHA[:12]; update the checklist item CHK010 to reference SHA[:12] (and the
birthday-paradox/fallback question remains relevant) so it validates the actual
contract from FR-005/Part 3; search for CHK010 and FR-005 in contract.md and
update the wording to "SHA[:12]" and ensure any cross-reference to
AGENTS.md/Part 3 remains accurate.
.specify/scripts/powershell/update-agent-context.ps1-309-309 (1)

309-309: ⚠️ Potential issue | 🟡 Minor

Dead variable: $changeAdded is assigned but never read.

Tracked in the loop ($changeAdded = $true at line 328) but never inspected afterward — unlike $techAdded which gates the post-loop insertion at line 345. Either remove it or add a symmetric post-loop check that appends $newChangeEntry when the ## Recent Changes section was never encountered (which is the likely intent for new-style agent files that lack the section).

🔧 Option A: drop the dead variable
-    $inTech = $false; $inChanges = $false; $techAdded = $false; $changeAdded = $false; $existingChanges = 0
+    $inTech = $false; $inChanges = $false; $techAdded = $false; $existingChanges = 0
@@
-            if ($newChangeEntry) { $output.Add($newChangeEntry); $changeAdded = $true }
+            if ($newChangeEntry) { $output.Add($newChangeEntry) }
🔧 Option B: add a symmetric post-loop fallback (preserves intent if section is missing)
     if ($inTech -and -not $techAdded -and $newTechEntries.Count -gt 0) {
         $newTechEntries | ForEach-Object { $output.Add($_) }
     }
+
+    # If the file had no `## Recent Changes` section, append one so this run is recorded.
+    if (-not $changeAdded -and $newChangeEntry) {
+        $output.Add('')
+        $output.Add('## Recent Changes')
+        $output.Add($newChangeEntry)
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/scripts/powershell/update-agent-context.ps1 at line 309, The
variable $changeAdded is set inside the loop but never used after; either remove
$changeAdded entirely or add a symmetric post-loop fallback like the existing
$techAdded check: after the loop, if -not $changeAdded and -not $existingChanges
then append $newChangeEntry under the "## Recent Changes" section (same behavior
as the $techAdded post-loop insertion) so new-style agent files that lack the
section get the changes entry; update references to $changeAdded, the loop where
it’s set, and the post-loop insertion logic to maintain symmetry with $techAdded
and $newChangeEntry.
specs/001-workspace-worktree-orchestration/context/go-live-runbook.md-138-155 (1)

138-155: ⚠️ Potential issue | 🟡 Minor

Add a language to the commit-message fences.

markdownlint flags these unlabeled fences; use text for commit message examples.

Proposed fix
-```
+```text
 spec(001): finalize workspace orchestration — slash command via markdown, FR-009 dual-path, FR-005 no-sub SHA[:12]
@@
-```
+```
 
 ### Commit 2 — Implementation: FR-024 + drift fixes
 
-```
+```text
 feat(worktree): implement FR-024 /dev slash + fix 4 spec drifts
@@
-```
+```

Also applies to: 159-186

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/context/go-live-runbook.md` around
lines 138 - 155, Replace unlabeled triple-backtick fences used for
commit-message examples with labeled fences using text (e.g. change ``` to
```text) so markdownlint stops flagging them; locate the commit message examples
(the fences containing lines like "spec(001): finalize workspace orchestration —
slash command via markdown, FR-009 dual-path, FR-005 no-sub SHA[:12]" and
"feat(worktree): implement FR-024 /dev slash + fix 4 spec drifts" or the "###
Commit 2 — Implementation" block) and add the language tag to the opening fence
(closing fence remains ```), and apply the same change to the other instances
noted in the comment (the additional commit-example fences around the 159-186
area).
specs/001-workspace-worktree-orchestration/plan.md-216-216 (1)

216-216: ⚠️ Potential issue | 🟡 Minor

Add text to the flow-diagram fences.

markdownlint flags these unlabeled fences; these are plain-text diagrams.

Proposed fix
-```
+```text
 User invokes /dev prd_1
@@
-```
+```text
 User invokes /dev prd_1 (existing workspace)
@@
-```
+```text
 SDK calls worktree_create({ branch, headless: true, repoPath })

Also applies to: 251-251, 274-274

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/plan.md` at line 216, Update the
unlabeled triple-backtick flow-diagram fences to specify the language "text" so
markdownlint stops flagging them: replace the three occurrences of ``` that wrap
the diagram blocks (the ones surrounding the blocks starting with "User invokes
/dev prd_1", "User invokes /dev prd_1 (existing workspace)", and "SDK calls
worktree_create({ branch, headless: true, repoPath })") with ```text
respectively.
specs/001-workspace-worktree-orchestration/context/target-state.md-16-18 (1)

16-18: ⚠️ Potential issue | 🟡 Minor

Stale spec drift: /- substitution rule was supposed to be removed.

The PR summary lists "removed /- substitution" as one of four spec-drift fixes applied, but this target-state still says / in base_branch is replaced by -. Either the drift fix wasn't applied to this document, or the summary is inaccurate. Please reconcile with the implementation in workspace.ts/workspace-create.ts so spec and code agree on branch-name derivation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/context/target-state.md` around
lines 16 - 18, The spec text in target-state.md still says we replace `/` with
`-` in base_branch, but the implementation in workspace.ts / workspace-create.ts
removed that substitution; update target-state.md to match code by removing the
"`/` in `base_branch` is replaced by `-`" sentence (or rewrite to state that the
stored branch name is reused verbatim and any detachment uses abbreviated SHA)
so the branch-name derivation description in target-state.md matches the logic
in workspace.ts and workspace-create.ts.
src/plugin/worktree/sync.ts-32-50 (1)

32-50: ⚠️ Potential issue | 🟡 Minor

.. substring check is overly broad and baseDir is not normalized.

filePath.includes("..") rejects legitimate filenames that merely contain two dots (e.g., config..bak, a..b.env). Use segment-based detection after normalization. Also, resolved.startsWith(baseDir + path.sep) assumes baseDir is already absolute and without trailing separator; if a caller ever passes a relative or trailing-slash baseDir this comparison can produce wrong results.

Suggested hardening
-export function isPathSafe(filePath: string, baseDir: string, log: Logger): boolean {
-	// Reject absolute paths
-	if (path.isAbsolute(filePath)) {
-		log.warn(`[worktree] Rejected absolute path: ${filePath}`)
-		return false
-	}
-	// Reject obvious path traversal
-	if (filePath.includes("..")) {
-		log.warn(`[worktree] Rejected path traversal: ${filePath}`)
-		return false
-	}
-	// Verify resolved path stays within base directory
-	const resolved = path.resolve(baseDir, filePath)
-	if (!resolved.startsWith(baseDir + path.sep) && resolved !== baseDir) {
+export function isPathSafe(filePath: string, baseDir: string, log: Logger): boolean {
+	if (path.isAbsolute(filePath)) {
+		log.warn(`[worktree] Rejected absolute path: ${filePath}`)
+		return false
+	}
+	const segments = filePath.split(/[\\/]/)
+	if (segments.includes("..")) {
+		log.warn(`[worktree] Rejected path traversal: ${filePath}`)
+		return false
+	}
+	const base = path.resolve(baseDir)
+	const resolved = path.resolve(base, filePath)
+	if (resolved !== base && !resolved.startsWith(base + path.sep)) {
 		log.warn(`[worktree] Path escapes base directory: ${filePath}`)
 		return false
 	}
 	return true
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/sync.ts` around lines 32 - 50, The isPathSafe function
currently rejects any filename containing ".." and compares resolved paths
against a non-normalized baseDir; change it to first normalize/resolve baseDir
(e.g., baseDir = path.resolve(baseDir) and trim any trailing path.sep), then
normalize the incoming filePath (but still reject if path.isAbsolute(filePath)),
compute resolved = path.resolve(baseDir, filePath) and ensure resolved
startsWith(baseDir + path.sep) or equals baseDir; replace the
filePath.includes("..") check with a segment-based check by splitting
path.normalize(filePath) on path.sep (or by comparing path.relative(baseDir,
resolved) and ensuring it does not start with "..") to detect true traversal,
and keep using log.warn in the same cases (referencing isPathSafe, filePath,
baseDir, resolved, path.resolve, path.isAbsolute, path.sep).
src/plugin/worktree/workspace.ts-294-309 (1)

294-309: ⚠️ Potential issue | 🟡 Minor

FR-016 warning can falsely flag previously-managed worktrees as "unrelated".

The warning set is computed as targetEntries \ currentlyDetectedRepos. The spec's edge-case rule says "A repo previously in the workspace is now absent from <cwd> → skip silently, not represented in the current response." But if a user removes repoB from <cwd> and re-runs /dev prd_1, the existing worktrees/prd_1/repoB/ directory will now be listed under "unrelated root-level entries" because it's no longer in the detected repos set — even though it was created by the plugin on a prior run.

Consider cross-referencing workspace member records from the per-project databases (or at least filtering entries that contain a .git file pointing back into a known source repo's .git/worktrees/) before classifying them as unrelated. Otherwise reconciliation runs against a shrunken source set will surface noisy false warnings.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace.ts` around lines 294 - 309, The current
unrelated-entry check in the block using readdir(workspacePath) and repos (Set
from repos.map(r => r.name)) can falsely flag previously-created worktrees as
"unrelated"; update the logic in the workspace reconciliation (the code around
workspacePath, readdir, repoNames and warnings) to filter out directories that
were created by prior runs by checking for known workspace membership before
adding warnings — e.g., consult per-project workspace records (the persistent DB
used by the plugin) for existing members, or inspect each candidate entry for a
.git that points into a known source repo's .git/worktrees/ before classifying
it as unrelated; only push warnings for entries that are neither in the current
repos set nor verified as previously-managed by the plugin.
specs/001-workspace-worktree-orchestration/spec.md-149-152 (1)

149-152: ⚠️ Potential issue | 🟡 Minor

FR ordering inverted: FR-024 appears before FR-023.

Line 151 declares FR-024 and line 152 declares FR-023. Since requirements are numbered for cross-reference (and FR-023 is cited inside FR-024's narrative elsewhere in the spec), readers expect ascending order. Swap the two blocks so FR-023 (mutation ordering) precedes FR-024 (auto-create .opencode/commands/dev.md).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/spec.md` around lines 149 - 152,
The numbered requirement blocks are out of order: FR-024 appears before FR-023;
update the spec so FR-023 (the mutation ordering paragraph referencing FR-009
and FR-015) comes before FR-024 (the auto-create `.opencode/commands/dev.md`
paragraph) by swapping those two blocks so the requirements appear in ascending
numeric order and cross-references resolve correctly.
.specify/scripts/powershell/create-new-feature.ps1-169-179 (1)

169-179: ⚠️ Potential issue | 🟡 Minor

Get-BranchName caps at 3 words whenever there are 5+ meaningful words — intentional?

$maxWords = if ($meaningfulWords.Count -eq 4) { 4 } else { 3 } yields 4 only when the count is exactly 4; with 5, 6, … meaningful words it drops back to 3. The help text claims "2-4 words" and the comment says "first 3-4". If the intent was "at most 4", the condition should be -ge 4:

Proposed fix
-        $maxWords = if ($meaningfulWords.Count -eq 4) { 4 } else { 3 }
+        $maxWords = if ($meaningfulWords.Count -ge 4) { 4 } else { 3 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/scripts/powershell/create-new-feature.ps1 around lines 169 - 179,
The branch-name logic mistakenly sets $maxWords to 4 only when
$meaningfulWords.Count -eq 4, causing counts of 5+ to be limited to 3; update
the condition in the Get-BranchName logic that sets $maxWords (the line using
$maxWords = if ($meaningfulWords.Count -eq 4) { 4 } else { 3 }) so it treats any
count >= 4 as 4 (use -ge 4) thereby enforcing the intended "2-4 words" behavior
when building the result from $meaningfulWords and returning the joined string.
.specify/scripts/powershell/common.ps1-142-146 (1)

142-146: ⚠️ Potential issue | 🟡 Minor

Test-FeatureBranch regex rejects sequential numbers ≥ 1000.

^[0-9]{3}- matches only exactly 3 digits before the dash, but create-new-feature.ps1 uses '{0:000}' -f $Number, which produces 1000-… once there are over 999 features in the repo. After that point, every feature branch will fail Test-FeatureBranch even though it was created by the companion script. Prefer ^\d{3,}- to match "at least 3 digits".

Proposed fix
-    if ($Branch -notmatch '^[0-9]{3}-' -and $Branch -notmatch '^\d{8}-\d{6}-') {
+    if ($Branch -notmatch '^\d{3,}-' -and $Branch -notmatch '^\d{8}-\d{6}-') {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/scripts/powershell/common.ps1 around lines 142 - 146, The
Test-FeatureBranch check currently only accepts exactly three digits before the
dash and will reject branch names like "1000-feature" created by
create-new-feature.ps1; update the regex used on $Branch in Test-FeatureBranch
from the exact-3-digits pattern to an "at least 3 digits" pattern (e.g., use
\d{3,} or [0-9]{3,} before the hyphen) so branches with 3 or more leading digits
are accepted, leaving the rest of the logic and the second timestamp-based regex
unchanged.
.claude/commands/speckit.specify.md-79-82 (1)

79-82: ⚠️ Potential issue | 🟡 Minor

"Bash example" commands point at a .ps1 script.

Lines 79–80 are labeled as Bash invocations but call .specify/scripts/powershell/create-new-feature.ps1 with --json/--short-name (POSIX-style flags), which is the wrong script/flag convention for a bash shell. PowerShell examples on lines 81–82 correctly use -Json/-ShortName. Either the bash examples should call the equivalent bash script (e.g. .specify/scripts/bash/create-new-feature.sh), or all four lines should be unified under one invocation. As-is this will mislead implementers.

Proposed fix
-   - Bash example: `.specify/scripts/powershell/create-new-feature.ps1 "$ARGUMENTS" --json --short-name "user-auth" "Add user authentication"`
-   - Bash (timestamp): `.specify/scripts/powershell/create-new-feature.ps1 "$ARGUMENTS" --json --timestamp --short-name "user-auth" "Add user authentication"`
+   - Bash example: `.specify/scripts/bash/create-new-feature.sh "$ARGUMENTS" --json --short-name "user-auth" "Add user authentication"`
+   - Bash (timestamp): `.specify/scripts/bash/create-new-feature.sh "$ARGUMENTS" --json --timestamp --short-name "user-auth" "Add user authentication"`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/speckit.specify.md around lines 79 - 82, The Bash examples
currently call the PowerShell script
`.specify/scripts/powershell/create-new-feature.ps1` with POSIX flags (`--json`,
`--short-name`) which is incorrect; update the two "Bash example" lines to
invoke the Bash script `.specify/scripts/bash/create-new-feature.sh` (or unify
all four to one canonical invocation) and use POSIX-style flags (`--json`,
`--timestamp`, `--short-name`) for that script, keeping the PowerShell examples
as-is using `-Json`/`-Timestamp`/`-ShortName` for
`.specify/scripts/powershell/create-new-feature.ps1`.
src/plugin/worktree/state.ts-711-712 (1)

711-712: ⚠️ Potential issue | 🟡 Minor

Docstrings drift from SQL.

JSDoc on both upsertWorkspaceAssociation and upsertWorkspaceMember says "Uses INSERT OR REPLACE" but the statements use INSERT ... ON CONFLICT ... DO UPDATE. They're not equivalent — INSERT OR REPLACE deletes the old row (losing created_at unless re-supplied) and re-fires triggers/FKs, while the upsert preserves the original created_at. The JSDoc should match the chosen semantics so readers don't assume delete-then-insert behavior.

Also applies to: 849-850

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/state.ts` around lines 711 - 712, The JSDoc for
upsertWorkspaceAssociation and upsertWorkspaceMember incorrectly states "Uses
INSERT OR REPLACE" whereas the SQL uses "INSERT ... ON CONFLICT ... DO UPDATE";
update both docstrings to describe the actual ON CONFLICT DO UPDATE semantics
(i.e., the row is updated in-place, original created_at is preserved unless
explicitly overwritten, and triggers/foreign-key cascade behavior from
DELETE/REPLACE will not occur) so readers won't assume delete-then-insert
behavior.
src/plugin/worktree/workspace-create.ts-310-333 (1)

310-333: ⚠️ Potential issue | 🟡 Minor

isWorktreeHealthy can throw and abort the whole plan.

worktreeListDetailed(repoPath) at line 330 can throw (e.g., lock contention, transient I/O, corrupt metadata). That error propagates up to planRepoWorktreesorchestrateWorkspaceCreate, which has no per-repo catch around planning, so one flaky source repo aborts all planning before the collision pre-check even runs. For a health check, the correct failure mode is "treat as unhealthy → retry", not to throw.

🔧 Proposed fix
 	// (c) Is the path listed in `git worktree list`?
-	const entries = await worktreeListDetailed(repoPath)
-	const normalizedTarget = path.resolve(worktreePath)
-	return entries.some((entry) => path.resolve(entry.path) === normalizedTarget)
+	try {
+		const entries = await worktreeListDetailed(repoPath)
+		const normalizedTarget = path.resolve(worktreePath)
+		return entries.some((entry) => path.resolve(entry.path) === normalizedTarget)
+	} catch {
+		return false
+	}

As per coding guidelines: "Implement worktree health check: a worktree is healthy when dir exists + .git entry + git worktree list includes the path (FR-022). Otherwise it needs retry."

Also applies to: 370-396

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace-create.ts` around lines 310 - 333, The
health-check function isWorktreeHealthy currently lets errors from
worktreeListDetailed(repoPath) bubble up and abort the whole plan; wrap the call
to worktreeListDetailed in a try/catch (and likewise the second occurrence
around lines 370-396) so any thrown error is caught, optionally logged/debugged,
and treated as "unhealthy" by returning false instead of rethrowing; keep the
existing directory and .git checks and the normalized path comparison intact,
but ensure worktreeListDetailed failures result in false to enable retries
rather than aborting planRepoWorktrees/orchestrateWorkspaceCreate.
src/plugin/worktree/workspace-create.ts-789-799 (1)

789-799: ⚠️ Potential issue | 🟡 Minor

Empty-string branch fallback will blow up at persistence.

MemberOutcome.branch here defaults to "" when plans.find misses. Downstream, persistWorkspaceState feeds this into upsertWorkspaceMember, whose Zod schema requires branch: z.string().min(1) (state.ts line 122) — so if this fallback ever triggers, state persistence throws mid-loop after some repos have already been written, leaving split-brain state across per-project DBs.

In practice plansToCheck ⊂ plans, so the fallback should be unreachable. Either remove the ?? fallbacks and assert, or look up the plan once and skip the outcome if missing, rather than silently synthesizing an invalid record.

🔧 Proposed fix
-		const preCheckFailureOutcomes: MemberOutcome[] = preCheck.preCheckFailures.map((failure) => {
-			const failedPlan = plans.find((p) => p.repo.name === failure.repoName)
-			return {
-				name: failure.repoName,
-				worktreePath: failedPlan?.worktreePath ?? path.join(target.workspacePath, failure.repoName),
-				branch: failedPlan?.branch ?? "",
-				status: "failed" as const,
-				error: failure.reason,
-			}
-		})
+		const preCheckFailureOutcomes: MemberOutcome[] = preCheck.preCheckFailures.flatMap((failure) => {
+			const failedPlan = plans.find((p) => p.repo.name === failure.repoName)
+			if (!failedPlan) {
+				// Invariant: pre-check only runs over planned repos.
+				log.warn(`[workspace] pre-check failure for unknown repo ${failure.repoName}; dropping`)
+				return []
+			}
+			return [{
+				name: failure.repoName,
+				worktreePath: failedPlan.worktreePath,
+				branch: failedPlan.branch,
+				status: "failed" as const,
+				error: failure.reason,
+			}]
+		})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace-create.ts` around lines 789 - 799, The code
builds MemberOutcome entries with branch fallback to an empty string which
violates the zod schema used by persistWorkspaceState/upsertWorkspaceMember;
instead, in workspace-create.ts change the preCheckFailureOutcomes construction
(the map that creates MemberOutcome) to lookup the matching plan once (from
plans) and either assert/throw if missing or skip creating an outcome for
missing plans — do not synthesize branch="" — ensure the code references the
same plan lookup used elsewhere (the failedPlan variable in this map) and only
produce MemberOutcome when failedPlan is present (or throw a clear error before
persisting) so branch always satisfies z.string().min(1) when passed to
persistWorkspaceState/upsertWorkspaceMember.
🧹 Nitpick comments (10)
.specify/templates/agent-file-template.md (1)

3-28: Add an explicit “do not overwrite curated root docs” warning in this template.

Given this template can drive regeneration, add a short guard note that root AGENTS.md/CLAUDE.md require explicit review before replacement.

Based on learnings "Do not auto-regenerate AGENTS.md or root CLAUDE.md from generic SpecKit agent-context template without explicit review — treat as curated guidance, not disposable generated summaries."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/templates/agent-file-template.md around lines 3 - 28, Add a short,
explicit guard note in the template between the existing <!-- MANUAL ADDITIONS
START --> and <!-- MANUAL ADDITIONS END --> markers warning that curated root
docs (e.g., AGENTS.md and CLAUDE.md) must not be auto-overwritten and require
explicit human review before replacement; reference the files by name in the
message and use clear phrasing like "DO NOT OVERWRITE / REGENERATE WITHOUT
EXPLICIT REVIEW" so downstream generators will preserve these root files unless
a maintainer confirms changes.
.specify/integrations/codex/scripts/update-context.ps1 (1)

8-15: Try/catch won't catch git rev-parse failures; code works by accident due to the -not $repoRoot guard.

Native command failures don't trigger PowerShell exceptions by default (even with $ErrorActionPreference = 'Stop', unless $PSNativeCommandUseErrorActionPreference = $true on PS 7.3+). When git is missing or outside a repo, the command runs but returns a non-zero exit code; the try/catch doesn't catch anything. The variable still gets $null from the failed command, so the subsequent -not $repoRoot guard handles it correctly—but the try/catch is misleading about what's actually happening.

Explicitly check $LASTEXITCODE instead:

♻️ Suggested refactor
-$repoRoot = try { git rev-parse --show-toplevel 2>$null } catch { $null }
+$repoRoot = (& git rev-parse --show-toplevel 2>$null)
+if ($LASTEXITCODE -ne 0) { $repoRoot = $null }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/integrations/codex/scripts/update-context.ps1 around lines 8 - 15,
The try/catch around the native git invocation is ineffective because native
commands don’t throw PowerShell exceptions; update the git rev-parse logic to
explicitly inspect $LASTEXITCODE after running git rev-parse --show-toplevel and
treat a non-zero exit code as a failure by setting $repoRoot to $null (so the
existing -not $repoRoot fallback runs). Ensure the surrounding variables
($scriptDir, $fsRoot, and the upward Search loop using Split-Path and Join-Path
to find '.specify') remain unchanged and only replace the try/catch with an
explicit $LASTEXITCODE check and fallback assignment.
.agents/skills/README.md (1)

1-26: Optional follow-up: scan docs for other absolute local paths.
This helps avoid similar portability issues elsewhere in shared markdown.

#!/bin/bash
# Find Windows-style absolute paths in markdown docs.
set -euo pipefail
rg -n --type=md '\b[A-Za-z]:[\\/]' .agents .specify specs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/skills/README.md around lines 1 - 26, The README contains
Windows-style absolute paths (e.g., the string
"E:/workspaces/opencode-worktree/..." in .agents/skills/README.md) which break
portability; search the repo for other Markdown files containing Windows
absolute paths (patterns like drive-letter: followed by slash/backslash) and
replace them with relative paths or repo-root placeholders (e.g., ./,
<repo-root>/, or documented environment variables), and update this README to
use the corrected placeholder paths; you can locate occurrences using the regex
pattern \b[A-Za-z]:[\\/], then apply replacements consistently across files such
as .agents/skills/README.md and any other .md files found.
.claude/commands/speckit.taskstoissues.md (1)

27-30: Consider guarding against destructive/duplicate issue creation.

The outline iterates tasks and calls the GitHub MCP issue_write tool but doesn't specify dedupe/idempotency behavior. Re-running the command on a feature whose tasks were previously exported would create a second set of issues. Consider adding an instruction to first list existing open issues (e.g., by a speckit:<feature> label or a title prefix derived from the task ID) and skip tasks that already have a matching issue, and/or require a dry-run preview before writes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/speckit.taskstoissues.md around lines 27 - 30, The
task-to-issue loop currently calls the GitHub MCP tool issue_write
unconditionally; update the flow to first query existing issues (using the
GitHub MCP list_issues or equivalent) filtered by a speckit:<feature> label or
by a title prefix derived from the task ID, compare returned issues to each task
and skip calling issue_write for tasks that already have a matching issue
(idempotency/dedupe), and add a dry-run flag that prints the would-be creations
instead of calling issue_write so users can preview changes before writes.
specs/001-workspace-worktree-orchestration/context/target-state.md (1)

16-18: Confirm detached-HEAD SHA length is documented.

PR summary also mentions the detached-HEAD SHA length was fixed to 12 chars. The target state only says "abbreviated SHA". Consider stating the exact length (12) so spec and code stay tied.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/001-workspace-worktree-orchestration/context/target-state.md` around
lines 16 - 18, The spec text for branch naming currently says "abbreviated SHA"
but doesn't state the exact length; update the sentence that describes
first-create checkout names (the line mentioning
dev_{base_branch}_{name}_{YYMMDD} and "detached HEAD uses abbreviated SHA") to
explicitly specify the detached-HEAD abbreviated SHA is 12 characters (e.g.,
"detached HEAD uses a 12-character abbreviated SHA") so the documentation
matches the implementation.
src/plugin/worktree/workspace-session.ts (1)

49-60: Transient SDK errors treated as stale session.

isSessionValid returns false for any thrown error, including transient network failures. Per FR-014, "stale" is the intent, but a network blip will cause an unnecessary refork and a warning log. Consider distinguishing "not found" (truly stale) from transient errors (retry or surface), or at least logging the underlying error before swallowing it for diagnosability.

Suggested tweak
-	} catch {
+	} catch (error) {
+		logWarn(
+			client,
+			"workspace-session",
+			`Session validation failed for ${sessionId}: ${error instanceof Error ? error.message : String(error)}`,
+		)
 		return false
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace-session.ts` around lines 49 - 60, The
isSessionValid function currently treats any thrown error as a stale session;
change it to detect SDK "not found" (HTTP 404) vs other errors by inspecting the
caught error (e.g., error.response?.status) from client.session.get: return
false only for 404, but for other transient errors log the full error (using
processLogger or similar) for diagnosability and rethrow or let the error
propagate so the caller can retry/handle it; update the catch in isSessionValid
to perform this distinction and include the error details in the log when not a
404.
src/plugin/worktree/workspace.ts (1)

329-362: workspaceLocks map grows unboundedly.

workspaceLocks is keyed by normalized workspace path and entries are never removed. For a long-lived OpenCode session that invokes /dev against many distinct workspace names over its lifetime, this accumulates one Mutex per unique name. Not urgent, but consider deleting the entry when the mutex's queue is empty at release time, or using a weak reference, to avoid a slow leak.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace.ts` around lines 329 - 362, The workspaceLocks
map grows unbounded because entries are never removed; update
acquireWorkspaceLock (and the release closure it returns) so the Mutex stored
under normalizedPath is removed from workspaceLocks when it becomes idle: after
calling mutex.release(), check that the mutex instance currently stored at
workspaceLocks.get(normalizedPath) === mutex and that the mutex has no
waiters/owners (i.e., is not locked and has an empty queue or waiting count) and
then delete the key; alternatively use a WeakMap/WeakRef if suitable—but prefer
the explicit delete-from-map approach with the identity check to avoid races.
.specify/scripts/powershell/create-new-feature.ps1 (1)

239-264: Consider: git checkout -b failure detection relies on $LASTEXITCODE, not exceptions.

PowerShell's native commands (including git) don't throw terminating errors, so the try/catch block at lines 241–248 will almost never catch anything — even when git checkout -b fails. You're already correctly falling back to checking $LASTEXITCODE and git branch --list, so this works, but the try/catch is misleading dead code. Consider dropping the try/catch entirely, or use $ErrorActionPreference = 'Stop' combined with $PSNativeCommandUseErrorActionPreference = $true (PS 7.3+) if you actually want native commands to raise.

Also, line 252 invokes git branch --list $branchName but does not also check remote branches; a branch that exists only on a remote will produce a misleading "Failed to create git branch" error instead of the "already exists" message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/scripts/powershell/create-new-feature.ps1 around lines 239 - 264,
The try/catch around the native git call is dead code because git won’t throw
terminating errors; remove the try/catch and rely on checking $LASTEXITCODE
after running git checkout -q -b $branchName (the existing $branchCreated
logic), or if you want native commands to throw enable error action for native
commands ($PSNativeCommandUseErrorActionPreference = $true) before invoking git;
additionally, improve the "already exists" check (where you call git branch
--list $branchName) to also inspect remote branches by checking either git
branch --all --list $branchName or using git ls-remote --heads origin
$branchName (or show-ref --verify refs/remotes/origin/$branchName) and treat a
remote match as "already exists" so the script reports the correct error instead
of the generic failure message.
src/plugin/worktree.ts (1)

623-795: Non-trivial duplication between interactive and headless paths.

The two branches duplicate: config load, target-path computation, branchExists check, the needsNewBranch && baseBranch vs createWorktree switch, sync (copyFiles / symlinkDirs), and postCreate hook execution (lines ~639–688 mirrored by ~747–789). Extracting a prepareWorktree(repoRoot, branch, baseBranch, config, log) helper that returns Result<{ worktreePath, mainWorktreePath }, string> would shrink each branch to the parts that actually diverge (launch context + fork + terminal). Defer if you prefer to land the feature first.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree.ts` around lines 623 - 795, Duplicate worktree setup
logic (config load, getWorktreePath, branchExists/createWorktree, copyFiles,
symlinkDirs, runHooks) should be extracted into a single helper to avoid
mirrored code; implement a prepareWorktree(repoRoot: string, branch: string,
baseBranch?: string, log: Logger) -> Promise<Result<{ worktreePath: string;
mainWorktreePath: string }, string>> that internally calls loadWorktreeConfig,
getWorktreePath, branchExists, createWorktree (or git worktree add when
needsNewBranch && baseBranch), copyFiles, symlinkDirs, runHooks and returns the
created worktreePath (and mainWorktreePath = repoRoot); replace the duplicated
blocks in both the interactive (uses launch/fork/terminal) and headless paths to
call prepareWorktree, check result.ok, and then continue with their divergent
behavior (fork/launch vs buildHeadlessResult).
src/plugin/worktree/state.ts (1)

189-202: projectId parameter is misleading on getOrInitDb.

The caller-supplied projectId is only used for the fast-path cache hit; on miss, initStateDb recomputes its own id from projectRoot and registers under that key. If the caller ever passes a projectId that doesn't match getProjectId(projectRoot), the cache lookup silently misses forever and every call falls through to initStateDb. Consider deriving projectId inside the function from projectRoot and dropping the parameter, or asserting equality to catch mismatches early.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/state.ts` around lines 189 - 202, The getOrInitDb
function's projectId parameter is misleading because initStateDb computes and
registers its own project id from projectRoot; change getOrInitDb to derive the
project id from projectRoot (using getProjectId(projectRoot) or the same helper
initStateDb uses) and use that derived id for the dbPool lookup and registration
instead of the caller-supplied projectId, or alternatively assert that the
provided projectId equals the derived id and throw/log on mismatch; update
references to getOrInitDb signature and ensure dbPool.get, initStateDb, and any
registration now use the single canonical project id.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 76ffd650-03ab-48b7-bd36-d840df101102

📥 Commits

Reviewing files that changed from the base of the PR and between 7888dcf and 91ef0c6.

⛔ Files ignored due to path filters (1)
  • .claude/scheduled_tasks.lock is excluded by !**/*.lock
📒 Files selected for processing (90)
  • .agents/CLAUDE.md
  • .agents/skills/CLAUDE.md
  • .agents/skills/README.md
  • .agents/skills/workspace-worktree-context/CLAUDE.md
  • .agents/skills/workspace-worktree-context/SKILL.md
  • .claude/CLAUDE.md
  • .claude/commands/CLAUDE.md
  • .claude/commands/speckit.analyze.md
  • .claude/commands/speckit.checklist.md
  • .claude/commands/speckit.clarify.md
  • .claude/commands/speckit.constitution.md
  • .claude/commands/speckit.implement.md
  • .claude/commands/speckit.plan.md
  • .claude/commands/speckit.specify.md
  • .claude/commands/speckit.tasks.md
  • .claude/commands/speckit.taskstoissues.md
  • .gitignore
  • .specify/CLAUDE.md
  • .specify/integrations/CLAUDE.md
  • .specify/integrations/codex/CLAUDE.md
  • .specify/integrations/codex/scripts/CLAUDE.md
  • .specify/integrations/codex/scripts/install-user-speckit.ps1
  • .specify/integrations/codex/scripts/update-context.ps1
  • .specify/memory/CLAUDE.md
  • .specify/memory/README.md
  • .specify/memory/active-feature-context.md
  • .specify/memory/cases/001-workspace-worktree-orchestration.md
  • .specify/memory/cases/CLAUDE.md
  • .specify/memory/cases/repo-path-resolution.md
  • .specify/memory/cases/workspace-metadata-authority.md
  • .specify/memory/constitution.md
  • .specify/memory/contexts/CLAUDE.md
  • .specify/memory/contexts/current-vs-target-state.md
  • .specify/memory/contexts/repository-context.md
  • .specify/memory/project-context.md
  • .specify/memory/project-memory.md
  • .specify/memory/skills/CLAUDE.md
  • .specify/memory/skills/project-skill-map.md
  • .specify/memory/skills/recommended-workflows.md
  • .specify/scripts/CLAUDE.md
  • .specify/scripts/powershell/CLAUDE.md
  • .specify/scripts/powershell/check-prerequisites.ps1
  • .specify/scripts/powershell/common.ps1
  • .specify/scripts/powershell/create-new-feature.ps1
  • .specify/scripts/powershell/setup-plan.ps1
  • .specify/scripts/powershell/update-agent-context.ps1
  • .specify/templates/CLAUDE.md
  • .specify/templates/agent-file-template.md
  • .specify/templates/checklist-template.md
  • .specify/templates/constitution-template.md
  • .specify/templates/plan-template.md
  • .specify/templates/spec-template.md
  • .specify/templates/tasks-template.md
  • AGENTS.md
  • CLAUDE.md
  • README.md
  • specs/001-workspace-worktree-orchestration/CLAUDE.md
  • specs/001-workspace-worktree-orchestration/cases/CLAUDE.md
  • specs/001-workspace-worktree-orchestration/cases/reconcile-cases.md
  • specs/001-workspace-worktree-orchestration/cases/repo-path-resolution.md
  • specs/001-workspace-worktree-orchestration/cases/workspace-metadata-authority.md
  • specs/001-workspace-worktree-orchestration/checklists/CLAUDE.md
  • specs/001-workspace-worktree-orchestration/checklists/archive/2026-04-14-requirements-pre-rewrite.md
  • specs/001-workspace-worktree-orchestration/checklists/archive/2026-04-15-requirements-iteration-1.md
  • specs/001-workspace-worktree-orchestration/checklists/contract.md
  • specs/001-workspace-worktree-orchestration/checklists/error.md
  • specs/001-workspace-worktree-orchestration/checklists/requirements.md
  • specs/001-workspace-worktree-orchestration/checklists/state.md
  • specs/001-workspace-worktree-orchestration/context/CLAUDE.md
  • specs/001-workspace-worktree-orchestration/context/current-state.md
  • specs/001-workspace-worktree-orchestration/context/go-live-runbook.md
  • specs/001-workspace-worktree-orchestration/context/pr-description.md
  • specs/001-workspace-worktree-orchestration/context/target-state.md
  • specs/001-workspace-worktree-orchestration/plan.md
  • specs/001-workspace-worktree-orchestration/spec.md
  • specs/001-workspace-worktree-orchestration/tasks.md
  • specs/CLAUDE.md
  • src/CLAUDE.md
  • src/plugin/CLAUDE.md
  • src/plugin/kdco-primitives/CLAUDE.md
  • src/plugin/worktree.ts
  • src/plugin/worktree/CLAUDE.md
  • src/plugin/worktree/config.ts
  • src/plugin/worktree/dev-command.ts
  • src/plugin/worktree/git.ts
  • src/plugin/worktree/state.ts
  • src/plugin/worktree/sync.ts
  • src/plugin/worktree/workspace-create.ts
  • src/plugin/worktree/workspace-session.ts
  • src/plugin/worktree/workspace.ts

Comment thread .specify/integrations/codex/scripts/install-user-speckit.ps1
Comment thread .specify/memory/active-feature-context.md Outdated
Comment thread .specify/memory/constitution.md Outdated
Comment thread .specify/memory/project-memory.md Outdated
Comment thread .specify/scripts/powershell/check-prerequisites.ps1 Outdated
Comment thread specs/001-workspace-worktree-orchestration/plan.md
Comment thread src/plugin/worktree/state.ts
Comment thread src/plugin/worktree/sync.ts
Comment thread src/plugin/worktree/workspace-create.ts
Addresses 11 of 16 unresolved review threads from PR #1 round 1:

src/plugin/worktree/state.ts (thread 13, coderabbit):
  - SIGINT/SIGTERM cleanup now re-raises the signal after pool close,
    so the process actually terminates; without this Ctrl+C could hang
    if anything else kept the event loop alive.

src/plugin/worktree/workspace-create.ts (thread 15, coderabbit):
  - existingMembers loader now fails fast on per-project DB init error
    instead of swallowing it; the previous behaviour could misclassify
    a healthy worktree as an orphan and `rm -rf` it, destroying
    uncommitted work.

Stale documentation synced with 2026-04-17 spec (threads 4, 5, 6, 10, 11, 12):
  - .specify/memory/active-feature-context.md: branch naming (no /→-,
    SHA[:12]), FR-009 dual-path, FR-024 slash mechanism, FR-019 error
    MUST.
  - .specify/memory/project-memory.md: Current Shipped Behavior now
    includes worktree_workspace_create + /dev slash + workspace tables.
  - .specify/memory/constitution.md: Repository Constraints reflect
    multi-repo workspace runtime.
  - specs/.../cases/reconcile-cases.md: Branch Name Computation — no
    substitution, SHA[:12].
  - specs/.../plan.md: Module Contracts — PreCheckOutcome +
    dual-path checkBranchCollisions signature.
  - specs/.../context/go-live-runbook.md: Smoke setup now forces
    `git init -b main` + inline identity so repoA/repoB deterministically
    land on main without depending on global git config.

.specify/scripts/powershell/ (threads 3, 7, 8):
  - check-prerequisites.ps1: branch validation moved after -PathsOnly
    early return (path-only mode must skip prerequisite validation).
  - setup-plan.ps1: human-readable status routed through Write-Verbose
    when -Json is set, so JSON stdout stays parseable.
  - install-user-speckit.ps1: prefer $HOME (cross-platform) and fall
    back to $env:USERPROFILE; throw if both are missing.

Deferred (will reply in PR comments):
  - sync.ts bash -c on Windows (thread 0) — platform-dependent hook
    execution, deferred to platform-abstraction follow-up.
  - workspace-create.ts error-string matching (thread 1) — common git
    pattern; risk of localisation accepted.
  - workspace.ts RESERVED_COMMAND_NAMES hardcoded (thread 2) —
    documented as known limitation L2; future dynamic query deferred.
  - update-agent-context.ps1 -replace escape (thread 9) — replacement
    of [Environment]::NewLine works as expected; no fix needed.
  - sync.ts Bun.spawnSync discussion (thread 14) — reviewed; current
    async handling is correct in context.
@NeverMore93
Copy link
Copy Markdown
Owner Author

Round-1 review addressed in 9ae2e34.

Fixed (11): state.ts SIGINT (thread 13), workspace-create.ts DB fail-fast (15), 3 stale memory files (4/5/6), reconcile-cases.md branch naming (10), plan.md signature (12), go-live-runbook main branch force (11), check-prerequisites.ps1 PathsOnly order (7), setup-plan.ps1 -Json output (8), install-user-speckit.ps1 HOME fallback (3).

Deferred with rationale (5):

  • Thread 0 (sync.ts bash -c Windows): hook execution is platform-dependent by design — users supplying bash hooks accept the bash dependency. Defer to a platform-abstraction follow-up.
  • Thread 1 (workspace-create.ts error-string matching for already locked/already checked out): common git pattern across many tools; localisation risk accepted for MVP.
  • Thread 2 (workspace.ts RESERVED_COMMAND_NAMES hardcoded): documented as known limitation L2 in PR description; dynamic client.command.list() query deferred to a future refactor.
  • Thread 9 (update-agent-context.ps1 -replace with [Environment]::NewLine): the replacement value is a string, not a regex escape sequence; PowerShell handles it correctly.
  • Thread 14 (sync.ts Bun.spawnSync vs Bun.spawn async): current blocking use within runHooks is intentional — hooks block the per-repo pipeline until complete (with timeout).

Retriggering reviewers to confirm clean pass.

@NeverMore93
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@NeverMore93
Copy link
Copy Markdown
Owner Author

@claude review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 19, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…spawn

Addresses thread 5 (coderabbitai, sync.ts:193) from PR #1 round-1
review. Bun.spawnSync blocks the entire Bun event loop until the
child process exits; for long-running hooks like `pnpm install`
(typical 20-60s), this stalls every other async operation in the
plugin — including the FR-023 parallel-phase hooks running for
sibling repos, session-event delivery, and incoming SDK calls.

Switch to async Bun.spawn + `await proc.exited`. Behaviour is
preserved (same error classification, same stderr capture, same
withTimeout wrapping) but the event loop stays responsive during
hook execution, which is essential for the workspace-level
parallel pipeline.

Static verification: `bun build --target=bun --external '*'` on
sync.ts bundles cleanly (17ms, 3.68 KB).
@NeverMore93
Copy link
Copy Markdown
Owner Author

Round-2 update:

Thread 5 (sync.ts Bun.spawnSync): Fixed in 4a1d213 — switched runHooks to async Bun.spawn + await proc.exited. The event loop now stays responsive during long hooks (critical for FR-023 parallel-phase execution across sibling repos). Agreed this was a real blocker, not just a style point.

Thread 4 (go-live-runbook main branch): Already addressed in round-1 (9ae2e34 lines 24-26 of go-live-runbook.md) — uses git init -b main + inline git -c user.email -c user.name commit. Your exact diff and mine differ only in identity values; behaviour is equivalent.

Threads 0, 1, 2, 3 (gemini medium × 3 + coderabbit PS -replace): Rationale given in round-1 wrap-up comment — all explained, no code change intended for this PR. Resolving as addressed.

All 6 remaining unresolved threads are being marked resolved now.

@NeverMore93
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (1)
.specify/memory/project-memory.md (1)

27-29: Polish repeated sentence openings in Context Discipline.

Three consecutive bullets start with “specs/<feature>/... stores”, which reads repetitive. Consider varying one or two openings for readability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/memory/project-memory.md around lines 27 - 29, The three bullets in
the "Context Discipline" section are repetitive because they all start with
"specs/<feature>/... stores"; update one or two of those openings to vary
phrasing for readability (e.g., change one to "Contains" or "Holds" and another
to "Used for" or "References") while keeping the same paths and meanings
(specs/<feature>/context/, specs/<feature>/cases/, etc.); ensure wording
preserves the original intent and parallel structure across the list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.specify/integrations/codex/scripts/install-user-speckit.ps1:
- Around line 49-66: The parser currently allows an empty or whitespace-only
$body to pass and still returns the hashtable (Description/Body), producing
invalid SKILL.md; add a guard after extracting $body (and before returning) that
checks for non-whitespace content (e.g., trim and test length or regex for \S)
and throw a clear error referencing $Path when the body is empty so malformed
command files fail fast; ensure this check sits alongside the existing
$descriptionMatch validation and prevents returning the hashtable if $body is
blank.
- Around line 74-85: The New-Item calls that create $DestRoot and per-skill
directories use the -Path parameter which treats glob characters in user-derived
$DestRoot and $skillDir as wildcards; change these New-Item invocations to use
-LiteralPath instead (i.e., replace New-Item -ItemType Directory -Path $DestRoot
and New-Item -ItemType Directory -Path $skillDir with -LiteralPath) so literal
directory names are created safely and consistently with the existing uses of
-LiteralPath elsewhere in this script (references: $DestRoot, $skillDir,
New-Item).

In @.specify/scripts/powershell/setup-plan.ps1:
- Around line 27-29: Capture the output of Test-FeatureBranch into a variable
first and test explicitly for a false result instead of relying on implicit
boolean coercion; e.g. call Test-FeatureBranch and assign its output to
$testResult (using the same parameters $paths.CURRENT_BRANCH and
$paths.HAS_GIT), then check if ($testResult -eq $false) { exit 1 } (or use if
($testResult -contains $false) { exit 1 }) so any Write-Output strings returned
by Test-FeatureBranch don’t force the if-condition to true and the script
correctly exits on invalid feature branches.

In `@specs/001-workspace-worktree-orchestration/plan.md`:
- Around line 217-248: The fenced ASCII diagrams and example fences in
specs/001-workspace-worktree-orchestration/plan.md lack language markers and
trigger markdownlint MD040; update each unlabeled triple-backtick block
(including the main flow diagram that documents validateWorkspaceName,
detectRepos, resolveWorkspaceTarget, acquireWorkspaceLock,
executeWorktreeCreation, resolveWorkspaceSession, and worktree_create steps) to
use ```text, and likewise add ```text to the other example fences around the
"(existing workspace)" and "SDK calls worktree_create(...)" snippets (also at
the other ranges noted: ~252-271 and ~275-282) so all flow diagrams and ASCII
examples are labeled.

In `@src/plugin/worktree/state.ts`:
- Around line 107-115: The workspaceAssociationSchema currently allows any
string for sessionDisposition; change it to a Zod enum (or z.union) that permits
only "forked" or "reused" (and keep nullable if DB allows null) so persisted DB
rows are constrained; update any related validation points (e.g., the update
handler that writes sessionDisposition — referenced by the same module's update
function/validation at the other occurrence around lines 802-808) to use the
tightened schema/type so callers and DB writes reject/convert invalid values
before persisting.

In `@src/plugin/worktree/workspace-create.ts`:
- Around line 409-421: When resolveBaseBranch(repo.path) returns a failure, do
not compute or plan a real branch with computeBranchName("unknown", ...);
instead record a per-repo failure Result and skip adding a create/retry plan.
Update the planning logic in workspace-create.ts (around resolveBaseBranch,
computeBranchName, and the plans array) to push a {ok:false, error: ...} result
(or add a plans entry with action: "failed" and an error field) for that repo
and continue, ensuring downstream execution only sees {ok:true, value} plans for
mutations; follow the Result<T,E> pattern used elsewhere so fallible operations
return {ok:false, error} rather than producing a mutable plan that will trigger
a real git worktree add.
- Around line 821-836: The current logic only checks the first repo's DB for a
workspace association (using target.repos[0], getOrInitDb and
getWorkspaceAssociation) which can cause duplicate workspace sessions; update
the code that resolves storedSessionId to iterate over all target.repos, calling
getOrInitDb(projectId, path, client) and getWorkspaceAssociation(db, validName,
target.workspacePath) for each repo until you find an assoc.sessionId, set
storedSessionId to that value and break; on per-repo errors continue to the next
repo (non-fatal) so one successful association prevents forking a new workspace
session and ensures exactly one workspace-level session is used.
- Around line 260-263: worktreeListDetailed currently hides git failures by
returning []; change its signature to return Result<WorktreeEntry[], string> and
propagate errors as Result.err; then update callers: in workspace-create
pre-check (where worktreeListDetailed is awaited to populate worktreeCache)
check the returned result.ok — if false, set this repo's status="failed" and
skip adding it to processing (exclude from subsequent steps); and in the health
check (the function that tests target collision) check result.ok and return
false on failure, otherwise use result.value as the entries to perform the path
comparison.

---

Nitpick comments:
In @.specify/memory/project-memory.md:
- Around line 27-29: The three bullets in the "Context Discipline" section are
repetitive because they all start with "specs/<feature>/... stores"; update one
or two of those openings to vary phrasing for readability (e.g., change one to
"Contains" or "Holds" and another to "Used for" or "References") while keeping
the same paths and meanings (specs/<feature>/context/, specs/<feature>/cases/,
etc.); ensure wording preserves the original intent and parallel structure
across the list.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46fce856-b5c1-4c72-b222-97007cecacf6

📥 Commits

Reviewing files that changed from the base of the PR and between 91ef0c6 and 4a1d213.

📒 Files selected for processing (12)
  • .specify/integrations/codex/scripts/install-user-speckit.ps1
  • .specify/memory/active-feature-context.md
  • .specify/memory/constitution.md
  • .specify/memory/project-memory.md
  • .specify/scripts/powershell/check-prerequisites.ps1
  • .specify/scripts/powershell/setup-plan.ps1
  • specs/001-workspace-worktree-orchestration/cases/reconcile-cases.md
  • specs/001-workspace-worktree-orchestration/context/go-live-runbook.md
  • specs/001-workspace-worktree-orchestration/plan.md
  • src/plugin/worktree/state.ts
  • src/plugin/worktree/sync.ts
  • src/plugin/worktree/workspace-create.ts
✅ Files skipped from review due to trivial changes (1)
  • specs/001-workspace-worktree-orchestration/context/go-live-runbook.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/plugin/worktree/sync.ts
  • .specify/memory/constitution.md
  • .specify/memory/active-feature-context.md

Comment thread .specify/integrations/codex/scripts/install-user-speckit.ps1
Comment thread .specify/integrations/codex/scripts/install-user-speckit.ps1 Outdated
Comment thread .specify/scripts/powershell/setup-plan.ps1
Comment thread specs/001-workspace-worktree-orchestration/plan.md Outdated
Comment thread src/plugin/worktree/state.ts
Comment thread src/plugin/worktree/workspace-create.ts Outdated
Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts
Critical fixes:
- common.ps1 Test-FeatureBranch: switch to Write-Error to avoid
  pipeline pollution that silently swallowed branch validation
  failures in `if (-not (Test-FeatureBranch ...))` callers.

Major fixes:
- workspace-create.ts planRepoWorktrees: introduce PlanningOutcome
  returning { plans, planningFailures }. Stop fabricating
  `dev_unknown_*` branches when `git rev-parse HEAD` fails; emit a
  per-repo planning failure that flows into MemberOutcome as
  `status="failed"`, matching the FR-009 dual-path pattern.
- workspace-create.ts orchestrator: scan every member repo's DB for
  a stored workspace session, not just repos[0]. Preserves FR-013's
  "exactly one workspace-level session" invariant when a newly-added
  repo appears first in detection order.
- state.ts workspaceAssociationSchema: tighten sessionDisposition
  from z.string() to z.enum(["forked","reused"]) to match the
  SessionDisposition TypeScript union.

Minor fixes:
- install-user-speckit.ps1: use -LiteralPath on New-Item to handle
  skill-name chars PowerShell treats as wildcards; skip source files
  with empty body instead of writing an unusable frontmatter-only
  SKILL.md.
- plan.md: add `text` language labels to the three integration-flow
  diagram fences for markdownlint MD040 compliance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (1)
src/plugin/worktree/state.ts (1)

107-115: ⚠️ Potential issue | 🟠 Major

Validate sessionDisposition on the update path too.

workspaceAssociationSchema is now constrained, but updateWorkspaceSession still accepts any non-empty string and persists it directly. That can write a value that later fails DB-row validation. As per coding guidelines, **/*.{ts,tsx}: Validate all external inputs (branch names, config, DB rows) with Zod schemas at the boundary; internal code trusts validated types.

🛡️ Proposed validation tightening
+const workspaceSessionDispositionSchema = z.enum(["forked", "reused"])
+
 const workspaceAssociationSchema = z.object({
 	name: z.string().min(1),
 	workspacePath: z.string().min(1),
 	sessionId: z.string().nullable(),
-	sessionDisposition: z.enum(["forked", "reused"]).nullable(),
+	sessionDisposition: workspaceSessionDispositionSchema.nullable(),
 	sourceCwd: z.string().min(1),
 	createdAt: z.string().min(1),
 	updatedAt: z.string().min(1),
 })
@@
 export function updateWorkspaceSession(
 	db: Database,
 	name: string,
 	workspacePath: string,
 	sessionId: string,
-	disposition: string,
+	disposition: z.infer<typeof workspaceSessionDispositionSchema>,
 ): void {
 	if (!name || !workspacePath || !sessionId || !disposition) {
 		throw new Error("updateWorkspaceSession requires all parameters to be non-empty")
 	}
 
+	const parsedDisposition = workspaceSessionDispositionSchema.parse(disposition)
 	const now = new Date().toISOString()
@@
 	stmt.run({
 		$name: name,
 		$workspacePath: workspacePath,
 		$sessionId: sessionId,
-		$disposition: disposition,
+		$disposition: parsedDisposition,
 		$updatedAt: now,
 	})
 }

Also applies to: 802-829

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/state.ts` around lines 107 - 115, The update path accepts
any non-empty string for sessionDisposition and can persist invalid values;
update the updateWorkspaceSession flow to validate incoming update data with the
same Zod constraints used by workspaceAssociationSchema (specifically ensure
sessionDisposition is z.enum(["forked","reused"]).nullable()), e.g.
parse/validate the update payload before persisting and only write the validated
value (or reject/throw on parse failure) so the DB row validations match the
runtime types.
🧹 Nitpick comments (1)
.specify/integrations/codex/scripts/install-user-speckit.ps1 (1)

34-39: Use approved PowerShell verbs for functions.

Quote-Yaml and Parse-CommandFile use unapproved verbs (PSUseApprovedVerbs). Rename to approved verbs to silence the analyzer and follow PS conventions, e.g. ConvertTo-YamlString and Read-CommandFile (or ConvertFrom-CommandFile).

♻️ Proposed rename
-function Quote-Yaml {
+function ConvertTo-YamlString {
     param([Parameter(Mandatory = $true)][string]$Value)
     return "'" + ($Value -replace "'", "''") + "'"
 }

-function Parse-CommandFile {
+function Read-CommandFile {
     param([Parameter(Mandatory = $true)][string]$Path)

Update call sites at lines 80, 97–102 accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/integrations/codex/scripts/install-user-speckit.ps1 around lines 34
- 39, Rename the unapproved PowerShell verbs: change function Quote-Yaml to
ConvertTo-YamlString and change Parse-CommandFile to Read-CommandFile (or
ConvertFrom-CommandFile if you prefer that naming convention); keep their
parameter lists and behavior unchanged, update every call site that invokes
Quote-Yaml and Parse-CommandFile (including the calls referenced in the diff) to
use the new names, and ensure any references in help/comments or exports are
updated to match the new function names so the PSUseApprovedVerbs analyzer is
satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.specify/scripts/powershell/common.ps1:
- Around line 181-201: The status helpers Test-FileExists and Test-DirHasFiles
currently write "✓/✗" with Write-Output which pollutes the success stream and
can make callers see a truthy array; change those status prints to Write-Host so
they go to the host stream only, and update Test-Path and Get-ChildItem calls to
use -LiteralPath instead of -Path to correctly handle paths containing wildcard
characters (also keep existing -ErrorAction SilentlyContinue on Get-ChildItem).
- Around line 71-78: The repo-derived path checks and directory enumerations
(e.g., Test-Path $specsDir, Get-ChildItem -Path $specsDir -Directory and the
template-resolution block that builds/reads templates under templatesDir) are
vulnerable to wildcard interpretation for repo names with characters like [ ].
Change all path operations that use user/repo-derived values to their literal
equivalents: use Test-Path -LiteralPath $specsDir, Get-ChildItem -LiteralPath
$specsDir -Directory (and any other
Get-ChildItem/Get-Item/Resolve-Path/Join-Path usages that later access
templates) and likewise use -LiteralPath in the template resolution code paths
so lookups don’t treat bracketed names as wildcards. Ensure Join-Path remains
for concatenation but feed results into the -LiteralPath parameters for
existence checks and enumerations.

In `@src/plugin/worktree/workspace-create.ts`:
- Around line 804-810: planningFailureOutcomes currently builds entries with
branch: "" which violates workspaceMemberSchema and causes persistWorkspaceState
to throw; update the persistence step so that branchless planning failures are
excluded from any data passed to persistWorkspaceState (keep
planningFailureOutcomes as response-only). Specifically, before calling
persistWorkspaceState (and anywhere that writes members), filter out entries
where branch === "" or otherwise only include members that satisfy
workspaceMemberSchema; leave the original planningFailureOutcomes for the
API/response but do not persist them. Apply the same change to the other
affected block range (the similar logic around lines 847–905).
- Around line 415-421: The current logic marks any existing directory at
worktreePath as an orphan and later deletes it; instead first verify the
directory is an adoptable Git worktree before treating it as orphan. Replace the
simple stat/isDirectory check around worktreePath and the later orphan-removal
branch (the blocks using isOrphan) with a Git-sanity check: confirm presence of
Git metadata (e.g., a .git file/dir or run git -C worktreePath rev-parse
--git-dir) and only set isOrphan = true if that check succeeds; if the path
exists but is not a valid/adoptable Git worktree, do not delete it—log and
abort/return an error. Apply the same fix to the second similar block mentioned
(the other use of isOrphan).
- Around line 652-673: The two upserts (upsertWorkspaceAssociation and
upsertWorkspaceMember) can partially succeed leaving inconsistent state; change
the code after getOrInitDb(repo.projectId, repo.path, client) to execute both
upserts inside a single database transaction (e.g., db.transaction(...) or the
project's equivalent transaction API) so that either both succeed or both are
rolled back on error; use the db returned by getOrInitDb, call the transaction
wrapper and perform upsertWorkspaceAssociation(...) then
upsertWorkspaceMember(...), rethrow errors to let the transaction rollback and
preserve existing error handling if the DB implementation lacks transactions,
emulate atomicity via a single upsert operation or a batch/atomic write
supported by that DB.

---

Duplicate comments:
In `@src/plugin/worktree/state.ts`:
- Around line 107-115: The update path accepts any non-empty string for
sessionDisposition and can persist invalid values; update the
updateWorkspaceSession flow to validate incoming update data with the same Zod
constraints used by workspaceAssociationSchema (specifically ensure
sessionDisposition is z.enum(["forked","reused"]).nullable()), e.g.
parse/validate the update payload before persisting and only write the validated
value (or reject/throw on parse failure) so the DB row validations match the
runtime types.

---

Nitpick comments:
In @.specify/integrations/codex/scripts/install-user-speckit.ps1:
- Around line 34-39: Rename the unapproved PowerShell verbs: change function
Quote-Yaml to ConvertTo-YamlString and change Parse-CommandFile to
Read-CommandFile (or ConvertFrom-CommandFile if you prefer that naming
convention); keep their parameter lists and behavior unchanged, update every
call site that invokes Quote-Yaml and Parse-CommandFile (including the calls
referenced in the diff) to use the new names, and ensure any references in
help/comments or exports are updated to match the new function names so the
PSUseApprovedVerbs analyzer is satisfied.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a15585fb-b04e-44f5-b93e-989007d70dac

📥 Commits

Reviewing files that changed from the base of the PR and between 4a1d213 and 063f103.

📒 Files selected for processing (5)
  • .specify/integrations/codex/scripts/install-user-speckit.ps1
  • .specify/scripts/powershell/common.ps1
  • specs/001-workspace-worktree-orchestration/plan.md
  • src/plugin/worktree/state.ts
  • src/plugin/worktree/workspace-create.ts
✅ Files skipped from review due to trivial changes (1)
  • specs/001-workspace-worktree-orchestration/plan.md

Comment thread .specify/scripts/powershell/common.ps1 Outdated
Comment thread .specify/scripts/powershell/common.ps1
Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts Outdated
Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts
wentaoyuan and others added 2 commits April 21, 2026 02:01
CodeRabbit identified that worktreeListDetailed silently returned []
on `git worktree list` failure, making the try/catch in
checkBranchCollisions dead code. The pre-check would then pass as
"collision-free" despite never having queried git — violating FR-009
case (b), which requires marking the repo status="failed" and
excluding it from subsequent mutation.

Changes:
- git.ts: worktreeListDetailed now returns Result<WorktreeEntry[],
  string>. Empty array on parse success, Result.err(gitError) on git
  failure.
- workspace-create.ts checkBranchCollisions: replace try/catch with
  `if (!result.ok)` branch; cache map now stores WorktreeEntry[]
  directly.
- workspace-create.ts isWorktreeHealthy: treat git failure as
  unhealthy so reconcile re-runs `git worktree add` (which will
  surface the underlying git error through normal error paths).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CRITICAL fix:
- workspace-create.ts planRepoWorktrees: refuse to delete arbitrary
  directories during "orphan recovery". The previous pattern marked ANY
  existing <workspace>/<repo> directory as deletable, which could wipe
  user files or a retained worktree with uncommitted changes. Now: only
  adopt the path if it has a .git entry AND appears in `git worktree
  list` for the repo (reuse the existing branch in that case). Anything
  else surfaces as a planning failure — never a destructive mutation.
- workspace-create.ts executeSingleRepo: drop the now-unreachable
  `rm -rf worktreePath` orphan-removal step and the `rm` import, so
  future changes cannot accidentally reintroduce the dangerous path.

Major fixes:
- workspace-create.ts orchestrator: split outcomes into
  `persistableOutcomes` (execution + pre-check failures, all with
  non-empty branch names) and response-only `planningFailureOutcomes`
  (branch = ""). Pass only `persistableOutcomes` to
  `persistWorkspaceState`. Previously, a single HEAD-unresolvable repo
  would make `workspaceMemberSchema` (branch.min(1)) throw inside
  persistence, converting a per-repo failure into a whole-command
  failure that left successful repos unrecorded.
- workspace-create.ts persistWorkspaceState: wrap per-repo
  `upsertWorkspaceAssociation` + `upsertWorkspaceMember` in a
  `db.transaction()`. A partial persistence was leaving a session
  binding without a matching member row, which the next reconcile
  classified as an orphan and could (pre-Critical fix) trigger
  destructive recovery. Transaction ensures both succeed or both
  rollback.
- common.ps1 Test-FileExists / Test-DirHasFiles: replace `Write-Output`
  status prints with `Write-Host` so callers' boolean predicates
  (`if (-not (Test-FileExists ...))`) see the `$true/$false` return
  value instead of a truthy string array. Same failure mode already
  guarded in Test-FeatureBranch (round-3).

Minor fixes:
- common.ps1 Get-CurrentBranch + Resolve-Template: use -LiteralPath on
  all Test-Path / Get-ChildItem / Get-Content calls so repo paths with
  wildcard characters (`[`, `*`) resolve correctly instead of being
  interpreted as wildcard patterns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.specify/scripts/powershell/common.ps1 (1)

37-44: Optional: log git failures in catch blocks.

PSScriptAnalyzer flags the empty catch at lines 42–44 (and the similar one at 65–67). The current code relies on $LASTEXITCODE, which is correct, but a short Write-Verbose "git rev-parse failed: $_" would aid debugging without changing behavior or polluting the success stream.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.specify/scripts/powershell/common.ps1 around lines 37 - 44, The empty catch
after the git rev-parse call leaves failures unlogged; inside the catch block
that follows the git rev-parse invocation (the block that currently swallows
errors after setting $result and checking $LASTEXITCODE) add a non-invasive
diagnostic log such as Write-Verbose "git rev-parse failed: $_" (including the
automatic $_ error variable), and do the same for the other analogous catch that
wraps the other git rev-parse call; keep behavior unchanged (do not write to
output stream or modify return logic), just emit the verbose message for
debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/worktree/workspace-create.ts`:
- Around line 410-419: The retry branch currently sets action:"retry" with
isOrphan:false when isWorktreeHealthy returns false, but executeSingleRepo calls
createWorktree directly and fails if the stale directory remains; fix by (1) in
the planning step where you push the retry plan (the block that uses existing
and sets isOrphan:false), call worktreeListDetailed for the repo/path to check
if the worktree is still listed and adoptable and, if so, change the plan to
action:"reuse" (or otherwise set isOrphan:true so executeSingleRepo will adopt),
and (2) if worktreeListDetailed shows the path is not listed and the directory
exists, mark the plan to remove the stale directory before invoking
createWorktree (so createWorktree is never called against an existing dir);
update isWorktreeHealthy-related logic and the planner code that produces plans
to choose between reuse/adopt vs delete-and-retry, and adjust
executeSingleRepo/createWorktree to respect an explicit "cleanupStalePath" or
"isOrphan" flag.

---

Nitpick comments:
In @.specify/scripts/powershell/common.ps1:
- Around line 37-44: The empty catch after the git rev-parse call leaves
failures unlogged; inside the catch block that follows the git rev-parse
invocation (the block that currently swallows errors after setting $result and
checking $LASTEXITCODE) add a non-invasive diagnostic log such as Write-Verbose
"git rev-parse failed: $_" (including the automatic $_ error variable), and do
the same for the other analogous catch that wraps the other git rev-parse call;
keep behavior unchanged (do not write to output stream or modify return logic),
just emit the verbose message for debugging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 54f9b656-14b9-4cf2-b850-68f40c17a493

📥 Commits

Reviewing files that changed from the base of the PR and between 063f103 and dd94092.

📒 Files selected for processing (3)
  • .specify/scripts/powershell/common.ps1
  • src/plugin/worktree/git.ts
  • src/plugin/worktree/workspace-create.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/plugin/worktree/git.ts

Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts Outdated
Major: unstick retry when the worktree directory still exists but is
unhealthy. With round-4's rm removed, `executeSingleRepo` called
`createWorktree` directly against a pre-existing stale directory,
hitting `'<path>' already exists` which does not match the prune-retry
heuristics (`already locked` / `already checked out`). Every reconcile
then re-failed for the same reason, and the adoption branch in planning
never re-entered because that only fires when there is no member record.

Fix restores the rm step in `executeSingleRepo` under a stricter
contract: `isOrphan: true` is now set ONLY in the existing-member +
unhealthy branch of `planRepoWorktrees`, where the persisted member
record proves the path is ours. The critical round-4 no-DB-record
branch still refuses to touch non-adoptable paths and emits a planning
failure instead — so the rm fires only on paths we provably own.

Minor: eliminate the dead `branch: failedPlan?.branch ?? ""` fallback.
`PreCheckFailure` now carries `branch` and `worktreePath` directly,
populated from the plan at push-time in `checkBranchCollisions`. A new
`PlanningFailure` interface (without branch/worktreePath) covers the
planning-phase case where no plan exists yet. The orchestrator drops
the `plans.find(...)` lookup and its defensive empty-string fallback,
so a future refactor that desynced plans and pre-check failures would
surface as a compile error instead of silently injecting `branch: ""`
into persistence and throwing inside `db.transaction`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/plugin/worktree/workspace-create.ts (1)

964-976: Silent catch on DB open during session-binding lookup can still force a duplicate fork.

Fail-fast was applied to the earlier existingMembers load (Line 874-888) for exactly the reason called out in the Step 5 comment — a transient DB error there would mis-classify members as orphans. This block has the same structural risk in reverse: if getOrInitDb throws transiently for the one repo that holds workspace_associations, we silently skip it, storedSessionId stays undefined, and resolveWorkspaceSession forks a second workspace-level session, violating FR-013's "exactly one workspace-level session" invariant.

At minimum, push a warning onto warnings (consistent with the git-fetch handler) so operators can see that a DB read failed before a new session was forked; ideally fail-fast here too, matching Step 5. A silent catch makes the duplicate-session failure mode invisible.

🔧 Surface the DB read failure
 		for (const repo of target.repos) {
 			try {
 				const db = await getOrInitDb(repo.projectId, repo.path, client)
 				const assoc = getWorkspaceAssociation(db, validName, target.workspacePath)
 				if (assoc?.sessionId) {
 					storedSessionId = assoc.sessionId
 					break
 				}
-			} catch {
-				// Non-fatal — skip this repo and keep scanning the others.
+			} catch (error) {
+				const message = error instanceof Error ? error.message : String(error)
+				warnings.push(
+					`Could not read workspace session binding from ${repo.name} ` +
+					`(projectId=${repo.projectId}): ${message}. A new session may be forked.`,
+				)
 			}
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/worktree/workspace-create.ts` around lines 964 - 976, The silent
catch around getOrInitDb/getWorkspaceAssociation can hide transient DB errors
and allow resolveWorkspaceSession to fork a duplicate workspace-level session;
update the loop that sets storedSessionId to surface failures by either (a)
rethrowing the error to fail-fast consistent with the existingMembers load
logic, or (b) push a descriptive warning onto the warnings array (matching the
git-fetch handler) that includes the repo identifiers and the caught error
before continuing; ensure you reference getOrInitDb, getWorkspaceAssociation,
storedSessionId and resolveWorkspaceSession so operators will see DB read
failures that could cause duplicate sessions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/worktree/workspace-create.ts`:
- Around line 482-507: The code currently adopts an on-disk worktree using
existingEntry.branch verbatim; change the logic in the block that handles
hasGitEntry && existingEntry to validate the branch before creating the "reuse"
plan: compute the expected workspace branch(s) via the same routine used
elsewhere (e.g., computeBranchName or the branch-name pattern
dev_*_{workspaceName}_*), and only push the reuse plan when existingEntry.branch
matches that computed pattern; otherwise add a planningFailures entry (similar
to the other failure path) and continue. Update references around
worktreeListDetailed, existingEntry.branch, plans.push and planningFailures so
adopted branches always conform to the dev_* naming invariant and downstream
code like checkBranchCollisions and computeBranchName remains consistent.
- Around line 259-316: plansToCheck currently excludes plans with action ===
"reuse", which lets orphan-adoption-generated reuse plans (created using
existingEntry.branch) bypass external-collision detection; either include those
reuse plans in the collision loop or run the same targeted collision check when
creating an adoption reuse plan. Fix by updating the selection logic that builds
plansToCheck to include reuse plans that originate from the orphan-adoption path
(or mark them with a flag) or, at the orphan-adoption site where you emit the
reuse plan (the code that uses existingEntry.branch), call
worktreeListDetailed(repoRoot) and perform the same branch/path collision tests
(the same checks done in the loop that inspects worktreeCache, compares
entry.branch to plan.branch and normalized paths) and push a PreCheckFailure or
collision entry when a conflicting external checkout is found.
- Around line 443-456: The code currently computes hasStaleDir by stat-ing the
newly computed worktreePath rather than the authoritative existing.worktreePath;
change the check to stat existing.worktreePath (use existing.worktreePath when
setting hasStaleDir and deciding isOrphan) so the stale-directory detection and
the isOrphan flag reflect the prior owned location, and ensure executeSingleRepo
will remove existing.worktreePath (not the recomputed path) when performing the
"retry" action; update references in this block that set hasStaleDir, isOrphan,
and any subsequent removal logic to use existing.worktreePath.
- Around line 419-457: The current planner marks a healthy worktree as "retry"
when existing.status === "failed", which leads to rm -rf of live worktrees;
change the planner in workspace-create.ts so that if
isWorktreeHealthy(existing.worktreePath, repo.path) returns true you set action
"reuse" (or a distinct "healthy-retry" flag) instead of "retry" (i.e., adjust
the existing block around existing/status/healthy and the plans.push call to
treat healthy worktrees as reuse), and also update executeSingleRepo to detect
the case (check isWorktreeHealthy(worktreePath, repo.path) before doing
branchExists/createWorktree and before any rm/cleanup) and short-circuit the
add/remove steps when action === "retry" but the worktree is healthy (isOrphan
=== false); reference functions/variables: isWorktreeHealthy, existing,
plans.push, action, isOrphan, worktreePath, executeSingleRepo, branchExists,
createWorktree.

---

Nitpick comments:
In `@src/plugin/worktree/workspace-create.ts`:
- Around line 964-976: The silent catch around
getOrInitDb/getWorkspaceAssociation can hide transient DB errors and allow
resolveWorkspaceSession to fork a duplicate workspace-level session; update the
loop that sets storedSessionId to surface failures by either (a) rethrowing the
error to fail-fast consistent with the existingMembers load logic, or (b) push a
descriptive warning onto the warnings array (matching the git-fetch handler)
that includes the repo identifiers and the caught error before continuing;
ensure you reference getOrInitDb, getWorkspaceAssociation, storedSessionId and
resolveWorkspaceSession so operators will see DB read failures that could cause
duplicate sessions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1691ddf3-66c0-4a26-9d2e-9fbbff368e8a

📥 Commits

Reviewing files that changed from the base of the PR and between dd94092 and 5deb9dd.

📒 Files selected for processing (1)
  • src/plugin/worktree/workspace-create.ts

Comment thread src/plugin/worktree/workspace-create.ts Outdated
Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts
Comment thread src/plugin/worktree/workspace-create.ts
CRITICAL: don't rm healthy worktrees during retry.
When a previous run succeeded at `git worktree add` but failed later at
sync or hooks, the stored member has `status="failed"` but the worktree
itself is intact — potentially holding user work-in-progress between
runs. Round-5's blanket `retry` path would rm that healthy worktree.

Planning now distinguishes three cases for existing members:
- healthy + status != failed → reuse
- healthy + status == failed → retry with isOrphan=false (new branch)
- unhealthy → retry with isOrphan = (stale dir exists)

executeSingleRepo gates the rm + `git worktree add` behind a runtime
`isWorktreeHealthy` re-check: if the worktree is already healthy at
execution time (defense-in-depth against planning/execution races), it
skips straight to sync + hooks without touching anything on disk.

Major: include reuse plans in pre-check.
The round-4 orphan-adoption path emits `action="reuse"` using
`existingEntry.branch` verbatim, which may be a branch checked out at an
EXTERNAL worktree (user's `main` in the source repo). Dropping reuse
plans from `plansToCheck` let those external collisions slip through,
bypassing FR-009 on the adoption path. Include all plans; the existing
inside-workspace filter handles self-matches.

Minor: adopted branch must match workspace naming scheme.
No-DB-record adoption was happy to accept ANY branch (`main`, a stale
branch from another workspace, a user's personal branch) and persist it
as this workspace's member branch. That silently broke FR-005's
`dev_*_{workspaceName}_{YYMMDD}` contract. Adoption now requires the
branch to match `^dev_.+_{workspaceName}_\d{6}$`; anything else surfaces
as a planning failure so the user can resolve the conflict manually.

Minor: use `existing.worktreePath` for stale-dir detection.
DB's `existing.worktreePath` is the authoritative "we own this" signal;
checking the recomputed `path.join(workspacePath, repo.name)` would
leak the old directory if the workspace was moved between runs. Stat
and rm both now target `existing.worktreePath`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NeverMore93
Copy link
Copy Markdown
Owner Author

(Re-posting to the correct PR — prior round summaries accidentally went to kdcokenny#1.)

Rounds 1–6 shipped on branch 001-workspace-worktree-orchestration:

  • r1 (9ae2e34): 11 threads — SIGINT re-raise, DB fail-fast, stale memory docs
  • r2 (4a1d213): sync.ts Bun.spawnSync → async Bun.spawn
  • r3 (063f103 + b32a93e): 8 threads — Test-FeatureBranch pipeline pollution, PlanningOutcome pattern, scan-all-DBs, sessionDisposition z.enum, LiteralPath + empty-body guard, MD040 fences, worktreeListDetailed→Result
  • r4 (dd94092): 5 threads — CRITICAL refuse arbitrary rm during orphan recovery, persistable vs response outcomes, db.transaction atomicity, Write-Host on PS predicates, -LiteralPath everywhere
  • r5 (5deb9dd): 2 threads — restored rm under isOrphan=true contract, PreCheckFailure/PlanningFailure type split
  • r6 (42cc8e5): 4 threads — CRITICAL healthy-retry cannot rm, include reuse plans in pre-check, adoption must match FR-005 naming, use existing.worktreePath for stale-dir check

All review threads resolved via GraphQL. Please re-scan the tip of the branch.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

@NeverMore93 Thanks for the detailed round-by-round summary — that's a comprehensive remediation trail across 6 rounds covering critical safety fixes (arbitrary rm guards, isOrphan contract, healthy-retry protections), structural improvements (type splits, db.transaction atomicity, Result-wrapped git ops), and async correctness (Bun.spawnSyncBun.spawn). Triggering an incremental review of the tip now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@NeverMore93
Copy link
Copy Markdown
Owner Author

Merge gate summary — ready for user decision

  • reviewThreads: 0 unresolved
  • CodeRabbit: last inline feedback at 21:10:30Z (round 5 on commit 5deb9dd); explicitly retriggered at 21:39:32Z and acknowledged with "Triggering an incremental review of the tip now"; ~8 minutes elapsed with no new findings on tip 42cc8e5
  • Required CI checks: none configured
  • Branch: 001-workspace-worktree-orchestration at 42cc8e5
  • Total remediation: 6 rounds, 30 threads addressed, including 3 Critical (arbitrary rm guard, healthy-retry protection, pipeline-pollution on predicate helpers)

Ship decisions (merge, tag, publish) are paused here per gh-ship-with-review — awaiting explicit user approval.

@NeverMore93
Copy link
Copy Markdown
Owner Author

@codex review

@NeverMore93 NeverMore93 merged commit ac1b7ab into main Apr 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant