Skip to content

Commit 382c6c8

Browse files
mariusvniekerkcodex
andcommitted
fix(agent-hook): constrain reminders to exact reviews
Automatic reminders could rediscover the same open reviews and turn an unrelated user task into a repeated repair loop. Delivered review IDs now define the reminder boundary for one agent session and repository lineage. The bundled fix workflow must validate findings before editing, close disproved reviews without code changes, and defer valid work outside the active task. Hook installation refreshes supported bundled skills so the default instruction never needs the separate CLI agent fallback. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>
1 parent ae5c22d commit 382c6c8

20 files changed

Lines changed: 769 additions & 364 deletions

File tree

README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ your agentic loop while context is fresh.
3232

3333
```bash
3434
roborev init # layer 1: per-commit reviews
35-
roborev skills install
36-
roborev agent-hook install # layer 2: auto-detect and wire installed agents
35+
roborev agent-hook install # layer 2: wire agents and bundled skills
3736
roborev agent-hook install --agent all # or wire every supported profile
3837
```
3938

@@ -103,9 +102,11 @@ closing the loop.
103102
`roborev agent-hook install` auto-detects installed Claude Code, Codex, Copilot
104103
CLI, Cursor, Factory Droid, Gemini CLI, Hermes, Qwen, and Grok Build harnesses
105104
and adds optional hooks after configured turn, commit, or failed-review
106-
thresholds are met. Reminders include a complete CLI fallback when no roborev
107-
skill is installed. Hermes delivers queued post-tool reminders at `Stop`; Cursor
108-
records the same events but emits no control response.
105+
thresholds are met. Reminders name exact review IDs and invoke the bundled
106+
`roborev-fix` skill without running `roborev fix --open`; supported profiles get
107+
current bundled skills during hook installation. Hermes delivers queued
108+
post-tool reminders at `Stop`; Cursor records the same events but emits no
109+
control response.
109110
The hook uses a separate local `roborev-agent-hook` daemon for session counters;
110111
it does not run inside the main roborev daemon.
111112

docs/agent-hook.md

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,17 @@ Roborev scopes commit and failed-review accounting to repository lineage, so
4242
activity in one worktree does not consume another worktree's reminder. Outside a
4343
tracked git repository the hook returns an empty native response.
4444

45-
The default instruction is self-contained. It uses the richer `roborev-fix`
46-
skill when available and otherwise tells the agent how to discover, inspect,
47-
fix, comment on, and close each review with the CLI. Installing skills remains
48-
recommended for Claude Code, Codex, and Factory Droid, but it is not required
49-
for the other profiles to receive an actionable reminder. The built-in default
50-
also forbids expanding the current task: out-of-scope or unclear findings stay
51-
untouched until the user gives direction.
45+
The default instruction names the exact review job IDs and invokes the
46+
`roborev-fix` skill for only those jobs. It never runs `roborev fix --open` or
47+
discovers additional reviews. The skill treats every finding as an unverified
48+
claim: invalid findings are documented and closed without code changes, valid
49+
in-scope findings are fixed and verified, and valid out-of-scope or unclear
50+
findings remain open until the user gives direction.
51+
52+
Delivered review IDs are acknowledged in the Agent Hook daemon's session state,
53+
scoped to the repository lineage. They do not trigger another reminder in that
54+
session, while newly created review IDs still do. Deferred reminders acknowledge
55+
their IDs only when delivered.
5256

5357
`instruction` is a complete override. Custom instructions are emitted without
5458
the built-in scope or continuation guidance.
@@ -83,6 +87,11 @@ Automatic and `all` installs attempt every selected profile and report all
8387
errors after preserving successful installs. `--dry-run` plans the same changes
8488
without writing.
8589

90+
For Claude Code, Codex, Factory Droid, and Grok Build, installation also creates
91+
or updates that profile's bundled roborev skills before activating the hook.
92+
Other hook profiles do not currently have bundled skill variants and receive no
93+
CLI fallback.
94+
8695
Factory Droid remains user-scoped. Roborev rejects project `.factory/hooks.json`
8796
paths because they are executable repository-local configuration.
8897

@@ -152,7 +161,7 @@ roborev agent-hook run --agent <profile>
152161
passes it through kit's typed dispatcher, posts a normalized request to the
153162
local `roborev-agent-hook` daemon, and lets kit encode the native response.
154163

155-
The local hook daemon stores only session accounting under:
164+
The local hook daemon stores session accounting and delivered review IDs under:
156165

157166
```text
158167
${ROBOREV_DATA_DIR:-~/.roborev}/agent-hook/
@@ -177,7 +186,7 @@ trigger type. The next Hermes `Stop` delivers one reminder, ordered by failed
177186
reviews before commits and then creation time.
178187

179188
Queued reminders retain the absolute triggering worktree and tell the agent to
180-
change to it before running fallback commands, even if the session changed
189+
change to it before running review commands, even if the session changed
181190
directories or used `git -C`. Delivery waits until that worktree is back on the
182191
triggering branch, or the exact triggering commit for a detached checkout, so
183192
the fallback commands query the intended lineage. Repeated triggers coalesce

docs/automation/post-commit-reviews.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ Then act on the reviews in whichever way fits how you work:
4242
re-reviews until every review passes.
4343

4444
The `roborev-fix` and `roborev-refine` skills come from `roborev skills install`
45-
(see [Agent Skills](../guides/agent-skills.md)).
45+
(see [Agent Skills](../guides/agent-skills.md)). Agent Hook installation updates
46+
the bundled skills automatically for supported profiles.
4647

4748
## Layer 2 - Agent hook
4849

4950
The agent hook watches supported coding-agent sessions and, once review work
50-
piles up, supplies either the roborev-fix skill or a complete CLI fallback
51-
before the session ends - closing the write -> review -> fix loop automatically.
51+
piles up, supplies exact review IDs to the `roborev-fix` skill before the
52+
session ends. It never runs the separate `roborev fix --open` agent workflow.
5253

5354
```bash
54-
roborev skills install # optional richer workflow for bundled agents
55-
roborev agent-hook install # auto-detect and wire installed agent harnesses
55+
roborev agent-hook install # wire harnesses and update supported bundled skills
5656
```
5757

5858
See [Agent Hook](../agent-hook.md) for thresholds and configuration.

docs/changelog.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,15 @@ All notable changes to roborev, grouped by minor release.
4747
`roborev status` form remains available with identical output, and JSON
4848
status includes the additive `web_url` field.
4949
- Default Agent Hook autofix reminders now keep the user's current task as an
50-
immutable scope boundary. Agents leave out-of-scope or unclear findings
51-
untouched and ask the user before doing broader work. Custom instructions
52-
remain complete overrides.
50+
immutable scope boundary, name exact review job IDs, and invoke only the
51+
bundled `roborev-fix` skill. They never run `roborev fix --open` or discover
52+
additional reviews. Custom instructions remain complete overrides.
53+
- The bundled `roborev-fix` skills now require agents to prove every finding
54+
against current code before editing. Invalid reviews are documented and
55+
closed without code changes; valid out-of-scope findings remain open for
56+
user direction.
57+
- `roborev agent-hook install` now installs or updates bundled skills
58+
automatically for Claude Code, Codex, Factory Droid, and Grok Build.
5359
- `roborev status` now lists active Agent Hook snoozes with their exact
5460
repository, worktree, branch, and expiry, while the TUI shows a contextual
5561
snooze badge for an exactly filtered checkout. See
@@ -89,6 +95,10 @@ All notable changes to roborev, grouped by minor release.
8995
- Fresh agent sessions now receive a short, bounded agentsview usage-indexing
9096
retry before Roborev falls back to job-log token data, reducing permanently
9197
missing cost estimates. See [Token Usage](/commands/#token-usage).
98+
- Agent Hook remembers delivered review IDs per agent session and repository
99+
lineage, preventing repeated reminders for the same reviews while allowing
100+
newly created reviews to trigger. Deferred reminders acknowledge IDs only
101+
when they are delivered.
92102
- The Codex `maximum` preset now requests literal `max` for explicit GPT-5.6
93103
`sol`, `terra`, and `luna` models. Older, default, and unknown models retain
94104
the compatible `xhigh` mapping, while exact `xhigh` remains distinct.

docs/guides/agent-skills.md

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ before roborev receives the path. Custom destinations are not tracked by
7474
an invocation; Claude Code, Codex, and Factory Droid must handle the surrounding
7575
request with their native agent behavior.
7676

77+
An Agent Hook invocation names exact job IDs and never broadens the user's
78+
active task. The skill does not discover other reviews in that mode. It first
79+
proves or disproves each finding against the current code, fixes only valid
80+
in-scope findings, closes invalid reviews with evidence and no code change, and
81+
leaves valid out-of-scope findings open for user direction.
82+
7783
**Claude Code** enforces this in skill metadata: the bundled skills set
7884
`disable-model-invocation: true`, so the model never selects a roborev skill on
7985
its own. Invoke a skill by typing its slash command (`/roborev-review-branch`)
@@ -212,11 +218,11 @@ The agent:
212218

213219
1. Discovers open reviews (or uses provided job IDs)
214220
1. Fetches all reviews and collects findings
215-
1. Groups findings by file and prioritizes by severity
216-
1. Fixes all issues across all reviews
217-
1. Runs tests to verify
218-
1. Records a comment on each closed review
219-
1. Offers to commit
221+
1. Proves each finding against the current code and repository constraints
222+
1. Fixes and verifies valid findings within the current task
223+
1. Documents and closes invalid reviews without changing code
224+
1. Leaves valid out-of-scope reviews open and asks the user
225+
1. Audits the original review IDs before reporting completion
220226

221227
This is the interactive equivalent of `roborev fix --batch` -- the agent sees
222228
all findings at once and can make coordinated fixes across related issues.
@@ -229,8 +235,9 @@ Target a specific job ID with `/roborev-fix`:
229235
/roborev-fix 1019
230236
```
231237

232-
The agent fetches the review, fixes issues by priority, runs tests, and offers
233-
to commit.
238+
The agent fetches the review, validates every finding, fixes and verifies only
239+
valid in-scope issues, and records evidence before closing the review. Valid
240+
out-of-scope findings remain open.
234241

235242
!!! note
236243

internal/agenthook/config_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,5 +239,4 @@ func TestResolveOptionsForAgentGrokUsesSelfContainedInstruction(t *testing.T) {
239239

240240
require.NoError(t, err)
241241
assert.Equal(t, DefaultInstruction, opts.Instruction)
242-
assert.Contains(t, opts.Instruction, "roborev fix --open --list")
243242
}

internal/agenthook/grok_install.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,6 @@ func DefaultGrokHooksPath() string {
3030
return filepath.Join(home, "hooks", "roborev.json")
3131
}
3232

33-
func runGrokInstall(opts InstallOptions) (kitagenthook.Result, error) {
34-
result, err := planGrokInstall(opts)
35-
if err != nil || opts.DryRun || !result.Changed {
36-
return result, err
37-
}
38-
if err := commitAgentHookConfig(result.ConfigPath, result.Data); err != nil {
39-
return kitagenthook.Result{}, err
40-
}
41-
return result, nil
42-
}
43-
4433
func planGrokInstall(opts InstallOptions) (kitagenthook.Result, error) {
4534
path := strings.TrimSpace(opts.ConfigPath)
4635
if path == "" {

internal/agenthook/install.go

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ import (
44
"errors"
55
"fmt"
66
"io"
7+
"path/filepath"
78
"strings"
89
"time"
910
"unicode"
1011

1112
kitagenthook "go.kenn.io/kit/agenthook"
13+
14+
"go.kenn.io/roborev/internal/skills"
1215
)
1316

1417
const (
@@ -110,24 +113,29 @@ func RunDump(opts DumpOptions, stdout io.Writer) error {
110113
}
111114

112115
func runInstall(agent kitagenthook.Agent, opts InstallOptions) (kitagenthook.Result, error) {
116+
var planned kitagenthook.Result
117+
var err error
113118
if agent == AgentGrok {
114-
return runGrokInstall(opts)
115-
}
116-
kitOpts, err := validatedKitInstallOptions(agent, opts)
117-
if err != nil {
118-
return kitagenthook.Result{}, err
119-
}
120-
planned, err := kitagenthook.PlanInstall(agent, kitOpts)
121-
if err != nil {
122-
return kitagenthook.Result{}, err
119+
planned, err = planGrokInstall(opts)
120+
} else {
121+
var kitOpts kitagenthook.InstallOptions
122+
kitOpts, err = validatedKitInstallOptions(agent, opts)
123+
if err == nil {
124+
planned, err = kitagenthook.PlanInstall(agent, kitOpts)
125+
}
126+
if err == nil {
127+
planned, err = planLegacyHookMigration(agent, planned)
128+
}
123129
}
124-
planned, err = planLegacyHookMigration(agent, planned)
125130
if err != nil {
126131
return kitagenthook.Result{}, err
127132
}
128133
if opts.DryRun {
129134
return planned, nil
130135
}
136+
if err := installAgentHookSkills(agent, planned.ConfigPath); err != nil {
137+
return kitagenthook.Result{}, err
138+
}
131139
if !planned.Changed {
132140
return planned, nil
133141
}
@@ -137,6 +145,31 @@ func runInstall(agent kitagenthook.Agent, opts InstallOptions) (kitagenthook.Res
137145
return planned, nil
138146
}
139147

148+
func installAgentHookSkills(agent kitagenthook.Agent, configPath string) error {
149+
var skillAgent skills.Agent
150+
switch agent {
151+
case kitagenthook.AgentClaude:
152+
skillAgent = skills.AgentClaude
153+
case kitagenthook.AgentCodex:
154+
skillAgent = skills.AgentCodex
155+
case kitagenthook.AgentDroid:
156+
skillAgent = skills.AgentDroid
157+
case AgentGrok:
158+
skillAgent = skills.AgentGrok
159+
default:
160+
return nil
161+
}
162+
163+
configDir := filepath.Dir(configPath)
164+
if agent == AgentGrok && strings.EqualFold(filepath.Base(configDir), "hooks") {
165+
configDir = filepath.Dir(configDir)
166+
}
167+
if _, err := skills.InstallToPath(skillAgent, filepath.Join(configDir, "skills")); err != nil {
168+
return fmt.Errorf("install bundled %s skills: %w", skillAgent, err)
169+
}
170+
return nil
171+
}
172+
140173
func validatedKitInstallOptions(
141174
agent kitagenthook.Agent,
142175
opts InstallOptions,

internal/agenthook/kit_install_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,41 @@ func TestRunInstallUsesKitForQwen(t *testing.T) {
8787
assert.Contains(t, stdout.String(), "installed Qwen Code agent hooks")
8888
}
8989

90+
func TestRunInstallInstallsAndUpdatesBundledSkillsForSupportedProfiles(t *testing.T) {
91+
tests := []struct {
92+
agent string
93+
configName string
94+
}{
95+
{agent: "claude", configName: "settings.json"},
96+
{agent: "codex", configName: "hooks.json"},
97+
{agent: "droid", configName: "hooks.json"},
98+
{agent: "grok", configName: filepath.Join("hooks", "roborev.json")},
99+
}
100+
101+
for _, tt := range tests {
102+
t.Run(tt.agent, func(t *testing.T) {
103+
root := t.TempDir()
104+
configPath := filepath.Join(root, tt.configName)
105+
opts := InstallOptions{
106+
Agent: tt.agent, Executable: "/opt/bin/roborev",
107+
ConfigPath: configPath, Timeout: 10 * time.Second,
108+
}
109+
110+
require.NoError(t, RunInstall(opts, &bytes.Buffer{}))
111+
skillPath := filepath.Join(root, "skills", "roborev-fix", "SKILL.md")
112+
installed, err := os.ReadFile(skillPath)
113+
require.NoError(t, err)
114+
assert.NotEmpty(t, installed)
115+
116+
require.NoError(t, os.WriteFile(skillPath, []byte("stale"), 0o644))
117+
require.NoError(t, RunInstall(opts, &bytes.Buffer{}))
118+
updated, err := os.ReadFile(skillPath)
119+
require.NoError(t, err)
120+
assert.NotEqual(t, []byte("stale"), updated)
121+
})
122+
}
123+
}
124+
90125
func TestRunInstallMigratesLegacyProfileHooks(t *testing.T) {
91126
tests := []struct {
92127
agent string

internal/agenthook/output_test.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,3 @@ func TestPostToolUseAdditionalContextPreservesResolvedInstruction(t *testing.T)
1717
func TestPostToolUseAdditionalContextFallsBackToDefaultInstruction(t *testing.T) {
1818
assert.Equal(t, DefaultInstruction, PostToolUseAdditionalContext(""))
1919
}
20-
21-
func TestDefaultInstructionDefersOutOfScopeFindings(t *testing.T) {
22-
assert := assert.New(t)
23-
instruction := PostToolUseAdditionalContext("")
24-
25-
assert.Contains(instruction, "Never expand the scope of the user's current task")
26-
assert.Contains(instruction, "leave it unchanged and ask the user for direction")
27-
assert.Contains(instruction, "continue the task you were doing before this hook interrupted you")
28-
}

0 commit comments

Comments
 (0)