Skip to content

feat(worktree): path/branch helpers + WorktreeAddAttachAsync (AB#3094, PR 1b1) - #310

Merged
PolyphonyRequiem merged 1 commit into
mainfrom
feature/worktree-helpers
May 11, 2026
Merged

feat(worktree): path/branch helpers + WorktreeAddAttachAsync (AB#3094, PR 1b1)#310
PolyphonyRequiem merged 1 commit into
mainfrom
feature/worktree-helpers

Conversation

@PolyphonyRequiem

Copy link
Copy Markdown
Owner

Summary

PR 1b1 of the AB#3085 worktree-model epic. Lays the pure-helper foundation that PRs 1b2 (worktree init-apex) and 1b3 (worktree create) will build on. No new verbs ship in this PR — surface area is intentionally small to keep review focused.

Closes AB#3094 (parent: AB#3085).

What changes

RunsRootResolver (new)

src/Polyphony/Infrastructure/Worktrees/RunsRootResolver.cs

Pure function: Resolve(commonDir) → (runsRoot, mainWorktreePath). Used by future write-verbs to derive {runs_root}/apex-{N}/ from git rev-parse --path-format=absolute --git-common-dir without env vars or assumptions about CWD.

  • Bare layout: <parent>/polyphony.gitruns_root = <parent>/polyphony-runs, main = <parent>/polyphony.
  • Non-bare layout: <repo_dir>/.gitruns_root = <parent>/<basename>-runs, main = <repo_dir>.
  • Always canonicalizes via Path.GetFullPath first (handles trailing slashes, mixed separators, .. segments).
  • Throws ArgumentException on null/empty input or paths without a parent.

BranchSlug (new)

src/Polyphony/Infrastructure/Worktrees/BranchSlug.cs

Pure parser: BranchSlug.TryParse(branch, out ParsedBranch?, out reason). Validates branch names against the canonical grammar from the polyphony-branch-model skill and returns the parsed root id so callers can cross-check against --apex.

Grammar enforced:

  • feature/{r}feature/3085
  • plan/{r}[-{item_id}]plan/3085, plan/3085-3094
  • mg/{r}_{mg_path}mg/3085_pg-foo, mg/3085_pg_bar
  • impl/{r}-{item_id}impl/3085-3094
  • evidence/{r}-{item_id}evidence/3085-3094

Hostile-input defenses (covered by tests):

  • Path traversal: rejects .., \, \0 anywhere in the branch.
  • Multiple slashes: rejects feature/foo/bar.
  • Positive-int parsing: rejects leading zeros, signs, whitespace, non-digits.
  • MG segment grammar: ^[a-z][a-z0-9-]{0,30}$ per segment; _ only separates the MG hierarchy.

IGitClient.WorktreeAddAttachAsync (new method)

src/Polyphony/Infrastructure/Processes/{IGitClient.cs, GitClient.cs}

Implements git worktree add {path} {branch} (no -b, no --detach) — the attach half of the create-or-attach matrix. Split from the existing WorktreeAddAsync so future verbs (init-apex, create) can discriminate stderr cleanly per-purpose.

  • Returns ProcessResult (does NOT throw on non-zero) — same pattern as WorktreeAddAsync.
  • Validates non-empty branch and path via ArgumentException.ThrowIfNullOrEmpty.
  • Argument order is <path> <branch> (matches git worktree add CLI; inverting silently changes git's semantics — covered by a regression test).

Why split this out

Without these helpers in their own PR, PRs 1b2 and 1b3 would each carry ~150 lines of pure-function code under the verb scaffolding, making the verb-level review noisy. Splitting also lets path-derivation and grammar parsing get focused unit-test coverage independent of CLI plumbing.

Tests

35 new tests added; all green:

File Count Coverage
tests/Polyphony.Tests/Infrastructure/Worktrees/RunsRootResolverTests.cs 7 Bare/non-bare convergence, canonicalization, drive-root rejection.
tests/Polyphony.Tests/Infrastructure/Worktrees/BranchSlugTests.cs 25 Each branch class accept/reject, path traversal, leading zeros, MG segment grammar, multi-_ MG paths.
tests/Polyphony.Tests/Infrastructure/Processes/GitClientTests.cs (appended) 3 Success arg-order, git-fail returns ProcessResult, empty-input throws.

Test stubs LockCommandsTests.FakeGitClient and PolyphonyStatePathsTests.StubGitClient updated for the new IGitClient method.

Full suite: 3252 passed, 4 skipped, 1 unrelated flaky parallel race (passes on isolated re-run).

Rubber-duck-validated decisions baked in

The design decisions come from a critique on the original combined PR 1b proposal:

  • init-apex does NOT refuse cwd-inside-main — the launcher must run from the main worktree during bootstrap.
  • No HEAD default for --ref — missing-branch-without-ref must surface as a ref_required envelope error in PR 1b3, not silently take HEAD.
  • BranchSlug.TryParse returns the parsed root id — PR 1b3 uses this to validate parsed.RootId == --apex (catches --apex 3085 --branch impl/9999-1234 cross-apex bugs).
  • branch_in_use detection uses git worktree list --porcelain in future verbs (NOT stderr parsing).
  • Path-exists checks win over branch-state checks in the create-or-attach priority order (documented in plan.md).
  • Real safety invariant is target-path canonicalization under runs_rootRunsRootResolver exists precisely so PRs 1b2/1b3 can derive paths themselves rather than trust user input.

What this PR does NOT do (deferred to 1b2/1b3)

  • No new CLI verbs registered.
  • No DI registration changes.
  • No PolyphonyJsonContext additions (no new result types yet).
  • No Models/*Result.cs records.
  • No workflow YAML changes.

PR 1b2 (worktree init-apex) and PR 1b3 (worktree create) will both consume these helpers and ship the user-facing surface area.

Validation

  • dotnet build — green
  • dotnet test — 3252 passed, 4 skipped (1 unrelated flaky test passes in isolation)
  • tests/lint-prose-children.ps1 — pass
  • tests/lint-jinja-resolver.ps1 — pass
  • tests/lint-version-drift.ps1 — pass
  • tests/lint-conductor-validate.ps1 — pass
  • artifacts/verb-output-schemas.json — no drift (no new verbs)

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…, PR 1b1)

Pure-helper foundation for the AB#3085 bare-repo + per-run-worktree epic.

No new verbs ship in this PR; PRs 1b2 (init-apex) and 1b3 (create) will

consume these helpers and add the user-facing surface area.

RunsRootResolver: pure function that derives (runs_root, main_worktree_path)

from git's --git-common-dir output. Bare and non-bare layouts converge on the

same '<basename>-runs' sibling-of-repo invariant. Always canonicalizes via

Path.GetFullPath first.

BranchSlug: pure parser for the canonical branch grammar (feature/plan/mg/

impl/evidence). Returns ParsedBranch.RootId so future verbs can validate

branch root matches --apex. Hostile-input defenses for path traversal,

leading zeros, multiple slashes, MG segment grammar.

WorktreeAddAttachAsync: 'git worktree add {path} {branch}' (no -b). Split

from WorktreeAddAsync so future verbs can discriminate stderr per-purpose.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PolyphonyRequiem
PolyphonyRequiem merged commit 5a38360 into main May 11, 2026
1 check passed
@PolyphonyRequiem
PolyphonyRequiem deleted the feature/worktree-helpers branch May 11, 2026 20:32
PolyphonyRequiem added a commit that referenced this pull request May 12, 2026
Adds `polyphony worktree init-apex --apex N`: bootstraps the per-apex
container `{runs_root}/apex-{N}/` and the apex's `feature/{N}` worktree
under `{runs_root}/apex-{N}/feature-{N}/` with a full create-or-attach /
idempotency / refusal matrix. Always roots `feature/{N}` from local `main`.

Part of AB#3085 (bare-repo + per-run-worktree epic). Depends on PR #310
(PR 1b1 helpers: RunsRootResolver, BranchSlug, WorktreeAddAttachAsync).

New files
- src/Polyphony/Infrastructure/Worktrees/PathBoundary.cs
  Boundary-aware `IsSameOrSubpath(parent, child)`. Prevents the
  `polyphony` vs `polyphony-runs` naive-prefix collision a
  `string.StartsWith` would miss; OS-aware case sensitivity.
- src/Polyphony/Models/WorktreeInitApexResult.cs
  Result record: required ApexId/Outcome, nullable ApexRoot/WorktreePath/
  Branch/Reason/Error. Doc-comment is the authoritative reason taxonomy.
- src/Polyphony/Commands/WorktreeCommands.InitApex.cs
  Verb implementation. Path-exists wins over branch-state. Race-tolerant
  idempotency on git failure (re-list + check expected branch).
- tests/Polyphony.Tests/Infrastructure/Worktrees/PathBoundaryTests.cs (12)
- tests/Polyphony.Tests/Commands/WorktreeCommandsInitApexTests.cs (21)

Modified
- src/Polyphony/PolyphonyJsonContext.cs  add WorktreeInitApexResult
- src/Polyphony/Commands/WorktreeCommands.cs  add InitApex to summary

Reason taxonomy
  invalid_apex, common_dir_unavailable, filesystem_failure, git_failure,
  path_exists_wrong_branch, path_exists_not_worktree, branch_in_use,
  remote_branch_exists.

Outcomes: created, attached, idempotent, failed.

Rubber-duck-validated decisions baked in (3 blocking + 5 non-blocking, all
adopted): nullable result fields; PathBoundary not StartsWith; remote-branch
refusal to prevent silent fork from main; RequiredInput.MissingInt sentinel
for --apex; PathExists = Directory||File; explicit filesystem_failure on
CreateDirectory exceptions; race-tolerant idempotency probe; FormatException
from ParsePorcelain mapped to git_failure.

Validation
- 33/33 new tests pass.
- Full suite green modulo documented flaky pair
  (PrCommandsMergePlanAdoTests.OpenPr_CompletesAndRecordsLedger,
  ManifestCommandsTests.ReadPlanGenerationSnapshot_Grandchild_ChainOrderingPreserved)
  which pass in isolation.
- artifacts/verb-output-schemas.json regenerated; disk and embedded resource
  match.
- lint-prose-children, lint-jinja-resolver, lint-version-drift,
  lint-conductor-validate all PASS locally.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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