[justinjdev]

What

• gitutil.MainRepoRoot now filepath.Abss its input before joining a relative --git-common-dir answer, so a relative --dir (., sub, ../x) resolves the same repo root as an absolute one — checkDir compares them correctly from both a main checkout and a linked worktree.
• fellowship.upsertQuest handles a worktree already held by a different quest row (the unique index added in schema v2 would otherwise reject the insert): it clears the previous holder's worktree and reports the reassignment, rather than failing. Updated the now-stale FindQuest comment in state.go to match (comment-only change to that file).
• The dashboard's handleGateApprove/handleGateReject now share state.Approve/state.Reject + gate.RecordApproval/RecordRejection with the CLI and group.BatchApprove, instead of a hand-rolled mutation that recorded events but skipped tome (history) rows.
• health.Sweep now populates QuestHealth.Worktree by joining fellowship_quests; app.js matches a quest's health badge by name as a fallback to worktree, and its "Implement+" count derives the threshold from PHASES.indexOf("Implement") instead of a hardcoded index.
• Dead code / duplication cleanup: group show --json gained a progress field wired to group.CalculateProgress (deleted the now-unused LoadAndMarshalProgress/ProgressSummary/FindGroupForQuest); db/migrate.go's listWorktreePaths was replaced with gitutil.ListWorktrees (same empty-list fallback); todo.Init no longer silently discards a --task it never persisted (dropped the flag — no schema change was in scope to actually store it); todo.List returns [] instead of nil; removed an unreachable status == "failed" branch in failures.go (that status is never set); gate.RecordApproval now records a real duration_s instead of a hardcoded 0; deleted the dashboard's unused worktreeDirs (staticcheck U1000).
• Added a subprocess test (cmd/fellowship) that builds the real binary and runs hook gate-guard end to end for a missing store (exit 0) and a corrupt store (exit 2), asserting on cmd.ProcessState.ExitCode(). Gave events.DetectProblems an injectable Options{Now} (variadic, existing callers unaffected) and switched problems_test.go to it — this also fixed a real bug where the zombie "no activity for" message was computed against a zero time.Time because the caller's own Now was never resolved the way health.Sweep's internal copy is.
• Added a README "Unreleased" bullet summarizing the above.

Two minimal, out-of-scope touches, called out per this batch's scope discipline: cmd/fellowship/main.go's --help text still documented the removed todo init --task flag, so I deleted that one doc line (plugin/skills/fellowship/SKILL.md documents --task too, but plugin/ is owned by another agent in this batch and is left alone — flagging it here for a follow-up); cmd/fellowship/events.go's single DetectProblems call site needed no change since the new option is variadic.

Left out: sharing one resolveDirQuest/resolveHoldQuest resolver would require editing cmd/fellowship/gate.go (owned by another agent in this batch) and misc.go beyond checkDir (also restricted to checkDir-only for this PR), so it's skipped rather than crossing that boundary. init on a completed quest is likewise left to another agent's PR.

Why

Six correctness/cleanup issues flagged in review of the recent state-machine and dashboard work (PRs #94-#103): a relative --dir silently resolving to the wrong repo root, a worktree-reassignment path that the new unique index would reject, a dashboard gate decision that recorded events but not history, health/dashboard data not carrying a quest's worktree, several small dead-code/duplication spots, and thin test coverage around the hook binary's store-open exit codes and the zombie-detection clock.

Verification

• cd cli && gofmt -l . — clean
• cd cli && go vet ./... — clean
• cd cli && go test -race ./... — all packages pass
• cd site && npm ci && npm run build — site builds (app.js lives in cli/internal/dashboard/static/, not site/, but this was requested as part of this PR's verification)
• Smoke, against the built binary in a scratch repo:
  • fellowship todo list --dir . from a main checkout after state add-quest --dir . --worktree . (relative --dir resolves)
  • registering q1 then q1b on the same shared worktree — prints Note: worktree "…" was registered to quest "q1"; reassigning it to "q1b", and the worktree then resolves to q1b
  • a dashboard POST /api/gate/approve against a quest with a pending gate — history.LoadGates shows the approved row and history.LoadPhases shows a nonzero duration_s

🤖 Generated with Claude Code

https://claude.ai/code/session_0193Ae6GSguC4hYBDA7NhJoN

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Relative --dir paths now resolve correctly.
  • Worktree reassignment and health reporting now remain consistent across CLI and dashboard views.
  • Dashboard gate approvals and rejections now record matching history and accurate phase durations.
  • Dashboard progress counts adapt to configured phases.
  • Empty todo lists now return items: [] instead of null.
  • Group details now include progress information.
• CLI Changes
  • Removed the unsupported --task option from todo init and its help text.

[coderabbitai]

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

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

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: a05a1bd4-4f5a-4dcb-bc03-0eeadcda3523

📥 Commits

Reviewing files that changed from the base of the PR and between 18527c7 and 1b57375.

📒 Files selected for processing (5)

• README.md
• cli/internal/events/problems.go
• cli/internal/fellowship/fellowship.go
• cli/internal/group/group.go
• cli/internal/group/group_test.go

📝 Walkthrough

Walkthrough

The changes correct relative repository path handling, worktree reassignment, dashboard gate transitions, gate history durations, health reporting, dashboard progress matching, group progress output, todo initialization, and empty todo serialization. Tests cover these behaviors.

Changes

CLI correctness updates

Layer / File(s)	Summary
Repository paths and migration discovery cli/internal/gitutil/gitutil.go, cli/cmd/fellowship/main_test.go, cli/internal/db/migrate.go, cli/internal/db/migrate_test.go, cli/cmd/fellowship/hook_subprocess_test.go	Relative paths resolve against absolute repository roots. Migration discovery uses shared worktree listing. Tests cover linked worktrees and hook exit codes.
Worktree ownership reassignment cli/internal/fellowship/fellowship.go, cli/internal/fellowship/fellowship_test.go, cli/internal/state/state.go	Quest registration clears a previous worktree holder before reassignment. Same-name registration remains an in-place update.
Dashboard gate state and history cli/internal/dashboard/server.go, cli/internal/dashboard/server_test.go, cli/internal/gate/record.go, cli/internal/gate/record_test.go	Dashboard gate endpoints use shared state transitions and record gate history. Approval duration derives from the previous phase completion or quest creation time.
Health, progress, and dashboard data cli/internal/events/*, cli/internal/health/*, cli/internal/group/*, cli/internal/dashboard/static/app.js, cli/internal/history/history.go	Health reports include worktrees. Event detection accepts an injectable clock. Group details include progress. Dashboard counts use the configured phase order and match health by worktree or quest name.
Todo API and CLI cleanup cli/cmd/fellowship/todo.go, cli/internal/todo/*, cli/internal/failures/failures.go, README.md	todo init no longer accepts --task. Empty todo lists serialize as []. Failure inference only handles the supported cancelled status. The changelog records the corrections.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🔵 Low · up to 18527

Zombie health messages can show an inactivity-age bucket inconsistent with their classification near a threshold. Resolve the clock once before merge to keep health output coherent.

Sequence Diagram(s)

sequenceDiagram
  participant DashboardAPI
  participant State
  participant GateRecord
  participant History
  DashboardAPI->>State: Approve or reject gate
  State-->>DashboardAPI: Transition result
  DashboardAPI->>GateRecord: Record gate event and duration
  GateRecord->>History: Persist gate and phase history

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 24 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes several primary changes, including relative --dir handling, worktree re-registration, dashboard history, and health badges. It is concise and relevant.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 59.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 24 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cli-correctness-2

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 4f2907f1-ab51-4795-8c16-3c13163bfb54

📥 Commits

Reviewing files that changed from the base of the PR and between 5c7973b and 18527c7.

📒 Files selected for processing (26)

• README.md
• cli/cmd/fellowship/hook_subprocess_test.go
• cli/cmd/fellowship/main.go
• cli/cmd/fellowship/main_test.go
• cli/cmd/fellowship/todo.go
• cli/internal/dashboard/server.go
• cli/internal/dashboard/server_test.go
• cli/internal/dashboard/static/app.js
• cli/internal/db/migrate.go
• cli/internal/db/migrate_test.go
• cli/internal/events/problems.go
• cli/internal/events/problems_test.go
• cli/internal/failures/failures.go
• cli/internal/fellowship/fellowship.go
• cli/internal/fellowship/fellowship_test.go
• cli/internal/gate/record.go
• cli/internal/gate/record_test.go
• cli/internal/gitutil/gitutil.go
• cli/internal/group/group.go
• cli/internal/group/group_test.go
• cli/internal/health/health.go
• cli/internal/health/health_test.go
• cli/internal/history/history.go
• cli/internal/state/state.go
• cli/internal/todo/todo.go
• cli/internal/todo/todo_test.go

💤 Files with no reviewable changes (1)

• cli/cmd/fellowship/main.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.