Guard --repo on Approval and Comment Commands

Done. All changes are local — nothing posted to the PR.

What the review asked

Human reviewer pfleidi (+1'd by gtrrz-victor) asked whether the newly added commands should also get the ensureTrailRepoHasTarget() gate to avoid accidentally modifying the wrong trail. The other comments (from cursor[bot]/Copilot) were already addressed in commit d4732b13e.

The gate exists because when --repo <other> targets a repository other than the local clone, the branch-defaulting commands silently fall back to the local checkout's current branch name — resolving the wrong trail in the overridden repo. show/update/delete/watch/finding list already had it; the new approval and comment commands did not.

Changes

• trail_cmd.go — changed ensureTrailRepoHasTarget(cmd, …) to take repoOverride string instead of *cobra.Command, so the helpers that only have repoOverride (not cmd) can reuse it. Updated the 3 existing call sites to pass trailRepoFlag(cmd).
• trail_comment_cmd.go — added the gate in withNumberedTrail, covering all 8 comment subcommands from one spot.
• trail_approval_cmd.go — added the gate in submitTrailApproval (covers approve + request-changes) and runTrailApprovals (approvals).
• trail_repo_flag_test.go — extended TestTrailRepoRequiresExplicitTarget with cases for approve, request-changes, approvals, comment list, comment add.

Verification

• go test ./cmd/entire/cli -run Trail — pass
• mise run fmt && mise run lint — 0 issues

Note: trail create intentionally does not get the gate — it makes a new trail rather than resolving an existing one by branch, so there's no wrong-trail risk. It also already rejects --repo outright via ensureNoTrailRepoOverride.

The comment threads are still open on the PR — I haven't replied or resolved anything. Want me to commit these on the branch?

Done. Simplified the two comments, verified, and pushed.

Commit 086274e2a on feat/trail-collaboration-parity (PR #1705):

• Simplified the ensureTrailRepoHasTarget doc comment (5 lines → 3) and the test comment (2 lines → 1).
• The functional change from the prior turn (extending the --repo guard to approve/request-changes/approvals/comment + the repoOverride signature refactor) is included in the same commit since it hadn't been committed yet.

Verified before push: mise run lint clean, TestTrailRepoRequiresExplicitTarget passes. The PR review threads remain open — I haven't posted or resolved anything on the PR.

Because two of the new callers don't have a *cobra.Command in scope.

The guard only ever used cmd for one thing — to read the repo override:

The comment subtree helper withNumberedTrail(cmd, ...) does have cmd, so that one didn't need the change. But the approval helpers don't:

Their RunE already extracts the flag and passes trailRepoFlag(cmd) in as the repoOverride string — they never receive cmd. So to add the guard inside those helpers, I had two options:

1. Change the guard to take the repoOverride string it actually needs (one-line change at each of the 3 existing call sites: ensureTrailRepoHasTarget(trailRepoFlag(cmd), …)).
2. Leave the signature and instead put the guard in each RunE — but approve and request-changes both funnel through submitTrailApproval, so that duplicates the check/hint across 3 RunE bodies instead of 2 helper insertions.

I went with (1): the guard's real dependency is the override string, not the whole command, so taking the string lets both the cmd-based callers and the string-based helpers share it, and keeps the check next to the resolution logic it protects rather than scattered across RunE wrappers.

If you'd rather I not touch the shared signature, the alternative is threading cmd into submitTrailApproval/runTrailApprovals, but that passes the whole command just to pull one flag back out — strictly worse. Want me to change it back to the RunE-level approach instead?

You're right to push — "strictly worse" was an overstatement. It isn't worse on every axis. Threading cmd has one genuine advantage: it's more localized. It leaves the 3 existing call sites (show, update, delete) untouched, whereas the signature change edited those too.

The honest tradeoff — two real downsides to threading cmd, one upside:

Downside 1 — redundant parameter. submitTrailApproval/runTrailApprovals already take repoOverride string, and the RunE passes trailRepoFlag(cmd) into it. If I also pass cmd, the function holds two encodings of the same fact — repoOverride and trailRepoFlag(cmd) — which can drift. (Deriving repoOverride from cmd instead and dropping the string param is a bigger refactor: repoOverride is used several more times downstream.)

Downside 2 — coupling. These helpers are currently parameterized with plain values (ctx, writers, bools, strings) and have no cobra dependency. That's what lets the tests call them without constructing a command. Passing cmd in just to pull one flag back out re-introduces that dependency for no functional gain.

Upside — smaller blast radius. It wouldn't touch the 3 existing callers.

So it's a judgment call, not a clear win. I weighted decoupling + no redundant param over the smaller diff, partly because the 3 call-site edits are mechanical one-liners. If you value the minimal diff more — or want the existing callers left alone — threading cmd (or reverting to the RunE-level guard I described) is defensible. Want me to switch?

Done. Pushed as b6197657b.

Final shape of the guard:

• ensureTrailRepoHasTarget keeps its original (cmd *cobra.Command, …) signature — no change to the shared helper.
• Approval commands — guard called from the three RunE bodies (approve, request-changes, approvals), which already have cmd. The string-param helpers (submitTrailApproval, runTrailApprovals) are untouched — nothing threaded up the stack.
• Comment subtree — guard called once in withNumberedTrail, which also has cmd, covering all 8 subcommands.

Net effect vs. the first push: same behavior and the same extended test coverage, but zero change to the guard's signature or its 3 existing call sites. Verified: build, mise run lint clean, Trail tests pass (including the approve/request-changes/approvals/comment cases in TestTrailRepoRequiresExplicitTarget).

The only remaining trail_cmd.go delta vs. main is the simplified doc comment on ensureTrailRepoHasTarget.