feat(hooks): one code-host trigger row per subject family

[zfy0701]

What

A kind:"github"/kind:"gitlab" trigger row now covers exactly one subject family — pull_request/merge_request, issues, or push — recorded in a new family column and unique per (agent, kind, repo, family). Watching a repository for both pull requests and issues is two rows, each with its own cadence, label filter, and mentionOnly gate. The motivating configuration now works: PRs trigger on every update while issues summon the agent only on an explicit @mention.

Why the wire and relay are untouched

The relay already fans one delivery out to every rule matching the repository, and commentFamilies already isolates comment traffic per family — so each row compiles into an ordinary independent rule. A dedicated integration test asserts two sibling rows compile into two wire rules carrying their own mentionOnly.

Control plane

• HookDef.family + unique index (agentId, kind, repoId, family); the four "already watches" probes are gone — a duplicate arrives as the constraint violation and answers the same 409, now naming the family.
• New pure per-row shape gate (hooks/hook-family.ts, 400): every stored pattern belongs to the row's family (issue_comment:* rides GitHub thread families, pull_request_review_comment:* rides pull_request); commentFamilies is [] or [family], and a GitHub row with an issue_comment subscription must set it (empty would keep the legacy repo-wide meaning and double-fire against the sibling); review/reporting axes only on change-proposal families.
• Sibling rows of one repo answer the same threads, so they must share the anchoring target and the session-key prefix (409 otherwise; the session key is inherited from an existing sibling).
• family is immutable — the update body does not carry it; changing it is delete + create.

Migration

Idempotent split of legacy rows: the review-capable family keeps the existing row id (review projections, publication leases, and run history all key on it); siblings are new rows with review axes at defaults. A comment-only repo-wide GitHub rule splits into both thread families. Legacy empty commentFamilies (repo-wide comments) is deliberately narrowed to each row's own family — an accepted behavior change; the old width was an accident of the shared issue_comment event. Pre-existing duplicate rows on one (agent, repo) claim free family slots and otherwise stay legacy-inert with family = NULL. A test re-executes the migration SQL against legacy-shaped fixtures.

Console (minimal slice)

• Add integration creates one trigger per selected family and blocks already-watched families instead of whole repositories.
• Agent detail: the family pills are now read-only indicators of the row's family; the per-row trigger selector, review settings (change-proposal rows only), and delete keep working. A repo watched for two families renders as two rows — the grouped per-repo display is a follow-up, together with per-family trigger modes in a single Add-integration pass.

Tests

• control-plane test:unit 180 files / 2068 passed, test:int 114 files / 1804 passed (includes the migration-split and two-sibling wire-compile cases)
• web tests 183 files / 2069 passed; pnpm typecheck, lint, format:check clean

🤖 Generated with Claude Code

[agentconnect-md-test]

Fast pass — checked the pure gate (hook-family.ts), the routes (hooks.ts create/update), the schema/migration split, and the web-side family plumbing (github-events.ts, gitlab-events.ts, AddIntegrationModal.tsx, AgentDetailView.tsx, DeleteHookModal.tsx).

• Unique index (agentId, kind, repoId, family) correctly relies on Postgres NULL-distinct semantics so webhook rows and legacy-inert rows never collide.
• Migration backfill logic is idempotent (WHERE family IS NULL), correctly preserves the review-capable family on the original row id, and matches the documented "claim free slots, else stay legacy-inert" behavior for pre-existing duplicates.
• githubSessionKey/session-key inheritance for sibling rows is consistent between the migration and the route's siblingShapeError/repo.ts sibling lookup.
• Console: AddIntegrationModal correctly computes per-family "already watched" sets (including for null-family legacy rows), gates review/reporting UI to the PR/MR family, and re-mutates on partial create failure in both GitHub and GitLab submit paths.
• DeleteHookModal's repo-name dedup fixes a real pre-existing display bug for the "disconnect all" group case.

No blocking issues found.

_{sent by review-bot-fast (Claude Agent · sonnet) · open in session}

[agentconnect-md-test]

Requesting changes for one reliability regression in the new sibling-family model.

When an agent has both GitHub issue and pull-request rows, an issue_comment delivery is family-scoped by the relay and therefore produces a HookRun for only the row whose subject family actually matched. The control-plane redelivery reconciler, however, sees only GitHub’s summary event and currently treats both sibling rows as expected matches. Its exact-fanout claim then rejects the one landed row and clears the retry schedule, so retryable pre-admission failures on issue/PR comments are no longer automatically redelivered. This directly affects the motivating PR-every / issues-mention-only configuration and should be fixed before merge.

I also left a non-blocking compatibility warning about preserving a destination sibling’s historical session-key prefix when a row is reassigned between agents on the same repository.

Verification: the trusted checkout is the exact base/head merge; git diff --check passed. Focused Vitest could not start because dependencies are absent and the restricted runner’s writable-cache retry could not download the pinned pnpm package (EAI_AGAIN), so that is an environment verification gap rather than a test failure.

_{sent by review-bot (Codex · gpt-5.6-sol) · open in session}

[agentconnect-md-test]

[P1] Preserve exact fanout for comment redelivery when allowing sibling rows

This now permits the common PR+issues sibling pair, but HookRedeliveryReconciler.hookMatchesEvent still marks both rows as candidates for an issue_comment summary: both updated-mode rows carry issue_comment:created, and the summary has no subject-family field with which to apply commentFamilies. The relay sees the full payload and emits a HookRun only for the actual subject family. On a retryable delivery-stage failure, claimRetryableDeliveryRedelivery then compares the two expected hook IDs with the one landed hook ID, refuses the claim, and clears redeliveryNextAttemptAt. The failed comment is therefore never automatically redelivered once the sibling exists. Please make the reconciler/claim contract family-aware (while retaining its duplicate-safety proof) and cover the sibling issue-comment/PR-comment cases.

[zfy0701]

Fixed in abd7252. The retryable claim now requires the landed runs to be a subset of the candidate fanout instead of an exact match: an unlanded candidate is the relay's own precise filtering (subject family, mention text, labels — none of which the delivery summary carries), which the immutable payload reproduces on redelivery, so it no longer blocks or settles the retry schedule. A landed run from outside the candidate set still blocks, every landed row must still be an active side-effect-free retry row, and a candidate hook created after the delivery was ingested keeps blocking (it would read the redelivery as a first run of a stale event). Liveness and dispatch-authority checks narrowed to the landed rows. Covered by new integration cases including the PR-row-lands/issues-row-filtered regression, plus a reconciler-level candidate test.

[agentconnect-md-test]

Verified — the fix in abd7252e3 correctly resolves this. Specifically:

• claimRetryableDeliveryRedelivery now requires landed ⊆ expected (was exact-match), so a landed run for only the matched family (e.g. pull_request) no longer gets rejected because its issues sibling never landed.
• Every landed row must still be an active, side-effect-free retry row; a landed row from outside the candidate set still blocks — no duplicate-safety regression.
• The one exception you carved out is sound: an unlanded candidate hook created after the delivery's earliest landed startedAt still blocks the claim, so a freshly-created sibling can't silently absorb a stale redelivery as if it were its own first run.
• Liveness/dispatch-authority checks (currentHooks, authoritySafe) were correctly narrowed to query landed instead of expected, so an unlanded sibling's enablement/agent-status can't spuriously fail the claim.

The new integration tests exercise exactly the regression scenario (PR row lands, issues row filtered by the relay), plus the postdating-block case, the stray-landed-row-outside-candidates block case, and deleted-unlanded-tolerance — and the reconciler-level test confirms both siblings are still correctly offered as candidates (the summary genuinely can't disambiguate family). This closes the P1 finding.

_{sent by review-bot-fast (Claude Agent · sonnet) · open in session}

[agentconnect-md-test]

The immutable payload is only half the safety proof—the relay rule is mutable.

Example: agent A lands and fails; agent B’s pre-existing same-family hook filters the event via mentionOnly. Before reconciliation, B changes mentionOnly. B is now a candidate, has no original run, and predates the delivery, so the claim succeeds and redelivery starts a stale first run for B.

The creation-time guard therefore needs to also fence post-delivery rule/lifecycle changes, or use immutable evidence of the original fanout.

_{sent by review-bot (Codex · gpt-5.6-sol) · open in session}

[agentconnect-md-test]

Non-blocking compatibility warning: this lookup runs for a repository change, but not for an owner change on the same repository. Reassigning a row from agent A to agent B therefore keeps A’s existing githubSessionKey via the branch below instead of inheriting B’s sibling key. Fresh numeric keys usually coincide, but grandfathered owner/repo prefixes can differ (especially across a rename), splitting B’s sibling families into separate sessions. Consider including existing.agentId !== input.agentId in the condition.

[zfy0701]

Good catch — fixed in abd7252. The sibling lookup now runs whenever either half of the binding moves (repository re-target or agent reassignment), and a destination sibling's key wins over the row's own; a same-repo agent move with no destination sibling keeps its own key, and a repository move still mints the fresh numeric prefix. Test covers both the grandfathered-prefix adoption and the no-sibling continuity branch.

[agentconnect-md-test]

Confirmed — this is accurate and matches what I verified. hook.repo.ts now computes bindingMoved = existing?.repoId !== input.repoId || existing?.agentId !== input.agentId, so the sibling-key lookup runs on either kind of move, not just a repo re-target as before. The precedence is correct:

1. Destination sibling's key wins (siblingKeyRow?.githubSessionKey ?? siblingKeyRow?.repoFullName) when the binding moved and a sibling exists at the destination.
2. Falls back to the row's own prior key (ownSessionKey, gated on existing.repoId === input.repoId) when there's no destination sibling — correctly preserving continuity for a same-repo agent move with no sibling yet.
3. Mints a fresh github:${repoId} only when the repo itself changed and no destination sibling exists.

The added github-review-persistence.repo.test.ts case exercises exactly the two branches called out: adopting a grandfathered acme/legacy prefix from B's existing sibling on reassignment, and a solo same-repo move (no sibling) keeping its own acme/solo key. This closes the warning.

_{sent by review-bot-fast (Claude Agent · sonnet) · open in session}

[agentconnect-md-test]

Confirmed. The destination-sibling precedence and no-sibling continuity behavior are correct, and the new persistence test covers both branches. This resolves the session-key inheritance finding.

_{sent by review-bot (Codex · gpt-5.6-sol) · open in session}

[agentconnect-md-test]

This revision (abd7252) is the targeted fix for the redelivery regression a prior review round flagged: claimRetryableDeliveryRedelivery previously required an exact-fanout match between the expected candidate set and the landed hookRun rows for a GUID, but an issue_comment delivery summary can't distinguish which sibling family (issues vs. pull_request) actually matched, so the relay lands a run for only one of the two candidate rows — the old exact-equality check rejected that landed row and cleared its retry schedule, silently killing redelivery for retryable comment failures once an agent watched both families.

Verified the fix in packages/control-plane/src/persistence/repositories/hook.repo.ts:

• The claim now requires landed ⊆ expected (not landed == expected), with every landed row still an active, side-effect-free retry row, and any landed row outside the candidate set still blocking.
• An unlanded candidate is tolerated (relay-side filtering the redelivered payload reproduces exactly) unless that candidate hook was created after the delivery was ingested (createdAt > deliveredAt), which correctly still blocks — avoids a newly-created sibling silently absorbing a stale redelivery.
• Liveness/dispatch-authority checks and the currentHooks lookup now correctly narrow to landed instead of expected.
• Session-key inheritance on agent reassignment (githubSessionKey) now re-reads the destination's siblings on either a repo re-target or an agent reassignment (previously only repo re-target), with a documented fallback order (destination sibling → own prior key → fresh).

New integration tests directly cover the previously-broken scenario (redelivers a comment failure that landed only on the subject family that matched), the postdating-block case, the stray-landed-row block case, the deleted-unlanded-tolerance case, and the session-key reassignment case (github-review-persistence.repo.test.ts), plus a reconciler-level test confirming both siblings are still offered as candidates. Docs (webhook-triggers-and-github-events.md) updated to match.

Could not execute the test suite locally (no node_modules, sandboxed npm registry access — consistent with the environment limits noted on the prior attempt), so this is a static read of the diff, but the logic is sound, matches its own tests' expectations, and directly closes the previously-reported gap. No new issues found.

_{sent by review-bot-fast (Claude Agent · sonnet) · open in session}

[agentconnect-md-test]

Requesting changes on the synchronized revision. The update fixes both issues from the prior review for the intended steady-state case: relay-filtered family siblings no longer prevent retry, and same-repository agent reassignment now inherits the destination sibling session key.

There is still one blocking redelivery-safety gap. The reconciler rebuilds expectedHookIds from the repository's current enabled hooks and current event patterns, but an expected hook with no original HookRun is fenced only by createdAt. That does not prove the hook's relay filters and lifecycle state are unchanged since the original delivery. For example, two agents can have same-family hooks on one repository: agent A's hook lands and fails retryably, while agent B's pre-existing mentionOnly hook filters the unmentioned event. If B changes mentionOnly before reconciliation, B is still an unlanded candidate created before the delivery, so this claim succeeds and GitHub redelivery starts a stale first run for B. Enabling an older disabled hook has the same shape.

Please preserve historical routing safety for unlanded candidates—such as by fencing any rule/lifecycle/dispatch change after the delivery, or by using immutable evidence of the original candidate fanout—rather than checking only creation time. A focused regression covering a pre-existing filtered candidate changed after delivery would pin the invariant.

I inspected the exact synchronized revision through GitHub because no trusted local checkout was available; I did not rerun the test suite.

_{sent by review-bot (Codex · gpt-5.6-sol) · open in session}

[agentconnect-md-test]

[P1] Fence post-delivery changes on unlanded candidates

createdAt only excludes hooks added after the original delivery; it does not exclude a pre-existing candidate whose relay-facing configuration or enabled/placement state changed afterward. A concrete case is two agents watching the same repo/family: A lands and fails, B was filtered by mentionOnly, then B's setting is relaxed before this sweep. The reconciler builds expectedHookIds from current hooks, B is unlanded but old enough to pass this check, and redelivery gives B a first run for the stale event. Please also prove that every unlanded candidate had the same effective routing rule at original ingestion (or conservatively reject when that cannot be proven).