From f86954ba8f45d1d234a6fd1a7c6afb511cc9dcfa Mon Sep 17 00:00:00 2001 From: baixiaohang Date: Fri, 31 Jul 2026 18:52:29 +0800 Subject: [PATCH] fix: unify SCM audience and echo routing --- .../scm-entity-attention-parity.md | 14 +- .../server/src/__tests__/agent-chats.test.ts | 1 + .../src/__tests__/github-audience.test.ts | 161 ++++++- .../github-binding-invariants.test.ts | 3 + .../__tests__/github-delivery-unit.test.ts | 91 ++-- .../src/__tests__/github-delivery.test.ts | 418 +++++++++++------- .../src/__tests__/github-normalize.test.ts | 20 + .../__tests__/gitlab-identity-fencing.test.ts | 9 +- .../__tests__/gitlab-webhook-stage3.test.ts | 8 +- .../src/__tests__/resolve-target-chat.test.ts | 44 +- .../scm-audience-composition.test.ts | 120 +++++ .../scm-provider-attention-contract.test.ts | 227 +++++++++- .../server/src/services/github-audience.ts | 145 +++--- .../server/src/services/github-delivery.ts | 128 ++---- .../server/src/services/github-entity-chat.ts | 74 ++-- .../server/src/services/github-normalize.ts | 12 +- .../server/src/services/gitlab-webhook.ts | 92 ++-- .../src/services/scm-audience-composition.ts | 106 +++++ .../src/services/scm-chat-delivery-plan.ts | 161 ++++--- 19 files changed, 1300 insertions(+), 534 deletions(-) create mode 100644 packages/server/src/__tests__/scm-audience-composition.test.ts create mode 100644 packages/server/src/services/scm-audience-composition.ts diff --git a/packages/qa/cases/cross-surface/scm-entity-attention-parity.md b/packages/qa/cases/cross-surface/scm-entity-attention-parity.md index 53093ec3f..be0b36c71 100644 --- a/packages/qa/cases/cross-surface/scm-entity-attention-parity.md +++ b/packages/qa/cases/cross-surface/scm-entity-attention-parity.md @@ -40,7 +40,13 @@ card significance, lifecycle projection, topic protection, and archive behavior. removed line. - Exercise reviewer, assignee, and mention targets with equivalent identities. Only reviewer routing may reuse exactly one eligible membership chat without writing a new line; assignee and mention routing establish the target's own line/chat. - Ambiguous membership reuse must fail closed. + Ambiguous membership reuse must fail closed. Also create an agent follow whose stable human carrier is the reviewer but + whose wake agent differs from that human's current delegate. Confirm the carrier line remains intact, the current + delegate is still addressed by exact pair, reviewer membership reuse happens only when both sides already speak in one + candidate chat, and the strict path otherwise creates a separate deliverable line without human-scoped fallback. +- Have a delegate act through its human's provider identity in an entity followed by that human's line. Confirm every + valid routed chat keeps one provider card, the matching existing line does not wake itself, eligible sibling lines still + wake, and a chat with no eligible wake line receives a silent history card instead of losing the event. - Deliver equivalent code updates, draft/ready transitions, description mentions, terminal state changes, and metadata-only updates. Confirm code updates are actionable, ready reviewers are actionable, description mentions route only on open or actual description change, and observation-only/metadata-only activity refreshes title/state without an @@ -62,9 +68,9 @@ card significance, lifecycle projection, topic protection, and archive behavior. unfollow, target-chat, wake, card-significance, lifecycle, topic-protection, header-link, archive, and revive behavior; only the declared protocol exceptions differ. -`FAIL`: either provider permits a silent complete attention line, duplicates a pair across chats, chooses a different -target-chat policy, delivers a semantically noisy card, leaks internal identifiers, overwrites a manual topic, guesses an -entity URL, or archives despite a safety guard. +`FAIL`: either provider permits a complete attention line to become permanently unwakeable, drops an actor-authored card, +duplicates a pair across chats, chooses a different target-chat policy, delivers a semantically noisy card, leaks internal +identifiers, overwrites a manual topic, guesses an entity URL, or archives despite a safety guard. `BLOCKED`: the complete isolated harness cannot receive both providers' disposable webhooks or cannot observe the required Web, CLI, Inbox/session, and archive surfaces. diff --git a/packages/server/src/__tests__/agent-chats.test.ts b/packages/server/src/__tests__/agent-chats.test.ts index 4ca248b5c..d863069da 100644 --- a/packages/server/src/__tests__/agent-chats.test.ts +++ b/packages/server/src/__tests__/agent-chats.test.ts @@ -650,6 +650,7 @@ describe("Agent Chats API", () => { eventType: "pull_request", action: "opened", isMentionMatched: true, + intent: { kind: "strict_new_line" }, }); expect(resolved).not.toBeNull(); if (!resolved) throw new Error("unreachable"); diff --git a/packages/server/src/__tests__/github-audience.test.ts b/packages/server/src/__tests__/github-audience.test.ts index a3804075a..3a5a8a2be 100644 --- a/packages/server/src/__tests__/github-audience.test.ts +++ b/packages/server/src/__tests__/github-audience.test.ts @@ -8,11 +8,13 @@ import { githubEntityChatMappings } from "../db/schema/github-entity-chat-mappin import { organizationSettings } from "../db/schema/organization-settings.js"; import { putContextReviewerAssignment } from "../services/context-reviewer-settings.js"; import { + type GithubProviderTaskContext, isGithubAppTargetLogin, resolveGithubAudience as resolveAudienceResolution, resolveGithubActorHumanId, } from "../services/github-audience.js"; import { putOrgSetting } from "../services/org-settings.js"; +import type { ScmAudienceTarget } from "../services/scm-audience-composition.js"; import { putTeamAgentAssignment } from "../services/team-agent-settings.js"; import { createTestAdmin, seedClient, useTestApp } from "./helpers.js"; @@ -22,7 +24,52 @@ async function resolveAudience( db: Parameters[0], event: Parameters[1], ) { - return (await resolveAudienceResolution(db, event)).targets; + return projectAudienceTargets((await resolveAudienceResolution(db, event)).targets); +} + +function projectAudienceTargets(targets: ScmAudienceTarget[]) { + return targets.map((target) => { + if (target.entry.kind === "existing_line") { + return { + humanAgentId: target.entry.line.humanAgentId, + delegateAgentId: target.entry.line.wakeAgentId, + kind: "existing" as const, + chatId: target.entry.line.chatId, + involveReason: target.directedContext?.reason ?? null, + involveLogin: target.directedContext?.externalUsername ?? null, + provenance: target.entry.line.provenance, + }; + } + if (target.entry.kind === "legacy_route") { + return { + humanAgentId: null, + delegateAgentId: null, + kind: "legacy" as const, + chatId: target.entry.route.chatId, + involveReason: null, + involveLogin: null, + }; + } + if (target.entry.kind === "provider_task_target") { + return { + humanAgentId: target.entry.humanAgentId, + delegateAgentId: target.entry.wakeAgentId, + kind: "new" as const, + chatId: null, + involveReason: target.entry.reason, + involveLogin: target.entry.externalUsername, + teamAgentTask: { agentUuid: target.entry.providerContext.agentUuid }, + }; + } + return { + humanAgentId: target.entry.humanAgentId, + delegateAgentId: target.entry.wakeAgentId, + kind: "new" as const, + chatId: null, + involveReason: target.entry.reason, + involveLogin: target.entry.externalUsername, + }; + }); } async function seedAgent( @@ -272,7 +319,7 @@ describe("resolveAudience", () => { { appSlug: "test-app-slug", appPermissions: { issues: "write" } }, ); - expect(resolution.targets).toEqual([ + expect(projectAudienceTargets(resolution.targets)).toEqual([ expect.objectContaining({ humanAgentId: admin.humanAgentUuid, delegateAgentId: teamAgentUuid, @@ -301,7 +348,7 @@ describe("resolveAudience", () => { { appSlug: "test-app-slug", appPermissions: { issues: "write" } }, ); - expect(resolution.targets).toEqual([]); + expect(projectAudienceTargets(resolution.targets)).toEqual([]); }); it("ignores an unauthorized public text mention while preserving ordinary audience routing", async () => { @@ -323,7 +370,7 @@ describe("resolveAudience", () => { { appSlug: "test-app-slug", appPermissions: { issues: "write" } }, ); - expect(resolution.targets).toEqual([]); + expect(projectAudienceTargets(resolution.targets)).toEqual([]); }); it.each([ @@ -348,7 +395,7 @@ describe("resolveAudience", () => { }), { appSlug: "test-app-slug", appPermissions: permissions }, ); - expect(resolution.targets).toEqual([]); + expect(projectAudienceTargets(resolution.targets)).toEqual([]); expect(resolution.appTaskBlocker).toBe(blocker); }); @@ -369,8 +416,8 @@ describe("resolveAudience", () => { { updatedBy: admin.userId }, ); - const resolveAppAssignment = (projectKey: string) => - resolveAudienceResolution( + const resolveAppAssignment = async (projectKey: string) => { + const resolution = await resolveAudienceResolution( app.db, makeEvent({ orgId: admin.organizationId, @@ -384,6 +431,8 @@ describe("resolveAudience", () => { }), { appSlug: "test-app-slug", appPermissions: { issues: "write" } }, ); + return { ...resolution, targets: projectAudienceTargets(resolution.targets) }; + }; await expect(resolveAppAssignment("OWNER/CONTEXT-TREE")).resolves.toMatchObject({ targets: [ @@ -434,13 +483,15 @@ describe("resolveAudience", () => { { appSlug: "test-app-slug", appPermissions: { issues: "write" } }, ); - expect(resolution.targets).toEqual([ + expect(projectAudienceTargets(resolution.targets)).toEqual([ expect.objectContaining({ delegateAgentId: teamAgentUuid, teamAgentTask: { agentUuid: teamAgentUuid }, }), ]); - expect(resolution.targets).not.toEqual([expect.objectContaining({ delegateAgentId: reviewerUuid })]); + expect(projectAudienceTargets(resolution.targets)).not.toEqual([ + expect.objectContaining({ delegateAgentId: reviewerUuid }), + ]); }); it("returns subscribed mappings (existing) when no fresh involves", async () => { @@ -583,6 +634,67 @@ describe("resolveAudience", () => { ]); }); + it.each([ + [ + { externalUsername: "placeholder", reason: "assigned" as const }, + { externalUsername: "placeholder", reason: "mentioned" as const }, + ], + [ + { externalUsername: "placeholder", reason: "mentioned" as const }, + { externalUsername: "placeholder", reason: "assigned" as const }, + ], + ])("lets shared composition choose mentioned independent of GitHub target order", async (...orderedTargets) => { + const app = getApp(); + const admin = await createTestAdmin(app); + const delegate = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: `priority-delegate-${randomUUID().slice(0, 6)}`, + }); + const humanName = `priority-human-${randomUUID().slice(0, 6)}`; + const human = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: humanName, + delegateMention: delegate, + type: "human", + }); + const chatId = await seedChat(app, admin.organizationId, human); + const entityKey = `owner/repo#${Math.floor(Math.random() * 100_000)}`; + await seedMapping(app, { + orgId: admin.organizationId, + humanId: human, + delegateId: delegate, + entityType: "pull_request", + entityKey, + chatId, + }); + + const audience = await resolveAudience( + app.db, + makeEvent({ + orgId: admin.organizationId, + entityType: "pull_request", + entityKey, + actorLogin: "outsider", + targets: orderedTargets.map((target) => ({ ...target, externalUsername: humanName })), + kind: "opened", + }), + ); + + expect(audience).toEqual([ + { + humanAgentId: human, + delegateAgentId: delegate, + kind: "existing", + chatId, + involveReason: "mentioned", + involveLogin: humanName, + provenance: "identity_target", + }, + ]); + }); + it("target human reuses an existing mapping instead of creating a new delegate route", async () => { const app = getApp(); const admin = await createTestAdmin(app); @@ -749,13 +861,10 @@ describe("resolveAudience", () => { expect(new Set(audience.map((a) => a.chatId))).toEqual(new Set([chatA, chatB])); }); - it("dedups involves by human even when the existing mapping uses a different delegate", async () => { - // Regression for the assignee-creates-new-chat bug. The chat-binding was - // written under (human, delegateA) when the agent first created the - // entity. A later assign webhook arrives with involves=[human]; the human - // is configured with delegateMention=delegateB. Audience must dedup by - // human alone so the involves path does NOT add a sibling `kind: "new"` - // row — the entity is already routed to the existing chat. + it("does not let a same-human carrier line replace the current delegate target", async () => { + // An agent follow may borrow the target human as a stable carrier while a + // different agent owns the wake side. Personnel routing must retain that + // line and append the exact current-delegate target. const app = getApp(); const admin = await createTestAdmin(app); const delegateA = await seedAgent(app, { @@ -798,12 +907,18 @@ describe("resolveAudience", () => { }), ); - expect(audience).toHaveLength(1); + expect(audience).toHaveLength(2); expect(audience[0]?.kind).toBe("existing"); expect(audience[0]?.delegateAgentId).toBe(delegateA); expect(audience[0]?.chatId).toBe(chatId); - expect(audience[0]?.involveReason).toBe("assigned"); - expect(audience[0]?.involveLogin).toBe(humanName.toLowerCase()); + expect(audience[0]?.involveReason).toBeNull(); + expect(audience[1]).toMatchObject({ + kind: "new", + humanAgentId: human, + delegateAgentId: delegateB, + involveReason: "assigned", + involveLogin: humanName.toLowerCase(), + }); }); it("keeps subscribed targets when actor is an unresolved app bot", async () => { @@ -945,7 +1060,7 @@ describe("resolveAudience", () => { kind: "synchronized", }), ); - const audience = resolution.targets; + const audience = projectAudienceTargets(resolution.targets); expect(audience).toHaveLength(2); expect(new Set(audience.map((a) => a.humanAgentId))).toEqual(new Set([human, otherHuman])); expect(resolution.actorHumanId).toBe(human); @@ -1070,7 +1185,7 @@ describe("resolveAudience", () => { expect(resolution.actorHumanId).toBe(creator); expect(resolution.targets).toHaveLength(1); - expect(resolution.targets[0]).toMatchObject({ + expect(projectAudienceTargets(resolution.targets)[0]).toMatchObject({ humanAgentId: creator, kind: "new", involveReason: "assigned", @@ -1121,7 +1236,7 @@ describe("resolveAudience", () => { kind: "opened", }), ); - const audience = resolution.targets; + const audience = projectAudienceTargets(resolution.targets); expect(resolution.actorHumanId).toBe(creator); expect(audience).toHaveLength(2); @@ -1166,7 +1281,7 @@ describe("resolveAudience", () => { kind: "opened", }), ); - const audience = resolution.targets; + const audience = projectAudienceTargets(resolution.targets); expect(resolution.actorHumanId).toBe(creator); expect(audience).toHaveLength(1); diff --git a/packages/server/src/__tests__/github-binding-invariants.test.ts b/packages/server/src/__tests__/github-binding-invariants.test.ts index b6ec04ec6..c0282113b 100644 --- a/packages/server/src/__tests__/github-binding-invariants.test.ts +++ b/packages/server/src/__tests__/github-binding-invariants.test.ts @@ -85,6 +85,7 @@ describe("github binding invariants", () => { eventType: "pull_request", action: "opened", isMentionMatched: false, + intent: { kind: "strict_new_line" }, }); expect(result).toBeNull(); @@ -108,6 +109,7 @@ describe("github binding invariants", () => { eventType: "pull_request", action: "opened", isMentionMatched: true, + intent: { kind: "strict_new_line" }, }); expect(result).not.toBeNull(); @@ -129,6 +131,7 @@ describe("github binding invariants", () => { eventType: "issue_comment", action: "created", isMentionMatched: false, + intent: { kind: "strict_new_line" }, }); expect(result).not.toBeNull(); diff --git a/packages/server/src/__tests__/github-delivery-unit.test.ts b/packages/server/src/__tests__/github-delivery-unit.test.ts index 9fb53f651..85c8e96fc 100644 --- a/packages/server/src/__tests__/github-delivery-unit.test.ts +++ b/packages/server/src/__tests__/github-delivery-unit.test.ts @@ -1,14 +1,16 @@ import type { NormalizedScmEvent } from "@first-tree/shared"; import type { FastifyInstance } from "fastify"; import { afterEach, describe, expect, it, vi } from "vitest"; -import type { AudienceTarget } from "../services/github-audience.js"; +import type { GithubProviderTaskContext } from "../services/github-audience.js"; +import type { ScmAudienceTarget } from "../services/scm-audience-composition.js"; type MockFn = ReturnType; type MockBag = { - findReuseChatForInvolved: MockFn; + decideGithubPersonnelTargetChat: MockFn; refreshGithubChatTopic: MockFn; resolveTargetChat: MockFn; + applyMembershipWrite: MockFn; setEntityTitle: MockFn; sendMessage: MockFn; notifyRecipients: MockFn; @@ -48,27 +50,59 @@ function makeApp(): FastifyInstance { return { db: { id: "db" }, notifier: { id: "notifier" } } as unknown as FastifyInstance; } -function existingTarget(overrides: Partial = {}): AudienceTarget { +type TargetOverrides = { + humanAgentId: string; + delegateAgentId: string; + chatId: string; + involveReason: "review_requested" | "mentioned" | "assigned" | null; + involveLogin: string | null; +}; + +function existingTarget(overrides: Partial = {}): ScmAudienceTarget { + const humanAgentId = overrides.humanAgentId ?? "human-1"; + const delegateAgentId = overrides.delegateAgentId ?? "delegate-1"; + const chatId = overrides.chatId ?? "chat-1"; + const involveReason = overrides.involveReason ?? null; + const involveLogin = overrides.involveLogin ?? null; return { - humanAgentId: "human-1", - delegateAgentId: "delegate-1", - kind: "existing", - chatId: "chat-1", - involveReason: null, - involveLogin: null, - ...overrides, + entry: { + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId, + wakeAgentId: delegateAgentId, + chatId, + provenance: "identity_target", + }, + }, + ...(involveReason && involveLogin + ? { directedContext: { reason: involveReason, externalUsername: involveLogin } } + : {}), }; } -function newTarget(overrides: Partial = {}): AudienceTarget { +function newTarget(overrides: Partial = {}): ScmAudienceTarget { return { - humanAgentId: "human-1", - delegateAgentId: "delegate-1", - kind: "new", - chatId: null, - involveReason: "mentioned", - involveLogin: "alice", - ...overrides, + entry: { + kind: "personnel_target", + humanAgentId: overrides.humanAgentId ?? "human-1", + wakeAgentId: overrides.delegateAgentId ?? "delegate-1", + reason: overrides.involveReason ?? "mentioned", + externalUsername: overrides.involveLogin ?? "alice", + }, + }; +} + +function providerTaskTarget(): ScmAudienceTarget { + return { + entry: { + kind: "provider_task_target", + humanAgentId: "human-task", + wakeAgentId: "delegate-task", + reason: "mentioned", + externalUsername: "test-app-slug", + providerContext: { kind: "github_app_task", agentUuid: "delegate-task" }, + }, }; } @@ -79,9 +113,10 @@ async function loadDelivery(overrides: Partial = {}): Promise<{ vi.resetModules(); const mocks: MockBag = { - findReuseChatForInvolved: vi.fn(async () => null), + decideGithubPersonnelTargetChat: vi.fn(async () => ({ kind: "strict_new_line" })), refreshGithubChatTopic: vi.fn(async () => undefined), resolveTargetChat: vi.fn(async () => ({ chatId: "chat-created", created: true, boundVia: "direct" })), + applyMembershipWrite: vi.fn(async () => undefined), setEntityTitle: vi.fn(async () => undefined), sendMessage: vi.fn(async () => ({ message: { id: "message-1" }, recipients: ["recipient-1"] })), notifyRecipients: vi.fn(), @@ -89,13 +124,16 @@ async function loadDelivery(overrides: Partial = {}): Promise<{ }; vi.doMock("../services/github-entity-chat.js", () => ({ - findReuseChatForInvolved: mocks.findReuseChatForInvolved, + decideGithubPersonnelTargetChat: mocks.decideGithubPersonnelTargetChat, refreshGithubChatTopic: mocks.refreshGithubChatTopic, resolveTargetChat: mocks.resolveTargetChat, })); vi.doMock("../services/github-entity-state.js", () => ({ setEntityTitle: mocks.setEntityTitle, })); + vi.doMock("../services/participant-mode.js", () => ({ + applyMembershipWrite: mocks.applyMembershipWrite, + })); vi.doMock("../services/message.js", () => ({ sendMessage: mocks.sendMessage, })); @@ -110,6 +148,7 @@ async function loadDelivery(overrides: Partial = {}): Promise<{ afterEach(() => { vi.doUnmock("../services/github-entity-chat.js"); vi.doUnmock("../services/github-entity-state.js"); + vi.doUnmock("../services/participant-mode.js"); vi.doUnmock("../services/message.js"); vi.doUnmock("../services/notifier.js"); vi.resetModules(); @@ -198,17 +237,9 @@ describe("deliverGithubEvent dependency edge paths", () => { event.entity.url = undefined; event.surface.url = "https://github.com/owner/repo/pull/1"; - const stats = await deliverGithubEvent(makeApp(), event, [ - existingTarget({ - humanAgentId: "human-task", - delegateAgentId: "delegate-task", - involveReason: "mentioned", - involveLogin: "test-app-slug", - teamAgentTask: { agentUuid: "delegate-task" }, - }), - ]); + const stats = await deliverGithubEvent(makeApp(), event, [providerTaskTarget()]); - expect(stats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + expect(stats).toEqual({ delivered: 1, newChats: 1, failed: 0 }); expect(sentPayloads).toHaveLength(1); expect(sentPayloads[0]).toMatchObject({ content: { diff --git a/packages/server/src/__tests__/github-delivery.test.ts b/packages/server/src/__tests__/github-delivery.test.ts index 25f3defde..f39683847 100644 --- a/packages/server/src/__tests__/github-delivery.test.ts +++ b/packages/server/src/__tests__/github-delivery.test.ts @@ -10,12 +10,55 @@ import { inboxEntries } from "../db/schema/inbox-entries.js"; import { members } from "../db/schema/members.js"; import { messages } from "../db/schema/messages.js"; import { users } from "../db/schema/users.js"; -import { type AudienceTarget, resolveGithubAudience } from "../services/github-audience.js"; +import { type GithubProviderTaskContext, resolveGithubAudience } from "../services/github-audience.js"; import { deliverGithubEvent } from "../services/github-delivery.js"; import { resolveAgentScmBindingPair } from "../services/scm-attention-line.js"; +import type { ScmAudienceTarget } from "../services/scm-audience-composition.js"; import { createTestAdmin, useTestApp } from "./helpers.js"; type App = ReturnType>; +type GithubAudienceTarget = ScmAudienceTarget; + +function existingTarget(input: { + humanAgentId: string; + wakeAgentId: string; + chatId: string; + reason?: "review_requested" | "mentioned" | "assigned"; + externalUsername?: string; +}): GithubAudienceTarget { + return { + entry: { + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: input.humanAgentId, + wakeAgentId: input.wakeAgentId, + chatId: input.chatId, + provenance: "identity_target", + }, + }, + ...(input.reason && input.externalUsername + ? { directedContext: { reason: input.reason, externalUsername: input.externalUsername } } + : {}), + }; +} + +function personnelTarget(input: { + humanAgentId: string; + wakeAgentId: string; + reason: "review_requested" | "mentioned" | "assigned"; + externalUsername: string; +}): GithubAudienceTarget { + return { + entry: { + kind: "personnel_target", + reason: input.reason, + humanAgentId: input.humanAgentId, + wakeAgentId: input.wakeAgentId, + externalUsername: input.externalUsername, + }, + }; +} async function seedAgent( app: App, @@ -169,14 +212,7 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -240,14 +276,7 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -270,11 +299,11 @@ describe("deliverGithubEvent", () => { expect(delegateEntries).toHaveLength(0); }); - it("echo pruning: drops a self-only delivery before writing a card", async () => { + it("actor echo keeps a self-only card route while suppressing wake", async () => { // The delegate is a live speaker of the bound chat, so an unresolved actor // would normally wake it. When the GitHub actor resolves to the same human - // that owns this mapping, delivery prunes that entry before send: no card, - // no inbox row, and no wake. + // that owns this mapping, delivery keeps the shared history card while + // suppressing the matching line's wake. const app = getApp(); const admin = await createTestAdmin(app); const delegate = await seedAgent(app, { @@ -304,14 +333,7 @@ describe("deliverGithubEvent", () => { chatId, boundVia: "direct", }); - const baseTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - } satisfies AudienceTarget; + const baseTarget = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -319,9 +341,14 @@ describe("deliverGithubEvent", () => { }); const echoStats = await deliverGithubEvent(app, event, [baseTarget], { actorHumanId: human }); - expect(echoStats).toEqual({ delivered: 0, newChats: 0, failed: 0 }); - await expect(app.db.select().from(messages).where(eq(messages.chatId, chatId))).resolves.toHaveLength(0); - await expect(app.db.select().from(inboxEntries).where(eq(inboxEntries.chatId, chatId))).resolves.toHaveLength(0); + expect(echoStats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + await expect(app.db.select().from(messages).where(eq(messages.chatId, chatId))).resolves.toHaveLength(1); + await expect( + app.db + .select() + .from(inboxEntries) + .where(and(eq(inboxEntries.chatId, chatId), eq(inboxEntries.notify, true))), + ).resolves.toHaveLength(0); // Unknown actor: no self pruning, so the speaker-delegate IS woken. const okStats = await deliverGithubEvent(app, event, [baseTarget], { actorHumanId: null }); @@ -355,14 +382,12 @@ describe("deliverGithubEvent", () => { type: "human", }); const entityKey = "owner/repo#1536"; - const target = { + const target = personnelTarget({ humanAgentId: human, - delegateAgentId: delegate, - kind: "new", - chatId: null, - involveReason: "assigned", - involveLogin: humanName.toLowerCase(), - } satisfies AudienceTarget; + wakeAgentId: delegate, + reason: "assigned", + externalUsername: humanName.toLowerCase(), + }); const event = makeEvent({ orgId: admin.organizationId, entityType: "issue", @@ -439,7 +464,9 @@ describe("deliverGithubEvent", () => { // self-directed involve. expect(resolution.actorHumanId).toBe(human); expect(resolution.targets).toHaveLength(1); - expect(resolution.targets[0]).toMatchObject({ humanAgentId: human, kind: "new", involveReason: "assigned" }); + expect(resolution.targets[0]).toMatchObject({ + entry: { kind: "personnel_target", humanAgentId: human, reason: "assigned" }, + }); const stats = await deliverGithubEvent(app, event, resolution.targets, { actorHumanId: resolution.actorHumanId, @@ -464,12 +491,12 @@ describe("deliverGithubEvent", () => { } }); - it("self-echo boundary: a self-assign on an already-bound entity stays pruned (#1536)", async () => { + it("self-echo boundary: an already-bound self-assign keeps a silent card (#1536)", async () => { // The other side of the #1536 carve-out. When the entity ALREADY has a // bound chat (`kind: "existing"`), a self-directed involve is a true echo // of the actor's own action into a chat they already sit in — nothing new - // to create — so it must stay pruned even though it carries an - // `involveReason`. Locks the carve-out to `kind: "new"` only. + // to create — so its wake is suppressed even though it carries directed + // context. The shared activity card still remains in chat history. const app = getApp(); const admin = await createTestAdmin(app); const delegate = await seedAgent(app, { @@ -500,14 +527,13 @@ describe("deliverGithubEvent", () => { chatId, boundVia: "direct", }); - const target = { + const target = existingTarget({ humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", + wakeAgentId: delegate, chatId, - involveReason: "assigned", - involveLogin: humanName.toLowerCase(), - } satisfies AudienceTarget; + reason: "assigned", + externalUsername: humanName.toLowerCase(), + }); const event = makeEvent({ orgId: admin.organizationId, entityType: "issue", @@ -519,12 +545,17 @@ describe("deliverGithubEvent", () => { }); const stats = await deliverGithubEvent(app, event, [target], { actorHumanId: human }); - expect(stats).toEqual({ delivered: 0, newChats: 0, failed: 0 }); - await expect(app.db.select().from(messages).where(eq(messages.chatId, chatId))).resolves.toHaveLength(0); - await expect(app.db.select().from(inboxEntries).where(eq(inboxEntries.chatId, chatId))).resolves.toHaveLength(0); + expect(stats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + await expect(app.db.select().from(messages).where(eq(messages.chatId, chatId))).resolves.toHaveLength(1); + await expect( + app.db + .select() + .from(inboxEntries) + .where(and(eq(inboxEntries.chatId, chatId), eq(inboxEntries.notify, true))), + ).resolves.toHaveLength(0); }); - it("humanA requesting humanB review prunes humanA's follow delivery and wakes humanB's delegate", async () => { + it("humanA requesting humanB review keeps humanA's card route and wakes humanB's delegate", async () => { const app = getApp(); const admin = await createTestAdmin(app); const delegateA = await seedAgent(app, { @@ -585,8 +616,8 @@ describe("deliverGithubEvent", () => { const stats = await deliverGithubEvent(app, event, resolution.targets, { actorHumanId: resolution.actorHumanId, }); - expect(stats).toEqual({ delivered: 1, newChats: 1, failed: 0 }); - expect(await app.db.select().from(messages).where(eq(messages.chatId, chatA))).toHaveLength(0); + expect(stats).toEqual({ delivered: 2, newChats: 1, failed: 0 }); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatA))).toHaveLength(1); expect(await notifyCount(app, chatA, delegateA)).toBe(0); const [mappingB] = await app.db @@ -607,7 +638,7 @@ describe("deliverGithubEvent", () => { expect(messageB?.senderId).toBe(humanB); }); - it("reviewer comment prunes the reviewer's own follow chat but not the PR author's chat", async () => { + it("reviewer comment stays visible in both chats but does not self-wake the reviewer", async () => { const app = getApp(); const admin = await createTestAdmin(app); const delegateA = await seedAgent(app, { @@ -683,9 +714,9 @@ describe("deliverGithubEvent", () => { actorHumanId: resolution.actorHumanId, }); - expect(stats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + expect(stats).toEqual({ delivered: 2, newChats: 0, failed: 0 }); expect(await app.db.select().from(messages).where(eq(messages.chatId, chatA))).toHaveLength(1); - expect(await app.db.select().from(messages).where(eq(messages.chatId, chatB))).toHaveLength(0); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatB))).toHaveLength(1); expect(await notifyCount(app, chatA, delegateA)).toBe(1); expect(await notifyCount(app, chatB, delegateB)).toBe(0); }); @@ -990,14 +1021,7 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1045,14 +1069,7 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1077,7 +1094,7 @@ describe("deliverGithubEvent", () => { expect(mapping?.title).toBe("Renamed title"); }); - it("refreshes entity projection even when self-echo pruning drops the card", async () => { + it("refreshes entity projection while self-echo delivery keeps a silent card", async () => { const app = getApp(); const admin = await createTestAdmin(app); const delegate = await seedAgent(app, { @@ -1128,9 +1145,14 @@ describe("deliverGithubEvent", () => { const stats = await deliverGithubEvent(app, event, resolution.targets, { actorHumanId: resolution.actorHumanId, }); - expect(stats).toEqual({ delivered: 0, newChats: 0, failed: 0 }); - expect(await app.db.select().from(messages).where(eq(messages.chatId, chatId))).toHaveLength(0); - expect(await app.db.select().from(inboxEntries).where(eq(inboxEntries.chatId, chatId))).toHaveLength(0); + expect(stats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatId))).toHaveLength(1); + expect( + await app.db + .select() + .from(inboxEntries) + .where(and(eq(inboxEntries.chatId, chatId), eq(inboxEntries.notify, true))), + ).toHaveLength(0); const [chat] = await app.db.select({ topic: chats.topic }).from(chats).where(eq(chats.id, chatId)).limit(1); expect(chat?.topic).toBe("PR repo#209: New title"); @@ -1189,14 +1211,7 @@ describe("deliverGithubEvent", () => { }, ]); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1234,14 +1249,7 @@ describe("deliverGithubEvent", () => { topic: "agent-chosen label", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1270,14 +1278,12 @@ describe("deliverGithubEvent", () => { type: "human", }); - const target: AudienceTarget = { + const target = personnelTarget({ humanAgentId: human, - delegateAgentId: delegate, - kind: "new", - chatId: null, - involveReason: "review_requested", - involveLogin: humanName.toLowerCase(), - }; + wakeAgentId: delegate, + reason: "review_requested", + externalUsername: humanName.toLowerCase(), + }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1345,22 +1351,12 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const broken: AudienceTarget = { + const broken = existingTarget({ humanAgentId: goodHuman, - delegateAgentId: delegate, - kind: "existing", - chatId: null, // forces the runtime guard to throw - involveReason: null, - involveLogin: null, - }; - const ok: AudienceTarget = { - humanAgentId: goodHuman, - delegateAgentId: delegate, - kind: "existing", - chatId: goodChatId, - involveReason: null, - involveLogin: null, - }; + wakeAgentId: delegate, + chatId: "missing-chat", + }); + const ok = existingTarget({ humanAgentId: goodHuman, wakeAgentId: delegate, chatId: goodChatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1428,22 +1424,13 @@ describe("deliverGithubEvent", () => { }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", entityKey: "owner/repo#210" }); - const subscribed: AudienceTarget = { - humanAgentId: humanA, - delegateAgentId: delegateA, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; - const involved: AudienceTarget = { + const subscribed = existingTarget({ humanAgentId: humanA, wakeAgentId: delegateA, chatId }); + const involved = personnelTarget({ humanAgentId: humanR, - delegateAgentId: delegateR, - kind: "new", - chatId: null, - involveReason: "review_requested", - involveLogin: "humanr", - }; + wakeAgentId: delegateR, + reason: "review_requested", + externalUsername: "humanr", + }); const stats = await deliverGithubEvent(app, event, [subscribed, involved]); @@ -1483,6 +1470,146 @@ describe("deliverGithubEvent", () => { expect(mappings).toHaveLength(1); }); + it("routes a carrier-owned review request to the current delegate already in the same chat", async () => { + const app = getApp(); + const admin = await createTestAdmin(app); + const follower = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: `carrier-follower-${randomUUID().slice(0, 6)}`, + }); + const currentDelegate = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: `current-delegate-${randomUUID().slice(0, 6)}`, + }); + const humanName = `carrier-human-${randomUUID().slice(0, 6)}`; + const human = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: humanName, + delegateMention: currentDelegate, + type: "human", + }); + const chatId = `chat_${randomUUID()}`; + await app.db.insert(chats).values({ id: chatId, organizationId: admin.organizationId, type: "group" }); + await app.db.insert(chatMembership).values( + [human, follower, currentDelegate].map((agentId, index) => ({ + chatId, + agentId, + role: index === 0 ? "owner" : "member", + accessMode: "speaker" as const, + mode: "full" as const, + source: "manual" as const, + })), + ); + await app.db.insert(githubEntityChatMappings).values({ + organizationId: admin.organizationId, + humanAgentId: human, + delegateAgentId: follower, + entityType: "pull_request", + entityKey: "owner/repo#212", + chatId, + boundVia: "agent_declared", + }); + const event = makeEvent({ + orgId: admin.organizationId, + entityType: "pull_request", + entityKey: "owner/repo#212", + actorLogin: "review-requester", + targets: [{ externalUsername: humanName, reason: "review_requested" }], + kind: "review_requested", + action: "review_requested", + }); + + const resolution = await resolveGithubAudience(app.db, event); + expect(resolution.targets.map((target) => target.entry.kind)).toEqual(["existing_line", "personnel_target"]); + const stats = await deliverGithubEvent(app, event, resolution.targets, { + actorHumanId: resolution.actorHumanId, + }); + + expect(stats).toEqual({ delivered: 1, newChats: 0, failed: 0 }); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatId))).toHaveLength(1); + expect(await notifyCount(app, chatId, follower)).toBe(1); + expect(await notifyCount(app, chatId, currentDelegate)).toBe(1); + const mappings = await app.db + .select() + .from(githubEntityChatMappings) + .where(eq(githubEntityChatMappings.entityKey, "owner/repo#212")); + expect(mappings).toHaveLength(1); + expect(mappings[0]?.delegateAgentId).toBe(follower); + }); + + it("creates a strict review line when the current delegate is absent from the carrier chat", async () => { + const app = getApp(); + const admin = await createTestAdmin(app); + const follower = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: `carrier-follower-${randomUUID().slice(0, 6)}`, + }); + const currentDelegate = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: `current-delegate-${randomUUID().slice(0, 6)}`, + }); + const humanName = `carrier-human-${randomUUID().slice(0, 6)}`; + const human = await seedAgent(app, { + orgId: admin.organizationId, + memberId: admin.memberId, + name: humanName, + delegateMention: currentDelegate, + type: "human", + }); + const carrierChatId = `chat_${randomUUID()}`; + await app.db.insert(chats).values({ id: carrierChatId, organizationId: admin.organizationId, type: "group" }); + await app.db.insert(chatMembership).values([ + { chatId: carrierChatId, agentId: human, role: "owner", accessMode: "speaker", mode: "full", source: "manual" }, + { + chatId: carrierChatId, + agentId: follower, + role: "member", + accessMode: "speaker", + mode: "full", + source: "manual", + }, + ]); + await app.db.insert(githubEntityChatMappings).values({ + organizationId: admin.organizationId, + humanAgentId: human, + delegateAgentId: follower, + entityType: "pull_request", + entityKey: "owner/repo#213", + chatId: carrierChatId, + boundVia: "agent_declared", + }); + const event = makeEvent({ + orgId: admin.organizationId, + entityType: "pull_request", + entityKey: "owner/repo#213", + actorLogin: "review-requester", + targets: [{ externalUsername: humanName, reason: "review_requested" }], + kind: "review_requested", + action: "review_requested", + }); + + const resolution = await resolveGithubAudience(app.db, event); + const stats = await deliverGithubEvent(app, event, resolution.targets, { + actorHumanId: resolution.actorHumanId, + }); + + expect(stats).toEqual({ delivered: 2, newChats: 1, failed: 0 }); + const mappings = await app.db + .select() + .from(githubEntityChatMappings) + .where(eq(githubEntityChatMappings.entityKey, "owner/repo#213")); + expect(mappings).toHaveLength(2); + const strictLine = mappings.find((row) => row.delegateAgentId === currentDelegate); + expect(strictLine?.chatId).not.toBe(carrierChatId); + expect(strictLine?.boundVia).toBe("direct"); + expect(await notifyCount(app, strictLine?.chatId ?? "", currentDelegate)).toBe(1); + }); + it("a `mentioned` involve does NOT reuse the entity chat — it mints a fresh chat (S5, reuse is review_requested-only)", async () => { // S9 reuse is scoped to review_requested. An @mention of a human who is // already a speaker of the entity's bound chat must still pierce into a @@ -1538,14 +1665,12 @@ describe("deliverGithubEvent", () => { }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", entityKey: "owner/repo#211" }); - const involvedMention: AudienceTarget = { + const involvedMention = personnelTarget({ humanAgentId: humanM, - delegateAgentId: delegateM, - kind: "new", - chatId: null, - involveReason: "mentioned", - involveLogin: "humanm", - }; + wakeAgentId: delegateM, + reason: "mentioned", + externalUsername: "humanm", + }); const stats = await deliverGithubEvent(app, event, [involvedMention]); @@ -1608,14 +1733,7 @@ describe("deliverGithubEvent", () => { boundVia: "direct", }); - const target: AudienceTarget = { - humanAgentId: human, - delegateAgentId: delegate, - kind: "existing", - chatId, - involveReason: null, - involveLogin: null, - }; + const target = existingTarget({ humanAgentId: human, wakeAgentId: delegate, chatId }); const event = makeEvent({ orgId: admin.organizationId, entityType: "pull_request", @@ -1720,13 +1838,15 @@ describe("deliverGithubEvent", () => { const resolution = await resolveGithubAudience(app.db, event); const audience = resolution.targets; expect(audience).toHaveLength(2); - expect(new Set(audience.map((a) => a.chatId))).toEqual(new Set([chatA, chatB])); + expect( + new Set(audience.flatMap((target) => (target.entry.kind === "existing_line" ? [target.entry.line.chatId] : []))), + ).toEqual(new Set([chatA, chatB])); expect(resolution.actorHumanId).toBe(human); const stats = await deliverGithubEvent(app, event, audience, { actorHumanId: resolution.actorHumanId }); - expect(stats).toEqual({ delivered: 0, newChats: 0, failed: 0 }); - expect(await app.db.select().from(messages).where(eq(messages.chatId, chatA))).toHaveLength(0); - expect(await app.db.select().from(messages).where(eq(messages.chatId, chatB))).toHaveLength(0); + expect(stats).toEqual({ delivered: 2, newChats: 0, failed: 0 }); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatA))).toHaveLength(1); + expect(await app.db.select().from(messages).where(eq(messages.chatId, chatB))).toHaveLength(1); expect(await app.db.select().from(inboxEntries).where(eq(inboxEntries.notify, true))).toHaveLength(0); }); }); diff --git a/packages/server/src/__tests__/github-normalize.test.ts b/packages/server/src/__tests__/github-normalize.test.ts index 9fba96a61..f589611cd 100644 --- a/packages/server/src/__tests__/github-normalize.test.ts +++ b/packages/server/src/__tests__/github-normalize.test.ts @@ -357,6 +357,26 @@ describe("normalizeGithubEvent — pull_request", () => { expect(event.surface.title).toBe("PR #10: Refactor inbox"); }); + it("keeps the same login's assigned and mentioned evidence for shared composition", () => { + const event = normalize("pull_request", { + action: "opened", + sender: senderUser, + repository, + pull_request: { + number: 11, + title: "Route shared priority", + html_url: "https://github.com/owner/repo/pull/11", + body: "Please check this @Dave", + assignees: [{ login: "DAVE" }, { login: "dave" }], + }, + }); + + expect(event?.targets).toEqual([ + { externalUsername: "dave", reason: "assigned" }, + { externalUsername: "dave", reason: "mentioned" }, + ]); + }); + it("synchronize: kind=synchronized with empty involves (Bug 1: no longer silenced)", () => { const event = normalize("pull_request", { action: "synchronize", diff --git a/packages/server/src/__tests__/gitlab-identity-fencing.test.ts b/packages/server/src/__tests__/gitlab-identity-fencing.test.ts index e2771aee4..614f5202f 100644 --- a/packages/server/src/__tests__/gitlab-identity-fencing.test.ts +++ b/packages/server/src/__tests__/gitlab-identity-fencing.test.ts @@ -5,6 +5,7 @@ import type { Database } from "../db/connection.js"; import { agents } from "../db/schema/agents.js"; import { gitlabEntityChatMappings } from "../db/schema/gitlab-entity-chat-mappings.js"; import { gitlabIdentityLinks } from "../db/schema/gitlab-identity-links.js"; +import { inboxEntries } from "../db/schema/inbox-entries.js"; import { members } from "../db/schema/members.js"; import { messages } from "../db/schema/messages.js"; import { createAgent, suspendAgent, updateAgent } from "../services/agent.js"; @@ -230,6 +231,12 @@ describe("GitLab identity authority fencing", () => { .select() .from(messages) .where(and(eq(messages.chatId, chat.id), eq(messages.source, "gitlab"))), + ).toHaveLength(1); + expect( + await app.db + .select() + .from(inboxEntries) + .where(and(eq(inboxEntries.chatId, chat.id), eq(inboxEntries.notify, true))), ).toHaveLength(0); const response = await app.inject({ @@ -244,7 +251,7 @@ describe("GitLab identity authority fencing", () => { .select() .from(messages) .where(and(eq(messages.chatId, chat.id), eq(messages.source, "gitlab"))), - ).toHaveLength(1); + ).toHaveLength(2); }, 20_000); it.each([ diff --git a/packages/server/src/__tests__/gitlab-webhook-stage3.test.ts b/packages/server/src/__tests__/gitlab-webhook-stage3.test.ts index 690397c62..e4ef169bf 100644 --- a/packages/server/src/__tests__/gitlab-webhook-stage3.test.ts +++ b/packages/server/src/__tests__/gitlab-webhook-stage3.test.ts @@ -1203,7 +1203,7 @@ describe("GitLab Stage 3 personnel routing", () => { it.each([ { boundVia: "human_declared" as const, declaredBy: "human" as const }, { boundVia: "agent_declared" as const, declaredBy: "agent" as const }, - ])("prunes actor echo from an explicit $boundVia follow", async ({ boundVia, declaredBy }) => { + ])("keeps actor echo card silent for an explicit $boundVia follow", async ({ boundVia, declaredBy }) => { const app = getApp(); const setup = await setupTarget(app); const iid = declaredBy === "human" ? 61 : 62; @@ -1236,6 +1236,12 @@ describe("GitLab Stage 3 personnel routing", () => { .select() .from(messages) .where(and(eq(messages.chatId, chat.id), eq(messages.source, "gitlab"))), + ).toHaveLength(1); + expect( + await app.db + .select() + .from(inboxEntries) + .where(and(eq(inboxEntries.chatId, chat.id), eq(inboxEntries.notify, true))), ).toHaveLength(0); }); diff --git a/packages/server/src/__tests__/resolve-target-chat.test.ts b/packages/server/src/__tests__/resolve-target-chat.test.ts index f25986ede..ea50bbc44 100644 --- a/packages/server/src/__tests__/resolve-target-chat.test.ts +++ b/packages/server/src/__tests__/resolve-target-chat.test.ts @@ -17,13 +17,15 @@ import { createTestAdmin, useTestApp } from "./helpers.js"; */ async function resolveTargetChat( db: Parameters[0], - params: Omit[1], "isMentionMatched"> & { + params: Omit[1], "isMentionMatched" | "intent"> & { isMentionMatched?: boolean; + intent?: Parameters[1]["intent"]; }, ): Promise>>> { const result = await resolveTargetChatRaw(db, { ...params, isMentionMatched: params.isMentionMatched ?? true, + intent: params.intent ?? { kind: "strict_new_line" }, }); if (!result) throw new Error("resolveTargetChat returned null in legacy test path"); return result; @@ -410,7 +412,7 @@ describe("resolveTargetChat", () => { expect(mappings[0]?.chatId).toBe(r1.chatId); }); - it("reuses the existing chat when a different delegate hits the same (human, entity) tuple", async () => { + it("keeps provider-task human fallback when a different agent hits the same (human, entity) tuple", async () => { // Regression for the assignee-creates-new-chat bug. The agent that // created the issue (delegateA) wrote a mapping under (human, delegateA). // A later webhook resolves the audience via `human.delegateMention = @@ -440,6 +442,7 @@ describe("resolveTargetChat", () => { relatedEntities: [], eventType: "issues", action: "assigned", + intent: { kind: "provider_task_target" }, }); expect(followUp.chatId).toBe(created.chatId); @@ -466,6 +469,43 @@ describe("resolveTargetChat", () => { expect(allChats).toHaveLength(1); }); + it("executes strict personnel intent without human-scoped fallback", async () => { + const app = getApp(); + const admin = await createTestAdmin(app); + const delegateA = await seedDelegate(app, admin.organizationId, admin.memberId, `dlgA-${randomUUID().slice(0, 6)}`); + const delegateB = await seedDelegate(app, admin.organizationId, admin.memberId, `dlgB-${randomUUID().slice(0, 6)}`); + const carrierLine = await resolveTargetChat(app.db, { + organizationId: admin.organizationId, + humanAgentId: admin.humanAgentUuid, + delegateAgentId: delegateA, + entity: issue42, + relatedEntities: [], + eventType: "issues", + action: "opened", + }); + + const strict = await resolveTargetChatRaw(app.db, { + organizationId: admin.organizationId, + humanAgentId: admin.humanAgentUuid, + delegateAgentId: delegateB, + entity: issue42, + relatedEntities: [], + eventType: "issues", + action: "assigned", + isMentionMatched: true, + intent: { kind: "strict_new_line" }, + }); + + expect(strict?.chatId).not.toBe(carrierLine.chatId); + expect(strict?.boundVia).toBe("direct"); + const mappings = await app.db + .select() + .from(githubEntityChatMappings) + .where(eq(githubEntityChatMappings.entityKey, issue42.key)); + expect(mappings).toHaveLength(2); + expect(mappings.find((row) => row.delegateAgentId === delegateB)?.boundVia).toBe("direct"); + }); + it("renders 'PR Review' topic when a PR chat is first created by review_requested", async () => { const app = getApp(); const admin = await createTestAdmin(app); diff --git a/packages/server/src/__tests__/scm-audience-composition.test.ts b/packages/server/src/__tests__/scm-audience-composition.test.ts new file mode 100644 index 000000000..2d430ca3b --- /dev/null +++ b/packages/server/src/__tests__/scm-audience-composition.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "vitest"; +import { composeScmAudience, type ScmProviderTaskTarget } from "../services/scm-audience-composition.js"; + +function existing(humanAgentId: string, wakeAgentId: string, chatId: string) { + return { + kind: "existing_line" as const, + line: { + kind: "attention_line" as const, + humanAgentId, + wakeAgentId, + chatId, + provenance: "explicit" as const, + }, + }; +} + +function personnel( + humanAgentId: string, + wakeAgentId: string, + reason: "review_requested" | "mentioned" | "assigned" = "review_requested", +) { + return { + kind: "personnel_target" as const, + humanAgentId, + wakeAgentId, + reason, + externalUsername: humanAgentId, + }; +} + +describe("composeScmAudience", () => { + it("matches personnel context only to the exact human/wake pair", () => { + const composed = composeScmAudience({ + existingEntries: [existing("human-h", "agent-a", "chat-a"), existing("human-h", "agent-b", "chat-b")], + personnelTargets: [personnel("human-h", "agent-b")], + }); + + expect(composed).toHaveLength(2); + expect(composed[0]).toEqual({ entry: existing("human-h", "agent-a", "chat-a") }); + expect(composed[1]).toEqual({ + entry: existing("human-h", "agent-b", "chat-b"), + directedContext: { reason: "review_requested", externalUsername: "human-h" }, + }); + }); + + it("preserves a carrier line and appends a same-human different-wake personnel target", () => { + const composed = composeScmAudience({ + existingEntries: [existing("human-h", "agent-a", "chat-a")], + personnelTargets: [personnel("human-h", "agent-b")], + }); + + expect(composed).toEqual([ + { entry: existing("human-h", "agent-a", "chat-a") }, + { entry: personnel("human-h", "agent-b") }, + ]); + }); + + it("keeps distinct chats and never treats a legacy route as personnel authority", () => { + const legacy = { + kind: "legacy_route" as const, + route: { + kind: "legacy_route_only" as const, + chatId: "legacy-chat", + senderAgentId: "legacy-sender", + wakeAgentId: null, + provenance: "legacy_explicit" as const, + }, + }; + const composed = composeScmAudience({ + existingEntries: [existing("human-h", "agent-b", "chat-a"), existing("human-h", "agent-b", "chat-b"), legacy], + personnelTargets: [personnel("human-h", "agent-c")], + }); + + expect(composed).toHaveLength(4); + expect(composed.map((target) => target.entry.kind)).toEqual([ + "existing_line", + "existing_line", + "legacy_route", + "personnel_target", + ]); + }); + + it("selects directed reason by stable priority rather than input order", () => { + const first = composeScmAudience({ + existingEntries: [existing("human-h", "agent-b", "chat-a")], + personnelTargets: [ + personnel("human-h", "agent-b", "assigned"), + personnel("human-h", "agent-b", "review_requested"), + ], + }); + const second = composeScmAudience({ + existingEntries: [existing("human-h", "agent-b", "chat-a")], + personnelTargets: [ + personnel("human-h", "agent-b", "review_requested"), + personnel("human-h", "agent-b", "assigned"), + ], + }); + + expect(first).toEqual(second); + expect(first[0]).toMatchObject({ directedContext: { reason: "review_requested" } }); + }); + + it("keeps provider tasks discriminated and outside personnel dedupe", () => { + const providerTask: ScmProviderTaskTarget<{ capability: string }> = { + kind: "provider_task_target", + humanAgentId: "human-h", + wakeAgentId: "agent-b", + reason: "mentioned", + externalUsername: "provider-app", + providerContext: { capability: "reply-run" }, + }; + const composed = composeScmAudience({ + existingEntries: [existing("human-h", "agent-b", "chat-a")], + personnelTargets: [], + providerTaskTargets: [providerTask], + }); + + expect(composed).toEqual([{ entry: existing("human-h", "agent-b", "chat-a") }, { entry: providerTask }]); + }); +}); diff --git a/packages/server/src/__tests__/scm-provider-attention-contract.test.ts b/packages/server/src/__tests__/scm-provider-attention-contract.test.ts index 137e12a02..a1825869e 100644 --- a/packages/server/src/__tests__/scm-provider-attention-contract.test.ts +++ b/packages/server/src/__tests__/scm-provider-attention-contract.test.ts @@ -1,13 +1,24 @@ import type { ScmIngressContext } from "@first-tree/shared"; +import { eq } from "drizzle-orm"; import { describe, expect, it } from "vitest"; +import { agents } from "../db/schema/agents.js"; +import { createAgent } from "../services/agent.js"; +import { resolveGithubAudience } from "../services/github-audience.js"; import { normalizeGithubWebhook } from "../services/github-normalize.js"; -import { applyGitlabPersonnelEvidence, normalizeGitlabWebhook } from "../services/gitlab-webhook.js"; +import { createGitlabConnection } from "../services/gitlab-connections.js"; +import { createGitlabIdentityLink } from "../services/gitlab-identities.js"; +import { + applyGitlabPersonnelEvidence, + normalizeGitlabWebhook, + resolveGitlabAudience, +} from "../services/gitlab-webhook.js"; import { planScmChatDeliveries, type ScmAudienceTarget, scmWakeAgentIds, selectScmSenderId, } from "../services/scm-chat-delivery-plan.js"; +import { createTestAdmin, useTestApp } from "./helpers.js"; describe.each(["github", "gitlab"] as const)("%s SCM attention conformance", (provider) => { it("routes an explicit line to one chat with its human sender and wake agent", async () => { @@ -69,7 +80,99 @@ it("keeps an explicit legacy GitLab route silent without representing a nullable expect(scmWakeAgentIds(entries)).toEqual([]); }); +it("keeps an actor-owned existing route as a silent card", async () => { + const planned = await planScmChatDeliveries({ + targets: [ + { + entry: { + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: "actor-human", + wakeAgentId: "actor-delegate", + chatId: "actor-chat", + provenance: "explicit", + }, + }, + }, + ], + actorHumanId: "actor-human", + resolveChat: async () => ({ chatId: "actor-chat", created: false }), + onTargetError: () => { + throw new Error("unexpected target error"); + }, + }); + const entries = [...(planned.deliveries.get("actor-chat")?.entries.values() ?? [])]; + expect(entries).toHaveLength(1); + expect(entries[0]?.wakeEligibility).toBe("actor_echo_suppressed"); + expect(scmWakeAgentIds(entries)).toEqual([]); +}); + +it("keeps one mixed-chat card and wakes only eligible sibling lines", async () => { + const planned = await planScmChatDeliveries({ + targets: [ + { + entry: { + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: "actor-human", + wakeAgentId: "actor-delegate", + chatId: "shared-chat", + provenance: "explicit", + }, + }, + }, + { + entry: { + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: "other-human", + wakeAgentId: "other-delegate", + chatId: "shared-chat", + provenance: "explicit", + }, + }, + }, + ], + actorHumanId: "actor-human", + resolveChat: async () => ({ chatId: "shared-chat", created: false }), + onTargetError: () => { + throw new Error("unexpected target error"); + }, + }); + const entries = [...(planned.deliveries.get("shared-chat")?.entries.values() ?? [])]; + expect(entries).toHaveLength(2); + expect(scmWakeAgentIds(entries)).toEqual(["other-delegate"]); +}); + +it("keeps a fresh self-directed personnel target wake-eligible", async () => { + const planned = await planScmChatDeliveries({ + targets: [ + { + entry: { + kind: "personnel_target", + humanAgentId: "actor-human", + wakeAgentId: "actor-delegate", + reason: "assigned", + externalUsername: "actor", + }, + }, + ], + actorHumanId: "actor-human", + resolveChat: async () => ({ chatId: "fresh-chat", created: true }), + onTargetError: () => { + throw new Error("unexpected target error"); + }, + }); + const entries = [...(planned.deliveries.get("fresh-chat")?.entries.values() ?? [])]; + expect(entries[0]?.wakeEligibility).toBe("eligible"); + expect(scmWakeAgentIds(entries)).toEqual(["actor-delegate"]); +}); + describe("GitHub/GitLab semantic webhook conformance", () => { + const getApp = useTestApp(); const githubIngress: ScmIngressContext = { provider: "github", source: { organizationId: "org-1", externalId: "installation:1" }, @@ -176,6 +279,128 @@ describe("GitHub/GitLab semantic webhook conformance", () => { }); }); + it("keeps assigned+mentioned parity through each provider audience adapter", async () => { + const app = getApp(); + const admin = await createTestAdmin(app); + const delegate = await createAgent(app.db, { + name: `parity-delegate-${admin.username}`, + type: "agent", + displayName: "Parity Delegate", + managerId: admin.memberId, + organizationId: admin.organizationId, + }); + await app.db.update(agents).set({ delegateMention: delegate.uuid }).where(eq(agents.uuid, admin.humanAgentUuid)); + const [human] = await app.db + .select({ name: agents.name }) + .from(agents) + .where(eq(agents.uuid, admin.humanAgentUuid)) + .limit(1); + if (!human?.name) throw new Error("parity human login missing"); + const humanLogin = human.name; + const connection = await createGitlabConnection(app.db, { + organizationId: admin.organizationId, + memberId: admin.memberId, + displayName: "Parity GitLab", + instanceOrigin: "https://gitlab.internal", + }); + await createGitlabIdentityLink(app.db, { + organizationId: admin.organizationId, + connectionId: connection.connectionId, + membershipId: admin.memberId, + username: humanLogin, + }); + const github = normalizeGithubWebhook( + "pull_request", + { + action: "opened", + sender: { login: "author", type: "User" }, + repository: { full_name: "acme/api" }, + pull_request: { + number: 10, + title: "Shared priority", + body: `Please review @${humanLogin}`, + html_url: "https://github.com/acme/api/pull/10", + state: "open", + assignees: [{ login: humanLogin }], + }, + }, + { ...githubIngress, source: { organizationId: admin.organizationId, externalId: "installation:1" } }, + ); + const gitlabRaw = normalizeGitlabWebhook({ + organizationId: admin.organizationId, + connectionId: connection.connectionId, + instanceOrigin: "https://gitlab.internal", + stableDeliveryId: null, + eventHeader: "Merge Request Hook", + body: { + object_kind: "merge_request", + project: { + id: 11, + path_with_namespace: "acme/api", + web_url: "https://gitlab.internal/acme/api", + }, + user: { username: "author" }, + reviewers: [], + assignees: [{ username: humanLogin }], + object_attributes: { + iid: 10, + action: "open", + description: `Please review @${humanLogin}`, + title: "Shared priority", + url: "https://gitlab.internal/acme/api/-/merge_requests/10", + state: "opened", + }, + }, + }); + const gitlab = applyGitlabPersonnelEvidence(gitlabRaw, "reviewers"); + if (!github.event || !gitlab.event || !gitlabRaw.entityIdentity) throw new Error("adapter parity event missing"); + expect(github.event?.targets).toEqual([ + { externalUsername: humanLogin.toLowerCase(), reason: "assigned" }, + { externalUsername: humanLogin.toLowerCase(), reason: "mentioned" }, + ]); + expect(gitlab.event?.targets).toEqual([ + { externalUsername: humanLogin, reason: "assigned" }, + { externalUsername: humanLogin, reason: "mentioned" }, + ]); + const githubAudience = await resolveGithubAudience(app.db, github.event); + const gitlabAudience = await resolveGitlabAudience(app.db, { + organizationId: admin.organizationId, + connectionId: connection.connectionId, + event: gitlab.event, + entityIdentity: gitlabRaw.entityIdentity, + }); + const project = (targets: ScmAudienceTarget[]) => + targets.map((target) => ({ + kind: target.entry.kind, + reason: + target.entry.kind === "personnel_target" || target.entry.kind === "provider_task_target" + ? target.entry.reason + : target.directedContext?.reason, + humanAgentId: + target.entry.kind === "personnel_target" || target.entry.kind === "provider_task_target" + ? target.entry.humanAgentId + : target.entry.kind === "existing_line" + ? target.entry.line.humanAgentId + : null, + wakeAgentId: + target.entry.kind === "personnel_target" || target.entry.kind === "provider_task_target" + ? target.entry.wakeAgentId + : target.entry.kind === "existing_line" + ? target.entry.line.wakeAgentId + : null, + })); + const expected = [ + { + kind: "personnel_target", + reason: "mentioned", + humanAgentId: admin.humanAgentUuid, + wakeAgentId: delegate.uuid, + }, + ]; + expect(project(githubAudience.targets)).toEqual(expected); + expect(project(gitlabAudience.targets)).toEqual(expected); + }); + it("normalizes terminal pull requests to observation-only merged state", () => { const github = normalizeGithubWebhook( "pull_request", diff --git a/packages/server/src/services/github-audience.ts b/packages/server/src/services/github-audience.ts index 84f839c09..4b969845b 100644 --- a/packages/server/src/services/github-audience.ts +++ b/packages/server/src/services/github-audience.ts @@ -3,9 +3,9 @@ import { AGENT_TYPES, type GithubAppInstallationPermissions, type GithubTaskReplyErrorCode, - type InvolveReason, isDeclaredBoundVia, type NormalizedScmEvent, + type ScmAudienceEntry, } from "@first-tree/shared"; import { and, eq, inArray, sql } from "drizzle-orm"; import type { Database } from "../db/connection.js"; @@ -15,6 +15,12 @@ import { loadValidContextReviewerAgent } from "./context-reviewer-common.js"; import { normalizeGithubRepo } from "./context-reviewer-pr.js"; import { githubEntityKeyCandidates } from "./github-entity-key.js"; import { getOrgContextReviewRuntime } from "./org-settings.js"; +import { + composeScmAudience, + type ScmAudienceTarget, + type ScmPersonnelTarget, + type ScmProviderTaskTarget, +} from "./scm-audience-composition.js"; import { getTeamAgentUuid } from "./team-agent-settings.js"; /** @@ -46,11 +52,11 @@ export function evaluateDelegateTarget( * Resolve the GitHub actor to the represented First Tree human when possible. * * GitHub tells us which login triggered the event; it does not tell us which - * local agent, if any, performed the action. Echo pruning is therefore + * local agent, if any, performed the action. Echo wake suppression is therefore * human-scoped: if the actor login maps to an org-local human agent, delivery - * can remove entries that belong to that same human. Unknown humans, external - * users, and bot/app senders return null and are delivered without self - * pruning. + * can silence matching existing lines while retaining their card routes. + * Unknown humans, external users, and bot/app senders return null and are + * delivered without guessed suppression. */ export async function resolveGithubActorHumanId( db: Database, @@ -71,29 +77,13 @@ export async function resolveGithubActorHumanId( return agentRow?.uuid ?? null; } -/** One candidate delivery entry from Stage 2. */ -export type AudienceTarget = { - humanAgentId: string; - delegateAgentId: string; - kind: "existing" | "new"; - /** Set only when `kind === "existing"`. */ - chatId: string | null; - /** Set when this delivery is also a target-human route. */ - involveReason: InvolveReason | null; - /** - * Lower-cased GitHub login that caused this target route. Stage 3 reads it - * to fill the card's `mentionedUser` field so a chat targeted at user X - * never displays "Y was mentioned" because two involves shared the same - * reason. - */ - involveLogin: string | null; - /** The configured GitHub App was the directed target for this one Agent. */ - teamAgentTask?: { agentUuid: string }; - provenance?: "explicit" | "identity_target" | "related_entity"; +export type GithubProviderTaskContext = { + kind: "github_app_task"; + agentUuid: string; }; export type AudienceResolution = { - targets: AudienceTarget[]; + targets: ScmAudienceTarget[]; actorHumanId: string | null; appTaskBlocker: GithubTaskReplyErrorCode | null; }; @@ -171,14 +161,8 @@ async function resolveGithubAppTaskAgent( * * audience = follow mappings ∪ target deliveries * - * `subscribed` reads every `(human, delegate)` row already bound to - * `(org, entity)` in `github_entity_chat_mappings`. `involved` walks - * `event.targets` as target-human candidates. A target human first reuses - * their existing mapping(s) on the entity; only humans without a mapping fall - * back to their default delegate and produce a `kind: "new"` row. - * - * Echo pruning happens in delivery, before fresh-chat resolution, so self-only - * events do not write cards and mixed events keep the other humans' entries. + * Provider code resolves rows and external identities; shared SCM composition + * owns exact-pair target matching and preserves every distinct route. */ export async function resolveGithubAudience( db: Database, @@ -271,34 +255,35 @@ export async function resolveGithubAudience( return row.humanAgentName !== null && involvedLogins.has(row.humanAgentName.toLowerCase()); }; - const subscribed: AudienceTarget[] = [...earliestByAttentionLine.values()] + const existingEntries: Array> = [ + ...earliestByAttentionLine.values(), + ] .filter(keepSubscribedOpened) .map((row) => ({ - humanAgentId: row.humanAgentId, - delegateAgentId: row.delegateAgentId, - kind: "existing", - chatId: row.chatId, - involveReason: null, - involveLogin: null, - provenance: isDeclaredBoundVia(row.boundVia) - ? "explicit" - : row.boundVia === "fixes_link" - ? "related_entity" - : "identity_target", + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: row.humanAgentId, + wakeAgentId: row.delegateAgentId, + chatId: row.chatId, + provenance: isDeclaredBoundVia(row.boundVia) + ? "explicit" + : row.boundVia === "fixes_link" + ? "related_entity" + : "identity_target", + }, })); - const subscribedByHuman = new Map(); - for (const target of subscribed) { - const rows = subscribedByHuman.get(target.humanAgentId); - if (rows) rows.push(target); - else subscribedByHuman.set(target.humanAgentId, [target]); - } - - const involved: AudienceTarget[] = []; + const personnelTargets: ScmPersonnelTarget[] = []; if (humanTargets.length > 0) { const candidateLogins = humanTargets.map((target) => target.externalUsername.toLowerCase()); - const reasonByLogin = new Map(); - for (const target of humanTargets) reasonByLogin.set(target.externalUsername.toLowerCase(), target.reason); + const targetsByLogin = new Map(); + for (const target of humanTargets) { + const login = target.externalUsername.toLowerCase(); + const targets = targetsByLogin.get(login); + if (targets) targets.push(target); + else targetsByLogin.set(login, [target]); + } const candidates = await db .select({ @@ -338,32 +323,25 @@ export async function resolveGithubAudience( for (const c of candidates) { if (c.status !== AGENT_STATUSES.ACTIVE || !c.name) continue; const candidateLogin = c.name.toLowerCase(); - const reason = reasonByLogin.get(candidateLogin); - if (!reason) continue; - - const existingForHuman = subscribedByHuman.get(c.id); - if (existingForHuman) { - for (const target of existingForHuman) { - target.involveReason = reason; - target.involveLogin = candidateLogin; - } - continue; - } + const directedTargets = targetsByLogin.get(candidateLogin); + if (!directedTargets) continue; if (!c.delegateMention) continue; const verdict = evaluateDelegateTarget(delegateById.get(c.delegateMention), organizationId); if (verdict !== "ok") continue; - involved.push({ - humanAgentId: c.id, - delegateAgentId: c.delegateMention, - kind: "new", - chatId: null, - involveReason: reason, - involveLogin: candidateLogin, - }); + for (const target of directedTargets) { + personnelTargets.push({ + kind: "personnel_target", + reason: target.reason, + humanAgentId: c.id, + wakeAgentId: c.delegateMention, + externalUsername: candidateLogin, + }); + } } } + const providerTaskTargets: ScmProviderTaskTarget[] = []; if (teamAgentTaskTarget) { const supportedEntity = (event.entity.type === "issue" || event.entity.type === "pull_request") && @@ -376,23 +354,18 @@ export async function resolveGithubAudience( } else { const taskAgent = await resolveGithubAppTaskAgent(db, event); if (taskAgent) { - // Always model an App-directed request as a fresh personnel target, even - // when its attention line already exists. `resolveTargetChat` still - // reuses that mapping, while the personnel shape preserves a manager's - // self-directed @App request from actor-echo pruning. - involved.push({ + providerTaskTargets.push({ + kind: "provider_task_target", + reason: teamAgentTaskTarget.reason, humanAgentId: taskAgent.managerHumanAgentId, - delegateAgentId: taskAgent.uuid, - kind: "new", - chatId: null, - involveReason: teamAgentTaskTarget.reason, - involveLogin: teamAgentTaskTarget.externalUsername.toLowerCase(), - teamAgentTask: { agentUuid: taskAgent.uuid }, + wakeAgentId: taskAgent.uuid, + externalUsername: teamAgentTaskTarget.externalUsername.toLowerCase(), + providerContext: { kind: "github_app_task", agentUuid: taskAgent.uuid }, }); } } } - const audience = [...subscribed, ...involved]; + const audience = composeScmAudience({ existingEntries, personnelTargets, providerTaskTargets }); return { targets: audience, actorHumanId, appTaskBlocker }; } diff --git a/packages/server/src/services/github-delivery.ts b/packages/server/src/services/github-delivery.ts index 69f06a8e7..f28ec5a57 100644 --- a/packages/server/src/services/github-delivery.ts +++ b/packages/server/src/services/github-delivery.ts @@ -3,15 +3,16 @@ import type { FastifyInstance } from "fastify"; import type { GithubEntity } from "../api/webhooks/github-entity.js"; import { createLogger } from "../observability/index.js"; import { uuidv7 } from "../uuid.js"; -import type { AudienceTarget } from "./github-audience.js"; -import { findReuseChatForInvolved, refreshGithubChatTopic, resolveTargetChat } from "./github-entity-chat.js"; +import type { GithubProviderTaskContext } from "./github-audience.js"; +import { decideGithubPersonnelTargetChat, refreshGithubChatTopic, resolveTargetChat } from "./github-entity-chat.js"; import { type EntityStateSeed, setEntityTitle } from "./github-entity-state.js"; import { applyMembershipWrite } from "./participant-mode.js"; +import type { ScmAudienceTarget } from "./scm-audience-composition.js"; import { sendScmSystemCard } from "./scm-card-delivery.js"; import { compareScmDeliveryEntries, planScmChatDeliveries, - type ScmAudienceTarget, + scmProviderContextEntries, scmTargetHumanAgentId, scmTargetWakeAgentId, scmWakeAgentIds, @@ -53,17 +54,17 @@ type DeliveryOptions = { * Two phases. Phase 1 resolves every audience target to a chat (subscribed * targets short-circuit; involved targets reuse the entity's existing chat * when the involved human+delegate are already speakers, else mint a fresh - * one) and accumulates the per-chat entries. Self-echo pruning happens before - * fresh-chat resolution, so an actor's own target does not create an empty - * chat. Phase 2 delivers one card per chat, waking the union of surviving - * wake agents via native `metadata.mentions`. + * one) and accumulates the per-chat entries. Actor-owned existing lines retain + * their card routes but become wake-ineligible; fresh directed targets remain + * eligible to create the work entry. Phase 2 delivers one card per chat, + * waking the union of eligible agents via native `metadata.mentions`. * Each chat is delivered independently so a single failure doesn't poison the * rest — the loop logs and continues. */ export async function deliverGithubEvent( app: FastifyInstance, event: NormalizedScmEvent, - audience: AudienceTarget[], + audience: ScmAudienceTarget[], options: DeliveryOptions = {}, ): Promise { const stats: DeliveryStats = { delivered: 0, newChats: 0, failed: 0 }; @@ -71,48 +72,9 @@ export async function deliverGithubEvent( const existingMappedChatIds = existingMappedChatIdsForProjection(audience); const entity = entityFromEvent(event); - // Phase 1 — shared SCM planner owns echo pruning and one-delivery-per-chat. + // Phase 1 — shared SCM planner owns echo wake policy and one-delivery-per-chat. const planned = await planScmChatDeliveries({ - targets: audience.map( - (target): ScmAudienceTarget => - target.kind === "existing" - ? { - entry: { - kind: "existing_line", - line: { - kind: "attention_line", - humanAgentId: target.humanAgentId, - wakeAgentId: target.delegateAgentId, - chatId: target.chatId as string, - provenance: target.provenance ?? "identity_target", - }, - }, - directedContext: - target.involveReason && target.involveLogin - ? { - reason: target.involveReason, - externalUsername: target.involveLogin, - ...(target.teamAgentTask ? { teamAgentTask: target.teamAgentTask } : {}), - } - : null, - } - : { - entry: { - kind: "personnel_target", - reason: target.involveReason as InvolveReason, - humanAgentId: target.humanAgentId, - wakeAgentId: target.delegateAgentId, - externalUsername: target.involveLogin as string, - }, - directedContext: target.teamAgentTask - ? { - reason: target.involveReason as InvolveReason, - externalUsername: target.involveLogin as string, - teamAgentTask: target.teamAgentTask, - } - : null, - }, - ), + targets: audience, actorHumanId, resolveChat: (target) => resolveChatFor(app, event, target, options), onTargetError: (target, err) => { @@ -150,9 +112,8 @@ export async function deliverGithubEvent( const byChat = planned.deliveries; // Phase 1.5 — refresh the local projection for this entity independently - // from card delivery. A self-only event can prune every delivery entry, but - // it still proves the entity has an existing local mapping whose title/topic - // projection must stay fresh. + // from card delivery. Existing local mappings refresh their title/topic + // projection regardless of whether their wake is actor-echo suppressed. const shouldRefreshEntityProjection = byChat.size > 0 || existingMappedChatIds.length > 0; if (shouldRefreshEntityProjection && event.entity.title && event.entity.title.length > 0) { try { @@ -193,15 +154,20 @@ export async function deliverGithubEvent( const entries = [...delivery.entries.values()].sort(compareScmDeliveryEntries); const senderId = selectScmSenderId(entries); const cardContext = selectScmCardContext(entries); - const taskRun = - cardContext.teamAgentTask && cardContext.teamAgentTaskHumanAgentId - ? createGithubTaskRun(event, cardContext.teamAgentTask, cardContext.teamAgentTaskHumanAgentId) - : null; + const taskEntry = scmProviderContextEntries(entries) + .filter( + (entry) => + entry.providerContext.kind === "github_app_task" && entry.providerContext.agentUuid === entry.wakeAgentId, + ) + .sort(compareScmDeliveryEntries)[0]; + const taskRun = taskEntry?.humanAgentId + ? createGithubTaskRun(event, taskEntry.providerContext, taskEntry.humanAgentId) + : null; const card = buildCard( event, cardContext.involveReason, cardContext.involveLogin, - taskRun?.marker ?? cardContext.teamAgentTask, + taskRun?.marker ?? taskEntry?.providerContext ?? null, ); const mentionedUser = card.mentionedUser ?? undefined; // Native wake-set (S8): the delegates are passed as `metadata.mentions`, @@ -267,11 +233,9 @@ export async function deliverGithubEvent( return stats; } -function existingMappedChatIdsForProjection(audience: AudienceTarget[]): string[] { +function existingMappedChatIdsForProjection(audience: ScmAudienceTarget[]): string[] { return [ - ...new Set( - audience.filter((target) => target.kind === "existing" && target.chatId).map((target) => target.chatId as string), - ), + ...new Set(audience.flatMap((target) => (target.entry.kind === "existing_line" ? [target.entry.line.chatId] : []))), ].sort(); } @@ -289,7 +253,7 @@ type ResolvedChat = { chatId: string; created: boolean }; async function resolveChatFor( app: FastifyInstance, event: NormalizedScmEvent, - target: ScmAudienceTarget, + target: ScmAudienceTarget, options: DeliveryOptions, ): Promise { if (target.entry.kind === "existing_line") { @@ -306,24 +270,17 @@ async function resolveChatFor( title: event.entity.title, url: event.entity.url, }; - // Reviewer-reuse (S9, deliver-once-per-chat) — scoped to `review_requested` - // ONLY. When the entity already has exactly one reusable bound chat (the - // reviewer's human + delegate both already speak there), route there instead - // of minting a sibling chat, writing NO mapping row; the pair sees the - // entity's events through chat membership and Phase 2 dedups to one card. - // `mentioned` / `assigned` involves are deliberately NOT reused: a mention is - // a directed call that must mint a fresh chat (S5 — mentions pierce into a - // new chat, never back into an existing/unfollowed one). - if (target.entry.reason === "review_requested") { - const reuseChatId = await findReuseChatForInvolved( - app.db, - event.source.organizationId, - entity, - humanAgentId, - wakeAgentId, - ); - if (reuseChatId) return { chatId: reuseChatId, created: false }; - } + const intent = + target.entry.kind === "provider_task_target" + ? ({ kind: "provider_task_target" } as const) + : await decideGithubPersonnelTargetChat( + app.db, + event.source.organizationId, + entity, + humanAgentId, + wakeAgentId, + target.entry.reason, + ); const relatedEntities: GithubEntity[] = event.relatedRefs.map((ref) => ({ type: "issue", @@ -338,14 +295,19 @@ async function resolveChatFor( eventType: event.eventType, action: event.action ?? "", entityStateSeed: options.entityStateSeed ?? null, - // `kind: "new"` audience targets come from explicit mentions / involves in - // the event payload — the only path allowed to mint a fresh chat for an + // Personnel and provider-task targets come from explicit directed evidence + // in the event payload — the only paths allowed to mint a fresh chat for an // opened creation event. Subscription targets short-circuit above; the // guard is still wired so any future caller is safe by default. isMentionMatched: true, + intent, }); if (!resolved) return null; - if (target.directedContext?.teamAgentTask && target.directedContext.teamAgentTask.agentUuid === wakeAgentId) { + if ( + target.entry.kind === "provider_task_target" && + target.entry.providerContext.kind === "github_app_task" && + target.entry.providerContext.agentUuid === wakeAgentId + ) { await applyMembershipWrite(app.db, resolved.chatId, [{ agentId: wakeAgentId }], { upgradeWatcherToSpeaker: true }); } return { chatId: resolved.chatId, created: resolved.created }; diff --git a/packages/server/src/services/github-entity-chat.ts b/packages/server/src/services/github-entity-chat.ts index 9472f6cd5..2da8b3ec7 100644 --- a/packages/server/src/services/github-entity-chat.ts +++ b/packages/server/src/services/github-entity-chat.ts @@ -14,7 +14,7 @@ import { } from "./github-entity-key.js"; import type { EntityState, EntityStateSeed } from "./github-entity-state.js"; import { resolveAgentScmBindingPair } from "./scm-attention-line.js"; -import { decideScmPersonnelTargetChat } from "./scm-target-chat-policy.js"; +import { decideScmPersonnelTargetChat, type ScmTargetChatDecision } from "./scm-target-chat-policy.js"; const log = createLogger("GithubEntityChat"); @@ -73,19 +73,19 @@ export function isCreationEvent(eventType: string, action: string): boolean { * events through chat membership, and `deliverGithubEvent` dedups so the * chat receives one card whose wake-set includes this delegate. * - * Returns the chat id when exactly one such chat exists; null when there is - * none, or when the candidate is ambiguous (≥2 bound chats both speak in), - * in which case the caller mints a fresh chat via `resolveTargetChat` (the - * strict per-`(human, delegate)` path — we never guess). Preserves S1 (the - * chat follows, not the person) and S7 (no followed chat is dropped). + * Returns a discriminated policy decision so the provider executor cannot + * collapse `strict_new_line` into a nullable chat id and accidentally re-enter + * human-scoped fallback. Preserves S1 (the chat follows, not the person) and + * S7 (no followed chat is dropped). */ -export async function findReuseChatForInvolved( +export async function decideGithubPersonnelTargetChat( db: Database, organizationId: string, entity: GithubEntity, humanAgentId: string, delegateAgentId: string, -): Promise { + reason: "review_requested" | "mentioned" | "assigned", +): Promise { const candidateKeys = githubEntityKeyCandidates(entity.type, entity.key); const boundChats = await db .selectDistinct({ chatId: githubEntityChatMappings.chatId }) @@ -97,17 +97,18 @@ export async function findReuseChatForInvolved( inArray(githubEntityChatMappings.entityKey, candidateKeys), ), ); - if (boundChats.length === 0) return null; + if (boundChats.length === 0) return { kind: "strict_new_line" }; - const decision = await decideScmPersonnelTargetChat(db, { - reason: "review_requested", + return decideScmPersonnelTargetChat(db, { + reason, candidateChatIds: boundChats.map((row) => row.chatId), humanAgentId, wakeAgentId: delegateAgentId, }); - return decision.kind === "reuse" ? decision.chatId : null; } +export type GithubTargetChatIntent = { kind: "provider_task_target" } | ScmTargetChatDecision; + /** * Resolve which chat a GitHub event for (human, delegate, entity) belongs to. * @@ -151,6 +152,10 @@ export async function resolveTargetChat( * chat-proliferation behaviour. */ isMentionMatched: boolean; + /** Shared policy intent. Strict personnel targets may never reuse another + * delegate's human-scoped mapping; provider tasks retain their established + * team-level fallback behavior. */ + intent: GithubTargetChatIntent; /** * State derived from the current webhook payload. Used only when this * resolution writes a new mapping for the same entity; existing rows are @@ -158,7 +163,7 @@ export async function resolveTargetChat( */ entityStateSeed?: EntityStateSeed | null; }, -): Promise<{ chatId: string; created: boolean; boundVia: BoundVia } | null> { +): Promise<{ chatId: string; created: boolean; boundVia: BoundVia | null } | null> { const { organizationId, humanAgentId, @@ -168,36 +173,41 @@ export async function resolveTargetChat( eventType, action, isMentionMatched, + intent, } = params; const entity = normalizeGithubEntity(rawEntity); const relatedEntities = rawRelatedEntities.map(normalizeGithubEntity); const entityState = stateSeedForEntity(params.entityStateSeed ?? null, entity); + if (intent.kind === "reuse") { + return { chatId: intent.chatId, created: false, boundVia: null }; + } + // (a) Direct hit. const direct = await lookupMapping(db, organizationId, humanAgentId, delegateAgentId, entity); if (direct) { return { chatId: direct.chatId, created: false, boundVia: direct.boundVia }; } - // (a.5) Human-scoped fallback. The mapping primary key still includes - // `delegate_agent_id`, but routing treats `(org, human, entity)` as the - // logical cluster: an entity that is already bound to a chat under this - // human should never trigger a fresh chat just because a *different* - // delegate happened to drive this event. Pick the existing chat (open - // entities first, then earliest `bound_at`) and write a sibling mapping - // row so the next event hits (a) directly. - const humanScoped = await lookupMappingByHuman(db, organizationId, humanAgentId, entity); - if (humanScoped) { - const inserted = await insertMappingIfAbsent(db, { - organizationId, - humanAgentId, - delegateAgentId, - entity, - chatId: humanScoped.chatId, - boundVia: "human_fallback", - entityState, - }); - return { chatId: inserted.chatId, created: false, boundVia: inserted.boundVia }; + // (a.5) Provider-task human-scoped fallback. Personnel + // `strict_new_line` intent deliberately skips this branch: another wake + // agent using the same human as carrier is not the current delegate's line. + // The provider-owned task path retains the established deterministic fallback + // and writes a sibling mapping so its next event hits (a). + if (intent.kind === "provider_task_target") { + const humanScoped = await lookupMappingByHuman(db, organizationId, humanAgentId, entity); + if (humanScoped) { + const inserted = await insertMappingIfAbsent(db, { + organizationId, + humanAgentId, + delegateAgentId, + entity, + chatId: humanScoped.chatId, + boundVia: "human_fallback", + entityState, + }); + return { chatId: inserted.chatId, created: false, boundVia: inserted.boundVia }; + } } // (b) Fixes-link reuse. diff --git a/packages/server/src/services/github-normalize.ts b/packages/server/src/services/github-normalize.ts index d78eae69c..f9c22c211 100644 --- a/packages/server/src/services/github-normalize.ts +++ b/packages/server/src/services/github-normalize.ts @@ -206,18 +206,18 @@ function readStringArray(value: unknown): string[] { type InvolveItem = { externalUsername: string; reason: InvolveReason }; function buildInvolves(items: ReadonlyArray<{ logins: string[]; reason: InvolveReason }>): InvolveItem[] { - // First-occurrence wins per (lowercased) login. Callers should list - // structural reasons (review_requested, assigned) before textual ones - // (mentioned) so a participant cited via both routes keeps the more - // specific reason in the audience card. + // Deduplicate repeated provider evidence only within the same reason. Cross- + // reason candidates must reach shared SCM composition, which owns the stable + // review_requested > mentioned > assigned priority for every provider. const seen = new Set(); const out: InvolveItem[] = []; for (const group of items) { for (const login of group.logins) { - const key = login.toLowerCase(); + const normalizedLogin = login.toLowerCase(); + const key = `${normalizedLogin}\u0000${group.reason}`; if (seen.has(key)) continue; seen.add(key); - out.push({ externalUsername: key, reason: group.reason }); + out.push({ externalUsername: normalizedLogin, reason: group.reason }); } } return out; diff --git a/packages/server/src/services/gitlab-webhook.ts b/packages/server/src/services/gitlab-webhook.ts index f1de40e88..b3f3457ff 100644 --- a/packages/server/src/services/gitlab-webhook.ts +++ b/packages/server/src/services/gitlab-webhook.ts @@ -4,6 +4,7 @@ import type { GitlabTargetClass, InvolveReason, NormalizedScmEvent, + ScmAudienceEntry, ScmEntityObservation, ScmIngressContext, ScmNormalizedWebhook, @@ -30,11 +31,11 @@ import { normalizeGitlabUsername, resolveActiveGitlabIdentity, } from "./gitlab-identities.js"; +import { composeScmAudience, type ScmAudienceTarget, type ScmPersonnelTarget } from "./scm-audience-composition.js"; import { type DeferredScmCardPostCommitEffects, sendScmSystemCard } from "./scm-card-delivery.js"; import { compareScmDeliveryEntries, planScmChatDeliveries, - type ScmAudienceTarget, scmTargetHumanAgentId, scmTargetWakeAgentId, scmWakeAgentIds, @@ -705,7 +706,7 @@ export async function resolveGitlabAudience( actorHumanId = actor.outcome === "ok" ? actor.identity.humanAgentId : null; const rows = input.followers ?? (await observeGitlabEntityAndResolveFollowers(db, input.connectionId, input.entityIdentity)); - const targets: ScmAudienceTarget[] = []; + const existingEntries: Array> = []; for (const row of rows) { if (row.boundVia === "identity_target") { if (!row.identityLinkId || !row.humanAgentId || !row.delegateAgentId) continue; @@ -735,51 +736,46 @@ export async function resolveGitlabAudience( ) { continue; } - targets.push({ - entry: { - kind: "existing_line", - line: { - kind: "attention_line", - humanAgentId: row.humanAgentId, - wakeAgentId: row.delegateAgentId, - chatId: row.chatId, - provenance: "identity_target", - }, + existingEntries.push({ + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId: row.humanAgentId, + wakeAgentId: row.delegateAgentId, + chatId: row.chatId, + provenance: "identity_target", }, }); } else { const humanAgentId = row.humanAgentId; const wakeAgentId = row.delegateAgentId; if (humanAgentId !== null && wakeAgentId !== null) { - targets.push({ - entry: { - kind: "existing_line", - line: { - kind: "attention_line", - humanAgentId, - wakeAgentId, - chatId: row.chatId, - provenance: "explicit", - }, + existingEntries.push({ + kind: "existing_line", + line: { + kind: "attention_line", + humanAgentId, + wakeAgentId, + chatId: row.chatId, + provenance: "explicit", }, }); } else { - targets.push({ - entry: { - kind: "legacy_route", - route: { - kind: "legacy_route_only", - chatId: row.chatId, - senderAgentId: row.declaredByAgentId, - wakeAgentId: null, - provenance: "legacy_explicit", - }, + existingEntries.push({ + kind: "legacy_route", + route: { + kind: "legacy_route_only", + chatId: row.chatId, + senderAgentId: row.declaredByAgentId, + wakeAgentId: null, + provenance: "legacy_explicit", }, }); } } } + const personnelTargets: ScmPersonnelTarget[] = []; for (const target of input.event.targets) { const normalizedUsername = normalizeGitlabUsername(target.externalUsername).normalized; const resolved = await resolveActiveGitlabIdentity(db, { @@ -799,34 +795,16 @@ export async function resolveGitlabAudience( }); continue; } - const existingIndex = targets.findIndex( - (candidate) => - candidate.entry.kind === "existing_line" && - candidate.entry.line.humanAgentId === resolved.identity.humanAgentId && - candidate.entry.line.wakeAgentId === resolved.identity.delegateAgentId, - ); - if (existingIndex >= 0) { - const existing = targets[existingIndex]; - if (existing) { - targets.push({ - entry: existing.entry, - directedContext: { reason: target.reason, externalUsername: normalizedUsername }, - }); - } - continue; - } - targets.push({ - entry: { - kind: "personnel_target", - reason: target.reason, - humanAgentId: resolved.identity.humanAgentId, - wakeAgentId: resolved.identity.delegateAgentId, - externalUsername: normalizedUsername, - }, + personnelTargets.push({ + kind: "personnel_target", + reason: target.reason, + humanAgentId: resolved.identity.humanAgentId, + wakeAgentId: resolved.identity.delegateAgentId, + externalUsername: normalizedUsername, }); } - return { targets, actorHumanId }; + return { targets: composeScmAudience({ existingEntries, personnelTargets }), actorHumanId }; } async function resolveGitlabTargetChat( diff --git a/packages/server/src/services/scm-audience-composition.ts b/packages/server/src/services/scm-audience-composition.ts new file mode 100644 index 000000000..0966d7a30 --- /dev/null +++ b/packages/server/src/services/scm-audience-composition.ts @@ -0,0 +1,106 @@ +import type { InvolveReason, ScmAudienceEntry } from "@first-tree/shared"; + +type ExistingScmAudienceEntry = Exclude; +export type ScmPersonnelTarget = Extract; + +export type ScmDirectedContext = { + reason: InvolveReason; + externalUsername: string; +}; + +export type ScmProviderTaskTarget = { + kind: "provider_task_target"; + reason: InvolveReason; + humanAgentId: string; + wakeAgentId: string; + externalUsername: string; + providerContext: TProviderContext; +}; + +export type ScmAudienceTarget = + | { + entry: ExistingScmAudienceEntry | ScmPersonnelTarget; + directedContext?: ScmDirectedContext | null; + } + | { + entry: ScmProviderTaskTarget; + directedContext?: never; + }; + +/** + * Compose provider-resolved attention lines and directed targets once for all + * SCM providers. Existing and legacy routes retain their distinct chat lines; + * only an exact human/wake pair can carry personnel context. + */ +export function composeScmAudience(input: { + existingEntries: ExistingScmAudienceEntry[]; + personnelTargets: ScmPersonnelTarget[]; + providerTaskTargets?: ScmProviderTaskTarget[]; +}): ScmAudienceTarget[] { + const targets: ScmAudienceTarget[] = []; + const existingKeys = new Set(); + for (const entry of input.existingEntries) { + const key = existingEntryKey(entry); + if (existingKeys.has(key)) continue; + existingKeys.add(key); + targets.push({ entry }); + } + + const personnelByPair = new Map(); + for (const target of input.personnelTargets) { + const pair = attentionPairKey(target.humanAgentId, target.wakeAgentId); + const current = personnelByPair.get(pair); + if (!current || comparePersonnelTargets(target, current) < 0) personnelByPair.set(pair, target); + } + + for (const [pair, personnelTarget] of [...personnelByPair.entries()].sort(([a], [b]) => a.localeCompare(b))) { + let matched = false; + for (const target of targets) { + if (target.entry.kind !== "existing_line") continue; + if (attentionPairKey(target.entry.line.humanAgentId, target.entry.line.wakeAgentId) !== pair) { + continue; + } + matched = true; + target.directedContext = { + reason: personnelTarget.reason, + externalUsername: personnelTarget.externalUsername, + }; + } + if (!matched) targets.push({ entry: personnelTarget }); + } + + for (const providerTaskTarget of input.providerTaskTargets ?? []) { + targets.push({ entry: providerTaskTarget }); + } + return targets; +} + +function existingEntryKey(entry: ExistingScmAudienceEntry): string { + if (entry.kind === "existing_line") { + const line = entry.line; + return ["line", line.humanAgentId, line.wakeAgentId, line.chatId, line.provenance].join(":"); + } + const route = entry.route; + return ["legacy", route.chatId, route.senderAgentId, route.provenance].join(":"); +} + +function attentionPairKey(humanAgentId: string, wakeAgentId: string): string { + return `${humanAgentId}\u0000${wakeAgentId}`; +} + +function comparePersonnelTargets(a: ScmPersonnelTarget, b: ScmPersonnelTarget): number { + return ( + involveReasonRank(a.reason) - involveReasonRank(b.reason) || a.externalUsername.localeCompare(b.externalUsername) + ); +} + +function involveReasonRank(reason: InvolveReason): number { + switch (reason) { + case "review_requested": + return 0; + case "mentioned": + return 1; + case "assigned": + return 2; + } +} diff --git a/packages/server/src/services/scm-chat-delivery-plan.ts b/packages/server/src/services/scm-chat-delivery-plan.ts index 49e663ae5..68d107cd9 100644 --- a/packages/server/src/services/scm-chat-delivery-plan.ts +++ b/packages/server/src/services/scm-chat-delivery-plan.ts @@ -1,28 +1,25 @@ -import type { InvolveReason, ScmAudienceEntry } from "@first-tree/shared"; - -export type ScmAudienceTarget = { - entry: ScmAudienceEntry; - directedContext?: { - reason: InvolveReason; - externalUsername: string; - teamAgentTask?: { agentUuid: string }; - } | null; -}; +import type { InvolveReason } from "@first-tree/shared"; +import type { ScmAudienceTarget } from "./scm-audience-composition.js"; + +export type { ScmAudienceTarget } from "./scm-audience-composition.js"; + +export type ScmWakeEligibility = "eligible" | "actor_echo_suppressed" | "route_only"; -export type ScmDeliveryEntry = { +export type ScmDeliveryEntry = { senderAgentId: string; humanAgentId: string | null; wakeAgentId: string | null; + wakeEligibility: ScmWakeEligibility; reasons: Set<"follow" | InvolveReason>; involveReason: InvolveReason | null; involveLogin: string | null; - teamAgentTask: { agentUuid: string } | null; + providerContext: TProviderContext | null; }; -export type ScmPlannedChatDelivery = { +export type ScmPlannedChatDelivery = { chatId: string; created: boolean; - entries: Map; + entries: Map>; }; export type ResolvedScmChat = { chatId: string; created: boolean }; @@ -30,27 +27,21 @@ export type ResolvedScmChat = { chatId: string; created: boolean }; /** * Provider-neutral per-processing-pass audience planner. * - * It owns the invariants shared by GitHub and GitLab: actor echo pruning - * before chat creation, provider-owned target→chat resolution, and exactly one - * accumulated delivery per chat. Providers still own their mapping stores, - * card content, topic projection and per-chat error telemetry. + * It keeps card routes separate from wake eligibility: actor echo suppresses + * only an existing line's wake, while the line still contributes its route and + * card context. Providers still own target-to-chat persistence, card content, + * topic projection, and opaque provider task capabilities. */ -export async function planScmChatDeliveries(input: { - targets: ScmAudienceTarget[]; +export async function planScmChatDeliveries(input: { + targets: ScmAudienceTarget[]; actorHumanId: string | null; - resolveChat: (target: ScmAudienceTarget) => Promise; - onTargetError: (target: ScmAudienceTarget, error: unknown) => void; - onTargetDropped?: (target: ScmAudienceTarget) => void; -}): Promise<{ deliveries: Map; failed: number }> { - const deliveries = new Map(); + resolveChat: (target: ScmAudienceTarget) => Promise; + onTargetError: (target: ScmAudienceTarget, error: unknown) => void; + onTargetDropped?: (target: ScmAudienceTarget) => void; +}): Promise<{ deliveries: Map>; failed: number }> { + const deliveries = new Map>(); let failed = 0; for (const target of input.targets) { - const humanAgentId = scmTargetHumanAgentId(target); - const freshDirectedSelfInvolve = target.entry.kind === "personnel_target"; - if (input.actorHumanId && humanAgentId === input.actorHumanId && !freshDirectedSelfInvolve) { - continue; - } - let resolved: ResolvedScmChat | null; try { resolved = await input.resolveChat(target); @@ -70,25 +61,32 @@ export async function planScmChatDeliveries(input: { } else if (resolved.created) { delivery.created = true; } - addScmDeliveryEntry(delivery, target); + addScmDeliveryEntry(delivery, target, input.actorHumanId); } return { deliveries, failed }; } -function addScmDeliveryEntry(delivery: ScmPlannedChatDelivery, target: ScmAudienceTarget): void { +function addScmDeliveryEntry( + delivery: ScmPlannedChatDelivery, + target: ScmAudienceTarget, + actorHumanId: string | null, +): void { const senderAgentId = scmTargetSenderAgentId(target); const humanAgentId = scmTargetHumanAgentId(target); const wakeAgentId = scmTargetWakeAgentId(target); const involveReason = - target.entry.kind === "personnel_target" ? target.entry.reason : (target.directedContext?.reason ?? null); + target.entry.kind === "personnel_target" || target.entry.kind === "provider_task_target" + ? target.entry.reason + : (target.directedContext?.reason ?? null); const involveLogin = - target.entry.kind === "personnel_target" + target.entry.kind === "personnel_target" || target.entry.kind === "provider_task_target" ? target.entry.externalUsername : (target.directedContext?.externalUsername ?? null); - const teamAgentTask = target.directedContext?.teamAgentTask ?? null; + const providerContext = target.entry.kind === "provider_task_target" ? target.entry.providerContext : null; + const wakeEligibility = scmWakeEligibility(target, actorHumanId); const key = `${senderAgentId}:${humanAgentId ?? "-"}:${wakeAgentId ?? "-"}`; const reasons = new Set<"follow" | InvolveReason>(); - if (target.entry.kind !== "personnel_target") reasons.add("follow"); + if (target.entry.kind === "existing_line" || target.entry.kind === "legacy_route") reasons.add("follow"); if (involveReason) reasons.add(involveReason); const existing = delivery.entries.get(key); if (existing) { @@ -100,71 +98,98 @@ function addScmDeliveryEntry(delivery: ScmPlannedChatDelivery, target: ScmAudien existing.involveReason = involveReason; existing.involveLogin = involveLogin; } - existing.teamAgentTask ??= teamAgentTask; + if (wakeEligibilityRank(wakeEligibility) < wakeEligibilityRank(existing.wakeEligibility)) { + existing.wakeEligibility = wakeEligibility; + } + existing.providerContext ??= providerContext; return; } delivery.entries.set(key, { senderAgentId, humanAgentId, wakeAgentId, + wakeEligibility, reasons, involveReason, involveLogin, - teamAgentTask, + providerContext, }); } -export function scmTargetHumanAgentId(target: ScmAudienceTarget): string | null { +function scmWakeEligibility( + target: ScmAudienceTarget, + actorHumanId: string | null, +): ScmWakeEligibility { + if (target.entry.kind === "legacy_route") return "route_only"; + if ( + target.entry.kind === "existing_line" && + actorHumanId !== null && + target.entry.line.humanAgentId === actorHumanId + ) { + return "actor_echo_suppressed"; + } + return "eligible"; +} + +export function scmTargetHumanAgentId(target: ScmAudienceTarget): string | null { switch (target.entry.kind) { case "existing_line": return target.entry.line.humanAgentId; case "personnel_target": + case "provider_task_target": return target.entry.humanAgentId; case "legacy_route": return null; } } -export function scmTargetWakeAgentId(target: ScmAudienceTarget): string | null { +export function scmTargetWakeAgentId(target: ScmAudienceTarget): string | null { switch (target.entry.kind) { case "existing_line": return target.entry.line.wakeAgentId; case "personnel_target": + case "provider_task_target": return target.entry.wakeAgentId; case "legacy_route": return null; } } -export function scmTargetSenderAgentId(target: ScmAudienceTarget): string { +export function scmTargetSenderAgentId(target: ScmAudienceTarget): string { switch (target.entry.kind) { case "existing_line": return target.entry.line.humanAgentId; case "personnel_target": + case "provider_task_target": return target.entry.humanAgentId; case "legacy_route": return target.entry.route.senderAgentId; } } -export function compareScmDeliveryEntries(a: ScmDeliveryEntry, b: ScmDeliveryEntry): number { +export function compareScmDeliveryEntries( + a: ScmDeliveryEntry, + b: ScmDeliveryEntry, +): number { return ( (a.humanAgentId ?? a.senderAgentId).localeCompare(b.humanAgentId ?? b.senderAgentId) || (a.wakeAgentId ?? "").localeCompare(b.wakeAgentId ?? "") ); } -export function selectScmSenderId(entries: ScmDeliveryEntry[]): string { - const first = entries[0]; - if (!first) throw new Error("delivery plan must have at least one surviving entry"); +export function selectScmSenderId(entries: ScmDeliveryEntry[]): string { + const first = [...entries].sort( + (a, b) => + wakeEligibilityRank(a.wakeEligibility) - wakeEligibilityRank(b.wakeEligibility) || + compareScmDeliveryEntries(a, b), + )[0]; + if (!first) throw new Error("delivery plan must have at least one routed entry"); return first.senderAgentId; } -export function selectScmCardContext(entries: ScmDeliveryEntry[]): { +export function selectScmCardContext(entries: ScmDeliveryEntry[]): { involveReason: InvolveReason | null; involveLogin: string | null; - teamAgentTask: { agentUuid: string } | null; - teamAgentTaskHumanAgentId: string | null; } { const involved = [...entries] .filter((entry) => entry.involveReason) @@ -172,22 +197,40 @@ export function selectScmCardContext(entries: ScmDeliveryEntry[]): { (a, b) => involveReasonRank(a.involveReason) - involveReasonRank(b.involveReason) || compareScmDeliveryEntries(a, b), )[0]; - const taskEntry = [...entries] - .filter( - (entry): entry is ScmDeliveryEntry & { teamAgentTask: { agentUuid: string } } => - entry.teamAgentTask !== null && entry.teamAgentTask.agentUuid === entry.wakeAgentId, - ) - .sort(compareScmDeliveryEntries)[0]; return { involveReason: involved?.involveReason ?? null, involveLogin: involved?.involveLogin ?? null, - teamAgentTask: taskEntry?.teamAgentTask ?? null, - teamAgentTaskHumanAgentId: taskEntry?.humanAgentId ?? null, }; } -export function scmWakeAgentIds(entries: ScmDeliveryEntry[]): string[] { - return [...new Set(entries.flatMap((entry) => (entry.wakeAgentId ? [entry.wakeAgentId] : [])))].sort(); +export function scmProviderContextEntries( + entries: ScmDeliveryEntry[], +): Array & { providerContext: TProviderContext }> { + return entries.filter( + (entry): entry is ScmDeliveryEntry & { providerContext: TProviderContext } => + entry.providerContext !== null, + ); +} + +export function scmWakeAgentIds(entries: ScmDeliveryEntry[]): string[] { + return [ + ...new Set( + entries.flatMap((entry) => + entry.wakeEligibility === "eligible" && entry.wakeAgentId ? [entry.wakeAgentId] : [], + ), + ), + ].sort(); +} + +function wakeEligibilityRank(eligibility: ScmWakeEligibility): number { + switch (eligibility) { + case "eligible": + return 0; + case "actor_echo_suppressed": + return 1; + case "route_only": + return 2; + } } function involveReasonRank(reason: InvolveReason | null): number {