Skip to content

Commit 02f1fe0

Browse files
mariusvniekerkcodex
andcommitted
fix(agent-hook): keep acknowledged reviews suppressed
Attaching detached work to a branch and then amending it can restore the detached work identity after review IDs have already been filtered against the temporary branch identity. That stale result can present a delivered review as new and restart the automatic reminder loop. Filter reviews only after rewrite handling selects the final work identity, so an acknowledgement follows the work it belongs to. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>
1 parent 382c6c8 commit 02f1fe0

2 files changed

Lines changed: 71 additions & 2 deletions

File tree

internal/agenthook/state.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,6 @@ func (s *StateStore) recordPostToolUse(ctx context.Context, req Request) (Respon
412412
priorLineageKey = st.WorktreeLineageKeys[scope.WorktreeKey]
413413
}
414414
lineageKey := ensureLineageKey(&st, scope)
415-
actionableReviewIDs := unacknowledgedReviewIDs(st, lineageKey, openFailedReviewIDs)
416-
failedReviewCount := len(actionableReviewIDs)
417415
preserveDetachedRewriteLineage := false
418416
if commitCommand && scope.Branch != "" && detachedLineageKey(priorLineageKey) && lineageKey != priorLineageKey {
419417
previousWorktreeHead := st.RepoHeads[scope.WorktreeKey]
@@ -482,6 +480,8 @@ func (s *StateStore) recordPostToolUse(ctx context.Context, req Request) (Respon
482480
st.LastCommitHead = scope.Head
483481
}
484482

483+
actionableReviewIDs := unacknowledgedReviewIDs(st, lineageKey, openFailedReviewIDs)
484+
failedReviewCount := len(actionableReviewIDs)
485485
actionableReviews := hasActionableFailedReviews(failedReviewCount, haveFailedReviewCount)
486486
// The commit reminder fires once this checkout's threshold is met and
487487
// actionable failed reviews exist; it does not require a commit in this exact

internal/agenthook/state_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,75 @@ func TestRecordPostToolUseAmendAfterBranchAttachmentKeepsDetachedCommitThreshold
16471647
assert.Empty(store.sessions["session-1"].CommitSHAsSincePrompt[branchKey])
16481648
}
16491649

1650+
func TestRecordPostToolUseAmendAfterBranchAttachmentDoesNotRepeatAcknowledgedReviews(t *testing.T) {
1651+
assert := assert.New(t)
1652+
repo := testutil.NewGitRepo(t)
1653+
repo.CommitFile("main.go", "package main\n", "initial")
1654+
repo.CheckoutDetached()
1655+
reviewHead := repo.CommitFile("feature-a.go", "package main\n", "detached")
1656+
1657+
closed := false
1658+
verdict := "F"
1659+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1660+
if r.URL.Path == "/api/repos/resolve" {
1661+
assert.NoError(json.NewEncoder(w).Encode(map[string]any{
1662+
"tracked": true,
1663+
"repo": map[string]string{
1664+
"root_path": repo.Path(),
1665+
"name": filepath.Base(repo.Path()),
1666+
},
1667+
}))
1668+
return
1669+
}
1670+
assert.NoError(json.NewEncoder(w).Encode(jobsResponse{
1671+
Jobs: []storage.ReviewJob{{
1672+
ID: 101, Status: storage.JobStatusDone, Closed: &closed, Verdict: &verdict, GitRef: reviewHead,
1673+
}},
1674+
}))
1675+
}))
1676+
t.Cleanup(server.Close)
1677+
1678+
store := &StateStore{path: filepath.Join(t.TempDir(), "state.json"), sessions: map[string]SessionState{}}
1679+
baseReq := Request{
1680+
Event: Input{
1681+
SessionID: "session-1",
1682+
CWD: repo.Path(),
1683+
HookEventName: "PostToolUse",
1684+
ToolName: "Bash",
1685+
ToolInput: map[string]json.RawMessage{"command": json.RawMessage(`"go test ./..."`)},
1686+
},
1687+
CommitThreshold: 1,
1688+
FailedReviewThreshold: 1,
1689+
Instruction: "Resolve reviews.",
1690+
RoborevServerAddr: server.URL,
1691+
}
1692+
1693+
first, err := store.Record(baseReq)
1694+
require.NoError(t, err)
1695+
assert.True(first.Triggered)
1696+
1697+
repo.CheckoutBranchForce("feature/attached")
1698+
checkout := baseReq
1699+
checkout.Event.ToolInput = map[string]json.RawMessage{"command": json.RawMessage(`"git checkout -B feature/attached"`)}
1700+
_, err = store.Record(checkout)
1701+
require.NoError(t, err)
1702+
1703+
repo.CommitFile("feature-b.go", "package main\n", "attached")
1704+
commit := baseReq
1705+
commit.Event.ToolInput = map[string]json.RawMessage{"command": json.RawMessage(`"git commit -m attached"`)}
1706+
atCommit, err := store.Record(commit)
1707+
require.NoError(t, err)
1708+
assert.False(atCommit.Triggered)
1709+
1710+
repo.WriteFile("feature-b.go", "package main\nconst amended = true\n")
1711+
repo.AmendCommit("attached amended", "feature-b.go")
1712+
commit.Event.ToolInput = map[string]json.RawMessage{"command": json.RawMessage(`"git commit --amend -m attached amended"`)}
1713+
atAmend, err := store.Record(commit)
1714+
require.NoError(t, err)
1715+
1716+
assert.False(atAmend.Triggered, "amend must not repeat a review acknowledged before branch attachment")
1717+
}
1718+
16501719
func TestRecordPostToolUseDetachedFailedReviewDedupeScopesByWorktree(t *testing.T) {
16511720
assert := assert.New(t)
16521721
repo := testutil.NewGitRepo(t)

0 commit comments

Comments
 (0)