Repository navigation
fix(lane): show refusals and print no-backend commands in the worktree layout - #2578
Conversation
…the worktree layout Round-1 review of CLI-119 failed on four findings: - lane exec and lane next silenced their errors, so every refusal exited 1 with no output; - lane next with no backend and no --agent errored instead of printing the commands; - the printed worktree path ignored WORKTREE_ROOT and the branch slug; - a binary path no shell reads alike silently became bare dotf. Also reject a lane base without a remote/branch split, split Handoff and runLaneNext under 40 lines, disposition the minors in verification.md (#2576) and tick the stale contract boxes.
|
Warning Review limit reachedYou'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 3 minutes. View limit details
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. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| func launch(o Options, b Backend, wt Worktree, found bool, out Outcome, p plan) error { | ||
| if err := writePrompt(o, p.file, p.prompt); err != nil { | ||
| return err | ||
| } | ||
| st, err := start(o, b, wt, found, out.Branch, out.Issue) | ||
| if err != nil { | ||
| return out, err | ||
| return err | ||
| } | ||
| if err := ensureLease(o, st.Worktree.Path, issue.Number, out.Agent); err != nil { | ||
| return out, err | ||
| if err := ensureLease(o, st.Worktree.Path, out.Issue.Number, out.Agent); err != nil { | ||
| return err | ||
| } | ||
| return b.Type(st.Pane, p.exec) | ||
| } |
There was a problem hiding this comment.
Dry-run Side Effect
REAL: On the herdr path with found && wt.Open and o.DryRun false, ensureLease runs. Unchanged by this PR, but the new launch helper writes the prompt before start. If start fails, the prompt file is left behind under PromptDir with no lease, and a later run overwrites it. Low impact: stale prompt file only.
| // selfBinary is the path of the running dotf, which the successor is told to | ||
| // run. An empty answer falls back to `dotf` on the pane's PATH, which may be a | ||
| // different release: say so rather than start it silently. | ||
| func selfBinary(c *cobra.Command) string { | ||
| self, err := os.Executable() | ||
| if err != nil { | ||
| _, _ = fmt.Fprintf(c.ErrOrStderr(), "lane next: cannot locate this binary (%v); the successor runs `dotf` from the pane's PATH\n", err) | ||
| return "" | ||
| } | ||
| return self | ||
| } |
There was a problem hiding this comment.
Possible Issue
THEORETICAL: selfBinary prints a warning on stderr when os.Executable() fails, then returns "" so the successor runs dotf from PATH. The warning is emitted during option construction for every run, including dry runs, so it appears even when nothing is started. Only triggers when os.Executable errors, which is rare.
| "git -C " + o.Parent + " fetch " + remote, | ||
| "git -C " + o.Parent + " worktree add -b " + out.Branch + " " + dir + " " + o.Config.Base, | ||
| "cd " + dir, |
There was a problem hiding this comment.
Shell Quoting
THEORETICAL: manualCommands joins o.Parent, dir and out.Branch into a shell line without quoting. A repo path containing spaces (for example /home/a b/repo) yields a printed command a person cannot paste as-is. plainWord guards only the prompt and dotf paths, not these. Only affects the printed no-backend commands.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
⚙️ Agent run details
|
Review triageReviewer: the PR reviewer guide (github-actions); CodeRabbit was rate limited. The first review run published nothing (the NaN attempt hung, Haiku returned no parseable review); a re-run produced it.
|
1 participant
Closes the round-1 FAIL of CLI-119-lane-next (review merged in #2573). Refs #2511, #2528.
lane exec/lane nextno longer swallow errors: a refusal prints its text on stderr (Blocker).--agent: the commands print with--agent <agy|claude|pi>and a note, exit 0. No harness is privileged by a default; AC5 text is unchanged.WORKTREE_ROOT+worktree.ResolveWorktreePath, and refuses when the root does not resolve.dotf;LoadConfigrejects abasewithout<remote>/<branch>.HandoffandrunLaneNextsplit to 39 lines each (were 72 and 51).Verification:
go build,go vet,GOOS=windows go vet,go test ./internal/lane ./internal/cmd ./internal/worktree, golangci-lint (0 issues), and all sixfeatures.jsoncommands exit 0. Mutation: restoringSilenceErrorsfailsTestLane_RefusalsAreShownOnStderr. Dispositions inspecs/CLI-119-lane-next/verification.md.0.71.0 (#2520) is released and installed, which was the precondition for a live AC6 run. The re-review (
dotf spec review CLI-119-lane-next) comes after this merges.Knowledge
TestLane_RefusalsAreShownOnStderrReview triage
Reviewer: the PR reviewer guide (github-actions); CodeRabbit was rate limited. The first review run published nothing (the NaN attempt hung, Haiku returned no parseable review); a re-run produced it.
noBackendreturns an error withNote/Commandsunset whenWorktreeRootis empty: declined, no change. The reviewer itself confirmsrunLaneNextreturns the error to the terminal, which is the intended behaviour (TestNext_NoBackendWithoutAWorktreeRootRefuses).Refs, notCloses; both tickets stay open for thelane start/ landing-hook scope.