Skip to content

feat(herdr): create worktrees under ~/Projects/worktrees and trust them in agy and Copilot - #2504

Merged
mlorentedev merged 3 commits into
mainfrom
feat/herdr-worktree-root
Oct 15, 2026
Merged

mlorentedev merged 3 commits into
mainfrom
feat/herdr-worktree-root

Conversation

@mlorentedev

@mlorentedev mlorentedev commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Refs #2503. The config half of "one worktree root": herdr creates its worktrees where every harness trusts them.

  • ai/herdr/config.toml: [worktrees] directory moves from ~/Projects/.worktrees to ~/Projects/worktrees. herdr's layout is fixed at <directory>/<repo>/<branch-slug> (only the root is configurable, per herdr 0.9.3's config reference), and a dot-prefixed root is excluded by agy (**/.worktrees/** in workspace.excludePatterns). The comment said "not in a hidden dir" while the value was hidden; it now says what it means.
  • ai/agy/settings.json trustedWorkspaces and ai/copilot/config.json trustedFolders gain {HOME}/Projects/worktrees/*/*. Both deploy by merge (union), so entries a tool added at runtime survive.
  • cli/internal/harness/model_pins.go: stripJSONComments is now string-aware. Its regex treated the /* inside a glob string as a comment opener; on main it already cut ai/agy/settings.json from "{HOME}/Projects/*" to the */ in "**/node_modules/**" and happened to leave valid JSON. One more glob made it invalid, and CI's TestEveryRepoRoutingPinResolvesInTheMap and TestEveryDeclaredPinRejectsADeadID failed on every OS (pin "agy-model": unexpected end of JSON input). A table test pins globs, URLs, an escaped quote, the three comment forms and an unterminated block comment. Lesson 431.
  • tests/herdr-config.bats: a new case derives <directory>/*/* from the herdr config and asserts both trust lists hold it, so the three files cannot drift apart until CLI-118: one worktree root for dotf, herdr and every harness #2503's spec renders them from one declared root.

Not in this PR, and why: dotf worktree add/done, the using-git-worktrees and handoff skills and AGENTS.md §9 still use the sibling <repo>-wt-<slug>, so two layouts coexist until #2503 lands. That half changes the worktree reaper and gate f (BUG-093/094 territory), migrates existing worktrees, and needs a spec; spec init refuses while 11 specs are active against a limit of 10.

Verification

  • bats tests/antigravity.bats tests/copilot-config.bats tests/herdr-config.bats tests/verify-setup.bats: 107 tests, 0 not ok.
  • HERDR_CONFIG_PATH=ai/herdr/config.toml herdr config check: config: ok.
  • Mutations of the new case: dropping the Copilot entry fails it ("copilot trustedFolders lacks {HOME}/Projects/worktrees//"); moving the herdr root to another directory fails it ("agy trustedWorkspaces lacks {HOME}/Projects/wt2//").
  • cd cli && go build ./... && go vet ./... && GOOS=windows go vet ./... && go test ./...: all ok. golangci-lint run (2.12.2, the pin, after cache clean): 0 issues.
  • Mutation: the previous regex stripper fails TestStripJSONCommentsLeavesStringsAlone (glob, whole-line and trailing cases) and TestEveryRepoRoutingPinResolvesInTheMap.
  • go run ./cmd/dotf deploy --dry-run from this worktree: would deploy for copilot-config, agy-settings and herdr, every other entry in sync.
  • To be measured on first launch after deploy: agy and copilot started in a herdr-created worktree under ~/Projects/worktrees/<repo>/<slug> open without a trust prompt. The one-level * the existing entries use is taken as a path-segment glob; that is the claim the launch checks.

SDD skip rationale

A config alignment plus the defect fix it exposed, whose cause was measured on this PR's CI and is pinned by a table test. The design work (one declared worktree root read by dotf worktree, the skills, AGENTS.md and every harness) is #2503 and gets its own spec.

Knowledge

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: mlorentedev/dotfiles/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7880bfc4-a76f-4cfe-8e50-07f13d7e4f27
📥 Commits

Reviewing files that changed from the base of the PR and between 11d3ec6 and 1040099.

📒 Files selected for processing: 10
  • ai/agy/settings.json
  • ai/copilot/config.json
  • ai/herdr/config.toml
  • cli/internal/harness/model_pins.go
  • cli/internal/harness/model_pins_test.go
  • docs/lessons/_index.md
  • docs/lessons/lesson-431-a-comment-stripper-that-ignores-strings-can-still-produce-valid-json.md
  • tests/antigravity.bats
  • tests/copilot-config.bats
  • tests/herdr-config.bats
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread ai/agy/settings.json
Comment on lines 17 to 21
"trustedWorkspaces": [
"{HOME}/Projects/*",
"{HOME}/Projects/Workspace/*"
"{HOME}/Projects/Workspace/*",
"{HOME}/Projects/worktrees/*/*"
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Harness compliance

Harness compliance (AGENTS.md / CLAUDE.md): the PR body and branch are in English, but the PR title and body state the change is partial and defer the ticket. Standing Order 4 (clean as you go, track-or-fix) requires the deferred half (dotf worktree add/done, skills, AGENTS.md §9 still on sibling layout) to be ticketed with root cause. The PR body names it but does not reference a ticket for the remainder, only #2503 which is the umbrella. REAL but low severity; confirm a follow-up issue exists for the deferred half.

Comment thread ai/copilot/config.json
Comment on lines 5 to +7
"{HOME}/Projects/Workspace",
"{HOME}/Projects/Workspace/*"
"{HOME}/Projects/Workspace/*",
"{HOME}/Projects/worktrees/*/*"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible Issue

The trustedFolders hunk is malformed JSON. The diff removes the closing "{HOME}/Projects/Workspace/*" line as shown in the old hunk, but the new hunk adds "{HOME}/Projects/Workspace/*" after "{HOME}/Projects/Workspace" with a trailing comma structure that is valid only because the new array is present. Verify the file parses: jq . ai/copilot/config.json. REAL if the trailing element lacks a comma; the diff shows lines 6-7 correctly comma-separated, so this is THEORETICAL and should be confirmed by running jq.

Comment thread tests/herdr-config.bats
Comment on lines +43 to +53
@test "every harness that trusts folders trusts the worktrees herdr creates (#2503)" {
# herdr puts a worktree at <directory>/<repo>/<branch-slug>. A harness that
# does not trust that path asks, or refuses, in every worktree herdr opens.
run sed -n 's/^directory[[:space:]]*=[[:space:]]*"~\(.*\)"$/{HOME}\1\/*\/*/p' "$SRC"
[ -n "$output" ] || { echo "no ~-relative [worktrees] directory in $SRC"; false; }
want="$output"
run jq -e --arg w "$want" '.trustedWorkspaces | index($w)' "$REPO/ai/agy/settings.json"
[ "$status" -eq 0 ] || { echo "agy trustedWorkspaces lacks $want"; false; }
run jq -e --arg w "$want" '.trustedFolders | index($w)' "$REPO/ai/copilot/config.json"
[ "$status" -eq 0 ] || { echo "copilot trustedFolders lacks $want"; false; }
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Vacuous Assertion

The new @test derives the expected trust entry with sed -n and only asserts [ -n "$output" ]. The sed pattern requires directory = "~..." with no leading whitespace and a closing quote at end of line, so if the TOML is reformatted (e.g. a trailing comment after the value), $output is empty and the guard fires with a clear message, which is correct. However, the run jq -e ... index($w) checks exit status only, and index returns null (exit 1) on miss, so the [ "$status" -eq 0 ] check is sound. REAL behaviour is correct; no defect found on re-trace. THEORETICAL risk only: sed on a BSD host with the \/ and [[:space:]] forms is portable, so no action needed.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 3ce2912)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2503 - Partially compliant

Compliant requirements:

  • herdr root moved to a visible, non-dot-prefixed location.
  • agy trustedWorkspaces trusts the herdr worktree depth.
  • Copilot trustedFolders trusts the herdr worktree depth.
  • The comment/value contradiction in the herdr config is fixed.

Non-compliant requirements:

  • dotf worktree add/done still resolve the sibling -wt-, not the herdr root.
  • The root is not declared once; the path is still hand-written in three files.
  • agy exclude and trust lists are not derived from a shared root.
  • No migration of existing worktrees.
  • AGENTS.md §9 and the skills are not updated.

Requires further human verification:

  • Owner decision on the root (the ticket marks it as owner-only); the PR picks one without a recorded decision.
  • Whether the copilot deploy strategy is merge, so runtime-added trust entries survive (not visible in this diff).
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Ticket Decision Bypass
REAL: ticket #2503 lists the root as an "owner decision needed" with three options. The PR chooses ~/Projects/worktrees and ships it under "Refs #2503" with no recorded owner approval in the visible description. Trace: the config is merged, herdr creates worktrees under the chosen root, and the owner's choice is effectively made by the implementer. Record the owner decision on the ticket before merge, or mark the PR as provisional.

Harness Compliance
THEORETICAL: Spec-Driven Development requires either a change under specs/<id>/ or the skip-sdd label with a written rationale. The diff touches no specs/ path, and the visible PR body is truncated before any label or rationale. The Go change (about 50 executable lines in model_pins.go) and the new public config path are both over the FTD trigger. Confirm the label and rationale are present, or add the spec folder.


⚙️ Agent run details
  • Model: anthropic/claude-haiku-5-5
  • Tokens: 28,500 in / 6,163 out / 34,663 total
  • Time cost: 34.9s
  • AI calls: 1
✅ Resolved findings

ai/agy/settings.json:17-21

Harness compliance

Harness compliance (AGENTS.md / CLAUDE.md): the PR body and branch are in English, but the PR title and body state the change is partial and defer the ticket. Standing Order 4 (clean as you go, track-or-fix) requires the deferred half (dotf worktree add/done, skills, AGENTS.md §9 still on sibling layout) to be ticketed with root cause. The PR body names it but does not reference a ticket for the remainder, only #2503 which is the umbrella. REAL but low severity; confirm a follow-up issue exists for the deferred half.

tests/herdr-config.bats:43-53

Vacuous Assertion

The new @test derives the expected trust entry with sed -n and only asserts [ -n "$output" ]. The sed pattern requires directory = "~..." with no leading whitespace and a closing quote at end of line, so if the TOML is reformatted (e.g. a trailing comment after the value), $output is empty and the guard fires with a clear message, which is correct. However, the run jq -e ... index($w) checks exit status only, and index returns null (exit 1) on miss, so the [ "$status" -eq 0 ] check is sound. REAL behaviour is correct; no defect found on re-trace. THEORETICAL risk only: sed on a BSD host with the \/ and [[:space:]] forms is portable, so no action needed.

ai/copilot/config.json:5-7

Possible Issue

The trustedFolders hunk is malformed JSON. The diff removes the closing "{HOME}/Projects/Workspace/*" line as shown in the old hunk, but the new hunk adds "{HOME}/Projects/Workspace/*" after "{HOME}/Projects/Workspace" with a trailing comma structure that is valid only because the new array is present. Verify the file parses: jq . ai/copilot/config.json. REAL if the trailing element lacks a comma; the diff shows lines 6-7 correctly comma-separated, so this is THEORETICAL and should be confirmed by running jq.

@mlorentedev mlorentedev added the skip-sdd SDD spec-gate escape hatch (requires '## SDD skip rationale' in PR body) label Oct 15, 2026
@mlorentedev

Copy link
Copy Markdown
Owner Author

/review

Comment thread tests/copilot-config.bats Outdated
Comment on lines +135 to +136
# Copilot's half is untouched: `trustedFolders` is still ours to ship.
[ "$(jq -c '.trustedFolders' "$CFG")" = '["{HOME}/Projects","{HOME}/Projects/*","{HOME}/Projects/Workspace","{HOME}/Projects/Workspace/*"]' ]
[ "$(jq -c '.trustedFolders' "$CFG")" = '["{HOME}/Projects","{HOME}/Projects/*","{HOME}/Projects/Workspace","{HOME}/Projects/Workspace/*","{HOME}/Projects/worktrees/*/*"]' ]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Comment

REAL: the comment above the new assertion says "Copilot's half is untouched: trustedFolders is still ours to ship." This PR adds {HOME}/Projects/worktrees/*/* and {HOME}/Projects/Workspace/* to trustedFolders, so the comment is now false and will mislead the next editor of this test. Trace: a reader trusts the comment, assumes the exact-match assertion on line 136 was not changed, and skips verifying the new entry.

@mlorentedev

Copy link
Copy Markdown
Owner Author

Review triage

Reviewers: PR-Agent (automatic review on 24ee8813; /review on 3ce29129, since the push gate skips a single new commit). CodeRabbit did not review: its free OSS limit was reached, so this PR is reviewed by PR-Agent alone.

Finding Head Disposition
Possible Issue: Copilot trustedFolders malformed JSON 24ee881 Declined: the finding itself rates it THEORETICAL; jq -e . ai/copilot/config.json parses, and tests/copilot-config.bats pins the full list with jq.
Vacuous Assertion: tests/herdr-config.bats sed derivation 24ee881 Declined: the finding concludes "no defect found on re-trace". The case fails on an empty derivation and was mutated red twice (dropped Copilot entry, moved herdr root).
Harness compliance: deferred half needs a ticket 24ee881 Declined: #2503 is that ticket and stays open for exactly the deferred half; its scope is recorded on the issue (comments 6181801013, 6181818746, 6181873683).
Ticket Decision Bypass: root chosen without the owner 3ce2912 Declined: the owner chose ~/Projects/worktrees in the session that opened this PR; the decision is recorded on #2503 (comment 6181873683) before the PR's head commit.
Harness Compliance: SDD 3ce2912 Declined: the PR carries skip-sdd and a non-empty ## SDD skip rationale; spec-gate is green.
Stale Comment: tests/copilot-config.bats "Copilot's half is untouched" 3ce2912 Applied in 10400992: the comment now says AI-043 did not supersede Copilot's half and why the full list is pinned. Comment-only; bats tests/copilot-config.bats 0 not ok.

CI: every check green on 3ce29129; 10400992 changes one test comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 3/5 skip-sdd SDD spec-gate escape hatch (requires '## SDD skip rationale' in PR body)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant