feat(roborev): run reviews locally on each host - #2269
Conversation
|
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRoboRev now runs on Kyber and uses a local daemon at ChangesRoboRev hook integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Agent
participant roborev-agent.sh
participant RoboRevDaemon
participant Git
participant roborev-post-commit.sh
Agent->>roborev-agent.sh: send lifecycle event
roborev-agent.sh->>RoboRevDaemon: run agent-hook at 127.0.0.1:7373
Git->>roborev-post-commit.sh: run post-commit hook
roborev-post-commit.sh->>RoboRevDaemon: submit repository with post-commit --repo
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| }; | ||
| core = { | ||
| editor = "nvim -c \"stopinsert\""; | ||
| hooksPath = "${config.xdg.configHome}/git/hooks"; |
There was a problem hiding this comment.
Global core.hooksPath disables all per-repo hooks
git help config under core.hooksPath: when set, git looks only here for hooks and ignores $GIT_DIR/hooks/*. Setting this in the global user config means every repo on the machine will lose its own .git/hooks/* — including hooks installed by:
pre-commit(the Python framework)husky,lefthook,simple-git-hooks- Any project-managed
commit-msg,pre-push,pre-rebase,post-checkout,post-merge, etc.
The only hook that will ever run globally is the post-commit shim added here. If the goal is just to add a global post-commit, consider one of:
- Point
hooksPathat a directory that dispatches to both the roborev shim and the per-repo hook (delegator script that also execs$GIT_DIR/hooks/<hookname>when present); or - Install the post-commit hook via
git config --global init.templateDir+ a templatehooks/folder (only affects newly-initialized repos, but doesn't override existing ones); or - Have
roborevregister itself per-repo when it starts reviewing that repo, instead of globally.
As written this is a large behavior change for every repo you commit in.
| exit 0 | ||
| fi | ||
|
|
||
| "$ROBOREV_BIN" --server "127.0.0.1:7373" post-commit --repo "$(git rev-parse --show-toplevel)" || true |
There was a problem hiding this comment.
Fires on every commit in every repo
Because core.hooksPath in home-manager/programs/git/default.nix makes this the global post-commit for the user, on hosts where ~/.local/bin/roborev exists this will submit every commit in every repository (dotfiles, throwaway scratch repos, third-party clones you're just patching, work repos that shouldn't leave the box, etc.). If the daemon doesn't have a server-side allow-list, the review queue will be flooded with unrelated repos and possibly repos you don't want RoboRev seeing.
Consider gating here — e.g. only submit when the toplevel matches a configured allow-list or a marker file (.roborev at the repo root) is present.
| }, | ||
| { | ||
| "command": "$HOME/dotfiles/config/shared/hooks/roborev-agent.sh", | ||
| "type": "command" |
There was a problem hiding this comment.
Missing async/timeout
Every other hook in this PostToolUse Bash group (and in the new PreToolUse Bash / Stop groups added below) uses "async": true with an explicit "timeout". The Codex and Factory entries for the same script use "timeout": 10. As written this Claude entry runs synchronously with no timeout, so every Bash tool call and every Stop event will block until roborev responds — up to Claude's default hook timeout (60s) if the daemon is missing or slow.
Suggest matching the neighbors:
{
"async": true,
"command": "$HOME/dotfiles/config/shared/hooks/roborev-agent.sh",
"timeout": 10,
"type": "command"
}Same fix needed at the other two new occurrences in this file (around L285 and L465).
| ); | ||
| in | ||
| { | ||
| home.file.".local/bin/roborev-agent-hook" = { |
There was a problem hiding this comment.
Unused install
Nothing invokes ~/.local/bin/roborev-agent-hook or ~/.local/bin/roborev-post-commit. All hook configurations — config/claude/settings.json, config/codex/hooks.json, config/factory/settings*.json, and home-manager/programs/git/default.nix — point directly at the source scripts under $HOME/dotfiles/config/shared/hooks/. These home.file entries just create dead copies in ~/.local/bin/.
Either drop them, or switch all callers to the ~/.local/bin/ paths so the installed copies are the single source of truth. Also note: config/roborev/default.nix is imported unconditionally from config/default.nix:44, so these files land on hosts where the daemon is disabled too, which is inconsistent with the isGalactica || isKyber || isMatic gate on the service.
| { lib, ... }: | ||
| { config, lib, ... }: | ||
| { | ||
| home.file."${config.xdg.configHome}/git/hooks/post-commit" = { |
There was a problem hiding this comment.
Use xdg.configFile for XDG paths
Every other XDG-managed file in this repo uses xdg.configFile."<relative>" (e.g. config/amp/default.nix:2, config/ghostty/default.nix:9, config/hyprland/default.nix:20). home.file keys are documented as relative to $HOME; interpolating ${config.xdg.configHome} yields an absolute path here.
Suggested:
xdg.configFile."git/hooks/post-commit" = {
source = ../../../config/shared/hooks/roborev-post-commit.sh;
executable = true;
force = true;
};
| The output should include '127.0.0.1:7373' | ||
| End | ||
|
|
||
| Describe 'config/factory/activate-settings.sh' |
There was a problem hiding this comment.
Accidentally nested Describe
There's no End between L7 and this Describe, so config/factory/activate-settings.sh runs as a child block of config/shared/hooks/roborev-agent.sh instead of as a sibling. It still parses (End counts balance), but spec output will report config/factory/activate-settings.sh nested under config/shared/hooks/roborev-agent.sh.
Add an End after L7 to close the first Describe, and drop one of the two Ends on L14-15 so the counts remain balanced. Same fix would apply if you want the config/shared/hooks/roborev-post-commit.sh block to be a top-level sibling too.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
spec/activate_roborev_spec.sh (1)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest behavior instead of source text.
These assertions can pass when a string appears in an unused branch or when a hook path exists but is not executable. Assert rendered daemon arguments, regular executable hook files, and the commands received by a stub RoboRev client.
spec/activate_roborev_spec.sh#L22-L24: validate the evaluated host enablement condition.spec/activate_roborev_spec.sh#L42-L47: verify the daemon service arguments use127.0.0.1:7373.spec/activate_roborev_spec.sh#L49-L56: verify both hooks are regular executable files.spec/roborev_hooks_spec.sh#L3-L7: execute the agent-hook path with representative input.spec/roborev_hooks_spec.sh#L9-L13: verify managed-hook cleanup recognizes the installed command.spec/roborev_hooks_spec.sh#L17-L21: assert the post-commit command receives the repository path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/activate_roborev_spec.sh` around lines 22 - 24, Replace source-text assertions with behavioral tests: in spec/activate_roborev_spec.sh lines 22-24, validate evaluated host enablement; lines 42-47, assert rendered daemon arguments use the loopback address and port; lines 49-56, verify both hook paths are regular executable files. In spec/roborev_hooks_spec.sh lines 3-7, execute the agent hook with representative input; lines 9-13, verify cleanup recognizes the installed managed-hook command; lines 17-21, assert the post-commit command receives the repository path, using a stub RoboRev client to capture commands.config/claude/settings.json (1)
162-165: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid blocking on informational RoboRev events.
config/shared/hooks/roborev-agent.sh:1-11forwards the event to the local daemon.PostToolUseandStopdo not need to gate the completed tool or session termination. Mark these hooks as"async": true. If synchronous ordering is required, add a short explicit timeout and document the requirement.
config/claude/settings.json#L162-L165: makePostToolUseasynchronous or add a timeout.config/claude/settings.json#L463-L466: makeStopasynchronous or add a timeout.config/codex/hooks.json#L24-L33: makePostToolUseasynchronous.config/codex/hooks.json#L195-L199: makeStopasynchronous.config/factory/settings.json#L58-L67: makePostToolUseasynchronous.config/factory/settings.json#L77-L87: makeStopasynchronous.config/factory/settings.tpl.json#L58-L67: makePostToolUseasynchronous.config/factory/settings.tpl.json#L77-L87: makeStopasynchronous.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/claude/settings.json` around lines 162 - 165, PostToolUse and Stop RoboRev hooks currently block tool completion or session termination on informational events. In config/claude/settings.json lines 162-165 and 463-466, config/codex/hooks.json lines 24-33 and 195-199, config/factory/settings.json lines 58-67 and 77-87, and config/factory/settings.tpl.json lines 58-67 and 77-87, update the PostToolUse and Stop hook entries invoking roborev-agent.sh to run asynchronously with "async": true; do not add timeouts unless synchronous ordering is explicitly required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@home-manager/services/roborev/default.nix`:
- Line 51: Add X-SwitchMethod = "restart"; to the roborev systemd unit
containing ExecStart, ensuring Home Manager restarts the daemon when the
ExecStart command changes. Do not add X-RestartIfChanged.
In `@spec/roborev_hooks_spec.sh`:
- Around line 14-15: Remove the unmatched top-level End statement from
roborev_hooks_spec.sh, preserving the two End statements that close the existing
Describe blocks.
---
Nitpick comments:
In `@config/claude/settings.json`:
- Around line 162-165: PostToolUse and Stop RoboRev hooks currently block tool
completion or session termination on informational events. In
config/claude/settings.json lines 162-165 and 463-466, config/codex/hooks.json
lines 24-33 and 195-199, config/factory/settings.json lines 58-67 and 77-87, and
config/factory/settings.tpl.json lines 58-67 and 77-87, update the PostToolUse
and Stop hook entries invoking roborev-agent.sh to run asynchronously with
"async": true; do not add timeouts unless synchronous ordering is explicitly
required.
In `@spec/activate_roborev_spec.sh`:
- Around line 22-24: Replace source-text assertions with behavioral tests: in
spec/activate_roborev_spec.sh lines 22-24, validate evaluated host enablement;
lines 42-47, assert rendered daemon arguments use the loopback address and port;
lines 49-56, verify both hook paths are regular executable files. In
spec/roborev_hooks_spec.sh lines 3-7, execute the agent hook with representative
input; lines 9-13, verify cleanup recognizes the installed managed-hook command;
lines 17-21, assert the post-commit command receives the repository path, using
a stub RoboRev client to capture commands.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 99ccc32a-01a4-4e59-9b60-403f0273d0f4
📒 Files selected for processing (14)
Makefileconfig/claude/settings.jsonconfig/codex/hooks.jsonconfig/factory/activate-settings.shconfig/factory/settings.jsonconfig/factory/settings.tpl.jsonconfig/roborev/default.nixconfig/shared/hooks/roborev-agent.shconfig/shared/hooks/roborev-post-commit.shhome-manager/programs/git/default.nixhome-manager/services/roborev/default.nixspec/activate_roborev_spec.shspec/coverage_spec.shspec/roborev_hooks_spec.sh
| Service = { | ||
| Type = "notify"; | ||
| ExecStart = "${roborevBin} daemon run"; | ||
| ExecStart = "${roborevBin} daemon run --addr ${serverAddr}"; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 'X-SwitchMethod|X-RestartIfChanged|ExecStart|systemd\.user\.services' \
home-manager/services/roborev/default.nixRepository: shunkakinoki/dotfiles
Length of output: 808
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== home-manager/services/roborev/default.nix =="
cat -n home-manager/services/roborev/default.nix | sed -n '1,120p'
echo
echo "== package versions/providers mentioning robust/home-manager context =="
rg -n "home-manager|flake\.nix|nixpkgs|systemd\.user\.services|X-SwitchMethod|X-RestartIfChanged" -S . \
--glob '!result*' --glob '!*.swp' --glob '!*.swn' | sed -n '1,220p'Repository: shunkakinoki/dotfiles
Length of output: 21405
Add X-SwitchMethod = "restart"; to the roborev systemd unit.
ExecStart changes without X-SwitchMethod can leave the old daemon running after a Home Manager switch. Do not use X-RestartIfChanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@home-manager/services/roborev/default.nix` at line 51, Add X-SwitchMethod =
"restart"; to the roborev systemd unit containing ExecStart, ensuring Home
Manager restarts the daemon when the ExecStart command changes. Do not add
X-RestartIfChanged.
Source: Learnings
| @@ -45,7 +48,7 @@ lib.mkIf enabled { | |||
| }; | |||
There was a problem hiding this comment.
Missing X-SwitchMethod = "restart";
This PR changes ExecStart (adds --addr 127.0.0.1:7373). Without Unit.X-SwitchMethod set, home-manager may not restart the daemon on switch, so the old process keeps running on the old (default) address and hook callers connecting to 127.0.0.1:7373 will fail until the user manually restarts the unit.
Every other daemon in this repo picks one of the X-SwitchMethod values — openclaw and cpa-manager-plus use "restart"; cass/caam/docker-postgres/obsidian/etc. use "keep-old". For a service whose invocation just changed, "restart" is the right pick:
Unit = {
Description = "roborev code review daemon";
Documentation = [ "https://github.com/roborev-dev/roborev" ];
After = [ "network.target" ];
X-SwitchMethod = "restart";
};
1 participant
Summary
Verification
shellspec spec/activate_roborev_spec.sh spec/roborev_hydrate_spec.sh spec/roborev_hooks_spec.sh spec/coverage_spec.shmake buildSummary by cubic
Run RoboRev reviews locally per host with a loopback-only daemon and a shared agent hook. Enables on galactica, kyber, and matic; removes the unused global Git post-commit hook.
New Features
roborevdaemon on galactica, kyber, and matic, bound to 127.0.0.1:7373.config/shared/hooks/roborev-agent.shthat forwards agent events to the local daemon; wire into Codex/Claude/Factory settings (agent/session, post-tool, stop) with timeouts and allowlist updates.make systemctl-roborevto restart on kyber; add tests for daemon bind and hook wiring.Migration
~/.local/bin/roborevis installed on each host.make buildthenmake systemctl-roborev).Written for commit 857d272. Summary will update on new commits.