From b92fcb642305b5f5461e9ff7bbf8d7433a3d4d16 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Tue, 15 Sep 2026 11:22:29 +0200 Subject: [PATCH 1/2] codex-branch-sync-action-notifications: notify actionable drift once --- build/cli/index.js | 252 ++++++++++++++++-- build/github_action/index.js | 229 ++++++++++++++-- docs/development/architecture.mdx | 14 +- docs/issues/branch-synchronization.mdx | 35 ++- docs/issues/notifications-and-auto-close.mdx | 21 ++ scripts/coverage-budgets.json | 24 ++ specs/CATALOG.md | 10 +- ...h-synchronization-and-conflict-recovery.md | 131 ++++++--- specs/catalog.json | 25 +- ...tic-github-publication-and-notification.md | 22 +- .../branch_sync_notification_policy.test.ts | 68 +++++ .../policies/branch_sync_message_catalog.ts | 12 + .../branch_sync_notification_policy.ts | 93 ++++++- src/application/ports/branch_sync_ports.ts | 36 --- .../push_single_action_contexts.test.ts | 13 +- .../observe_branch_sync_use_case.test.ts | 152 ++++++++++- .../actions/observe_branch_sync_use_case.ts | 69 ++++- .../usecases/push_single_action_contexts.ts | 6 +- .../github_publication_mutation_baseline.json | 2 +- ...gle_action_capability_port_binding.test.ts | 7 - .../main_run_route_composition_root.ts | 3 +- ...h_single_action_capability_port_binding.ts | 13 - 22 files changed, 1043 insertions(+), 194 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index cc1c6cb22..12414ca97 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -40675,6 +40675,10 @@ exports.BRANCH_SYNC_MESSAGE_IDS = Object.freeze([ 'branchSync.aligned.heading', 'branchSync.aligned.status', 'branchSync.aligned.resolved', + 'branchSync.transition.required', + 'branchSync.transition.openStatus', + 'branchSync.transition.duplicate', + 'branchSync.transition.viewOriginal', ]); const ENGLISH_MESSAGES = Object.freeze({ 'branchSync.stale.heading': 'Action required: synchronize the branch', @@ -40691,6 +40695,10 @@ const ENGLISH_MESSAGES = Object.freeze({ 'branchSync.aligned.heading': 'Branch synchronized', 'branchSync.aligned.status': '{workingBranch} now contains the current history of its parent branch {parentBranch}.', 'branchSync.aligned.resolved': 'The previous synchronization recommendation has been resolved.', + 'branchSync.transition.required': 'Branch synchronization needs attention: {workingBranch} is behind {parentBranch}.', + 'branchSync.transition.openStatus': 'Open the current status', + 'branchSync.transition.duplicate': 'A duplicate action notification was suppressed.', + 'branchSync.transition.viewOriginal': 'View the original notification', }); const SPANISH_MESSAGES = Object.freeze({ 'branchSync.stale.heading': 'Acción necesaria: sincroniza la rama', @@ -40709,6 +40717,10 @@ const SPANISH_MESSAGES = Object.freeze({ 'branchSync.aligned.heading': 'Rama sincronizada', 'branchSync.aligned.status': '{workingBranch} ya contiene el historial actual de su rama padre {parentBranch}.', 'branchSync.aligned.resolved': 'La recomendación de sincronización anterior está resuelta.', + 'branchSync.transition.required': 'La sincronización de la rama necesita atención: {workingBranch} está por detrás de {parentBranch}.', + 'branchSync.transition.openStatus': 'Abrir el estado actual', + 'branchSync.transition.duplicate': 'Se ha suprimido una notificación de acción duplicada.', + 'branchSync.transition.viewOriginal': 'Ver la notificación original', }); exports.ENGLISH_BRANCH_SYNC_DEFINITION = Object.freeze({ version: message_catalog_1.MESSAGE_CATALOG_VERSION, @@ -40745,11 +40757,18 @@ Object.defineProperty(exports, "__esModule", ({ value: true })); exports.BRANCH_SYNC_ALIGNED_MARKER = exports.BRANCH_SYNC_STALE_MARKER = void 0; exports.selectBranchDependenciesForPush = selectBranchDependenciesForPush; exports.findLatestBranchSyncComment = findLatestBranchSyncComment; +exports.branchSyncPublicationIdentity = branchSyncPublicationIdentity; +exports.buildBranchSyncTransitionIntent = buildBranchSyncTransitionIntent; +exports.buildBranchSyncTransitionNotification = buildBranchSyncTransitionNotification; +exports.buildBranchSyncDuplicatePointer = buildBranchSyncDuplicatePointer; +exports.buildBranchSyncStatusCommentUrl = buildBranchSyncStatusCommentUrl; exports.isStaleBranchSyncComment = isStaleBranchSyncComment; exports.buildStaleBranchSyncComment = buildStaleBranchSyncComment; exports.buildAlignedBranchSyncComment = buildAlignedBranchSyncComment; const github_user_policy_1 = __nccwpck_require__(84403); +const git_object_id_1 = __nccwpck_require__(88623); const publication_identity_policy_1 = __nccwpck_require__(45403); +const github_comment_publication_policy_1 = __nccwpck_require__(72712); exports.BRANCH_SYNC_STALE_MARKER = ''; exports.BRANCH_SYNC_ALIGNED_MARKER = ''; const BRANCH_SYNC_KEY_MARKER = ''; exports.BRANCH_SYNC_ALIGNED_MARKER = ''; const BRANCH_SYNC_KEY_MARKER = ' ${values?.workingBranch}`; + if (id === 'branchSync.transition.openStatus') return '[Open](https://evil.example)'; + return '@team\n/fix'; + }), + } as typeof english; + const url = buildBranchSyncStatusCommentUrl('org/name', 'repo name', 42, 8); + const rendered = buildBranchSyncTransitionNotification(dependency, hostile, url); + expect(rendered).toContain('@\u200bteam'); + expect(rendered).toContain('\u200b/fix'); + expect(rendered).toContain('<!-- forged -->'); + expect(rendered).not.toContain('evil.example'); + const emptyLabels = { ...english, message: jest.fn(() => '') } as typeof english; + expect(buildBranchSyncTransitionNotification(dependency, emptyLabels, url)).toContain('[Open notification]'); + expect(url).toContain('org%2Fname/repo%20name'); + expect(() => buildBranchSyncStatusCommentUrl('org', 'repo', 0, 8)).toThrow('positive safe integer'); + expect(() => buildBranchSyncStatusCommentUrl('org', 'repo', 42, Number.NaN)).toThrow('positive safe integer'); + }); + it('uses locale-aware singular forms and neutralizes unsafe ref presentation', () => { const unsafe = { ...dependency, workingBranch: 'feature/`@team' }; const stale = buildStaleBranchSyncComment({ @@ -167,6 +231,10 @@ describe('branch sync message catalog', () => { 'branchSync.aligned.heading': 'Branche synchronisée', 'branchSync.aligned.status': '{workingBranch} contient maintenant l’historique actuel de {parentBranch}.', 'branchSync.aligned.resolved': 'La recommandation précédente est résolue.', + 'branchSync.transition.required': 'La synchronisation nécessite une action : {workingBranch} est derrière {parentBranch}.', + 'branchSync.transition.openStatus': 'Ouvrir le statut actuel', + 'branchSync.transition.duplicate': 'Une notification en double a été supprimée.', + 'branchSync.transition.viewOriginal': 'Voir la notification originale', } as const; const query = jest.fn().mockResolvedValue({ targetLocale: 'fr-FR', messages: translated }); diff --git a/src/application/policies/branch_sync_message_catalog.ts b/src/application/policies/branch_sync_message_catalog.ts index 2ad0ce054..e3f3249d9 100644 --- a/src/application/policies/branch_sync_message_catalog.ts +++ b/src/application/policies/branch_sync_message_catalog.ts @@ -20,6 +20,10 @@ export const BRANCH_SYNC_MESSAGE_IDS = Object.freeze([ 'branchSync.aligned.heading', 'branchSync.aligned.status', 'branchSync.aligned.resolved', + 'branchSync.transition.required', + 'branchSync.transition.openStatus', + 'branchSync.transition.duplicate', + 'branchSync.transition.viewOriginal', ] as const); export type BranchSyncMessageId = typeof BRANCH_SYNC_MESSAGE_IDS[number]; @@ -40,6 +44,10 @@ const ENGLISH_MESSAGES: Readonly> = 'branchSync.aligned.heading': 'Branch synchronized', 'branchSync.aligned.status': '{workingBranch} now contains the current history of its parent branch {parentBranch}.', 'branchSync.aligned.resolved': 'The previous synchronization recommendation has been resolved.', + 'branchSync.transition.required': 'Branch synchronization needs attention: {workingBranch} is behind {parentBranch}.', + 'branchSync.transition.openStatus': 'Open the current status', + 'branchSync.transition.duplicate': 'A duplicate action notification was suppressed.', + 'branchSync.transition.viewOriginal': 'View the original notification', }); const SPANISH_MESSAGES: Readonly> = Object.freeze({ @@ -59,6 +67,10 @@ const SPANISH_MESSAGES: Readonly> = 'branchSync.aligned.heading': 'Rama sincronizada', 'branchSync.aligned.status': '{workingBranch} ya contiene el historial actual de su rama padre {parentBranch}.', 'branchSync.aligned.resolved': 'La recomendación de sincronización anterior está resuelta.', + 'branchSync.transition.required': 'La sincronización de la rama necesita atención: {workingBranch} está por detrás de {parentBranch}.', + 'branchSync.transition.openStatus': 'Abrir el estado actual', + 'branchSync.transition.duplicate': 'Se ha suprimido una notificación de acción duplicada.', + 'branchSync.transition.viewOriginal': 'Ver la notificación original', }); export const ENGLISH_BRANCH_SYNC_DEFINITION: MessageCatalogDefinition = Object.freeze({ diff --git a/src/application/policies/branch_sync_notification_policy.ts b/src/application/policies/branch_sync_notification_policy.ts index d5d0080f1..50a6d319f 100644 --- a/src/application/policies/branch_sync_notification_policy.ts +++ b/src/application/policies/branch_sync_notification_policy.ts @@ -1,11 +1,18 @@ import type { BranchDependency, BranchSyncComparison, - BranchSyncNotificationComment, } from '../ports/branch_sync_ports'; +import type { IssueCommentPublicationTarget } from '../ports/issue_lifecycle_ports'; import { githubUsersMatch } from '../../domain/github_user_policy'; -import { buildPublicationMarker, createSemanticDigest } from './publication_identity_policy'; +import type { PublicationIdentity, TransitionPublicationIntent } from '../../domain/github_publication'; +import { canonicalGitObjectId } from '../../domain/git_object_id'; +import { + buildPublicationMarker, + createSemanticDigest, + createTransitionFingerprint, +} from './publication_identity_policy'; import type { BranchSyncMessageCatalog } from './branch_sync_message_catalog'; +import { sanitizeAgentMarkdown } from './github_comment_publication_policy'; export const BRANCH_SYNC_STALE_MARKER = ''; export const BRANCH_SYNC_ALIGNED_MARKER = ''; @@ -31,10 +38,10 @@ export function selectBranchDependenciesForPush( } export function findLatestBranchSyncComment( - comments: readonly BranchSyncNotificationComment[], + comments: readonly IssueCommentPublicationTarget[], botLogin?: string, dependency?: BranchDependency, -): BranchSyncNotificationComment | undefined { +): IssueCommentPublicationTarget | undefined { return [...comments] .reverse() .find((comment) => isBranchSyncComment(comment.body) @@ -42,6 +49,66 @@ export function findLatestBranchSyncComment( && Boolean(botLogin && comment.user?.login && githubUsersMatch(botLogin, comment.user.login))); } +export function branchSyncPublicationIdentity(dependency: BranchDependency): Readonly { + return Object.freeze({ + topic: 'branch-sync', + target: Object.freeze({ kind: 'issue', number: dependency.issueNumber }), + key: `dependency:${createSemanticDigest({ parent: dependency.parentBranch, working: dependency.workingBranch })}`, + }); +} + +export function buildBranchSyncTransitionIntent( + dependency: BranchDependency, + sourceHeadSha: string, + locale: string, +): Readonly { + const canonicalHead = canonicalGitObjectId(sourceHeadSha); + if (!canonicalHead) throw new Error('Branch synchronization transition requires a canonical source head.'); + const identity = branchSyncPublicationIdentity(dependency); + return Object.freeze({ + kind: 'transition', + identity, + fingerprint: createTransitionFingerprint(identity, 'branch-sync-required', `head:${canonicalHead}`), + messageKey: 'branchSync.transition.required', + locale, + values: Object.freeze({ + parentBranch: dependency.parentBranch, + workingBranch: dependency.workingBranch, + }), + }); +} + +export function buildBranchSyncTransitionNotification( + dependency: BranchDependency, + messages: BranchSyncMessageCatalog, + statusUrl: string, +): string { + return `${safeSentence(messages.message('branchSync.transition.required', { + workingBranch: inlineRef(dependency.workingBranch), + parentBranch: inlineRef(dependency.parentBranch), + }))} [${safeLinkLabel(messages.message('branchSync.transition.openStatus'))}](${statusUrl}).`; +} + +export function buildBranchSyncDuplicatePointer( + messages: BranchSyncMessageCatalog, + canonicalUrl: string, +): string { + return `${safeSentence(messages.message('branchSync.transition.duplicate'))} [${safeLinkLabel(messages.message('branchSync.transition.viewOriginal'))}](${canonicalUrl}).`; +} + +export function buildBranchSyncStatusCommentUrl( + owner: string, + repository: string, + issueNumber: number, + commentId: number, +): string { + if (!Number.isSafeInteger(issueNumber) || issueNumber < 1 + || !Number.isSafeInteger(commentId) || commentId < 1) { + throw new Error('Branch synchronization status link requires positive safe integer identifiers.'); + } + return `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/issues/${issueNumber}#issuecomment-${commentId}`; +} + export function isStaleBranchSyncComment(body: string | null | undefined): boolean { return body?.includes(BRANCH_SYNC_STALE_MARKER) === true; } @@ -104,11 +171,7 @@ function buildSharedBranchSyncMarker( digest: string, ): string { return buildPublicationMarker({ - identity: { - topic: 'branch-sync', - target: { kind: 'issue', number: dependency.issueNumber }, - key: `dependency:${createSemanticDigest({ parent: dependency.parentBranch, working: dependency.workingBranch })}`, - }, + identity: branchSyncPublicationIdentity(dependency), sourceVersion, digest, }); @@ -143,3 +206,15 @@ function buildCompareUrl( function inlineRef(value: string): string { return `\`${value.replace(/[\r\n`<>]/gu, '').replace(/@/gu, '@\u200b').slice(0, 255)}\``; } + +function safeSentence(value: string): string { + return sanitizeAgentMarkdown(value, 120).replace(/[\r\n]+/gu, ' ').trim(); +} + +function safeLinkLabel(value: string): string { + return sanitizeAgentMarkdown(value, 40) + .replace(/\[([^\]]*)\]\([^)]*\)/gu, '$1') + .replace(/https?:\/\/\S+/giu, '') + .replace(/[\r\n()[\]<>]/gu, '') + .trim() || 'Open notification'; +} diff --git a/src/application/ports/branch_sync_ports.ts b/src/application/ports/branch_sync_ports.ts index cc4ba7eb7..b9f0551b9 100644 --- a/src/application/ports/branch_sync_ports.ts +++ b/src/application/ports/branch_sync_ports.ts @@ -38,36 +38,6 @@ export interface BranchSyncComparisonPort { ): Promise; } -export interface BranchSyncNotificationComment { - readonly id: number; - readonly body: string | null; - readonly user?: { readonly login?: string }; -} - -export interface BranchSyncNotificationPort { - listIssueComments( - owner: string, - repository: string, - issueNumber: number, - token: string, - ): Promise; - addComment( - owner: string, - repository: string, - issueNumber: number, - comment: string, - token: string, - ): Promise; - updateComment( - owner: string, - repository: string, - issueNumber: number, - commentId: number, - comment: string, - token: string, - ): Promise; -} - export type BranchMergePreparation = | { readonly kind: "aligned"; @@ -125,12 +95,6 @@ export interface BoundBranchSyncComparisonPort { compare(parentBranch: string, workingBranch: string): Promise; } -export interface BoundBranchSyncNotificationPort { - listIssueComments(issueNumber: number): Promise; - addComment(issueNumber: number, comment: string): Promise; - updateComment(issueNumber: number, commentId: number, comment: string): Promise; -} - export interface BoundBranchSyncWorkspacePort { prepare(parentBranch: string, workingBranch: string): Promise; validatePreparedMerge(conflictPaths: readonly string[]): Promise; diff --git a/src/application/usecases/__tests__/push_single_action_contexts.test.ts b/src/application/usecases/__tests__/push_single_action_contexts.test.ts index e5b95e0f9..7c5ea394f 100644 --- a/src/application/usecases/__tests__/push_single_action_contexts.test.ts +++ b/src/application/usecases/__tests__/push_single_action_contexts.test.ts @@ -223,10 +223,21 @@ describe('push and single-action context projection', () => { const input = source(); input.inputs = { ...input.inputs, after }; - expect(projectBranchObservationContext(input)).toMatchObject({ + const projected = projectBranchObservationContext(input); + expect(projected).toMatchObject({ pushedBranch: 'feature/42-contexts', deletedPush, trustedBotLogin: 'copilot-bot', repository: { owner: 'owner', name: 'repo' }, }); + expect(projected.sourceHeadSha).toBe(deletedPush ? undefined : after); + }); + + it('canonicalizes the branch-observation source head and rejects malformed object ids', () => { + const input = source(); + input.inputs!.after = 'A'.repeat(64); + expect(projectBranchObservationContext(input).sourceHeadSha).toBe('a'.repeat(64)); + + input.inputs!.after = 'not-a-git-object-id'; + expect(projectBranchObservationContext(input)).not.toHaveProperty('sourceHeadSha'); }); it('omits absent branch-observation actor and classifies a missing after SHA as active', () => { diff --git a/src/application/usecases/actions/__tests__/observe_branch_sync_use_case.test.ts b/src/application/usecases/actions/__tests__/observe_branch_sync_use_case.test.ts index 8951c0cf6..c8eb4095d 100644 --- a/src/application/usecases/actions/__tests__/observe_branch_sync_use_case.test.ts +++ b/src/application/usecases/actions/__tests__/observe_branch_sync_use_case.test.ts @@ -1,9 +1,17 @@ import type { Execution } from "../../../../data/model/execution"; -import { BRANCH_SYNC_ALIGNED_MARKER, BRANCH_SYNC_STALE_MARKER } from "../../../policies/branch_sync_notification_policy"; +import { + BRANCH_SYNC_ALIGNED_MARKER, + BRANCH_SYNC_STALE_MARKER, + buildAlignedBranchSyncComment, + buildBranchSyncTransitionIntent, + buildStaleBranchSyncComment, +} from "../../../policies/branch_sync_notification_policy"; import { ObserveBranchSyncUseCase } from "../observe_branch_sync_use_case"; import { projectBranchObservationContext } from '../../push_single_action_contexts'; import type { MessageCatalogResolutionPort } from '../../../ports/message_catalog_ports'; import { ENGLISH_BRANCH_SYNC_DEFINITION } from '../../../policies/branch_sync_message_catalog'; +import { resolveStaticBranchSyncCatalog } from '../../../policies/branch_sync_message_catalog'; +import { renderTransitionNotification } from '../../steps/common/transition_notification_workflow'; const dependency = { issueNumber: 42, parentBranch: "develop", workingBranch: "feature/42" }; @@ -20,18 +28,42 @@ function execution(overrides: Record = {}) { } as unknown as Execution); } -function setup(input: { behindBy?: number; comments?: unknown[]; resolver?: MessageCatalogResolutionPort } = {}) { +function setup(input: { + behindBy?: number; + comments?: Array<{ id: number; body: string | null; user?: { login?: string } }>; + resolver?: MessageCatalogResolutionPort; + removal?: 'removed' | 'compaction-required'; +} = {}) { const dependencies = { listOpenDependencies: jest.fn().mockResolvedValue([dependency]), resolveTarget: jest.fn(), }; const comparisons = { compare: jest.fn().mockResolvedValue({ aheadBy: 1, behindBy: input.behindBy ?? 2 }) }; + const stored = [...(input.comments ?? [])]; + let nextId = Math.max(0, ...stored.map(comment => comment.id)) + 1; const notifications = { - listIssueComments: jest.fn().mockResolvedValue(input.comments ?? []), - addComment: jest.fn().mockResolvedValue(undefined), - updateComment: jest.fn().mockResolvedValue(undefined), + listIssueComments: jest.fn().mockImplementation(async () => stored.map(comment => ({ ...comment }))), + addComment: jest.fn().mockImplementation(async (_issueNumber: number, body: string) => { + stored.push({ id: nextId++, body, user: { login: 'vypbot' } }); + }), + updateComment: jest.fn().mockImplementation(async (_issueNumber: number, commentId: number, body: string) => { + const index = stored.findIndex(comment => comment.id === commentId); + if (index >= 0) stored[index] = { ...stored[index], body }; + }), + removeComment: jest.fn().mockImplementation(async (_issueNumber: number, commentId: number) => { + if (input.removal === 'compaction-required') return 'compaction-required' as const; + const index = stored.findIndex(comment => comment.id === commentId); + if (index >= 0) stored.splice(index, 1); + return 'removed' as const; + }), + }; + return { + dependencies, + comparisons, + notifications, + stored, + useCase: new ObserveBranchSyncUseCase(dependencies, comparisons, notifications, input.resolver), }; - return { dependencies, comparisons, notifications, useCase: new ObserveBranchSyncUseCase(dependencies, comparisons, notifications, input.resolver) }; } describe("ObserveBranchSyncUseCase", () => { @@ -57,6 +89,114 @@ describe("ObserveBranchSyncUseCase", () => { expect(context.notifications.addComment).not.toHaveBeenCalled(); }); + it('skips an unchanged stale-card update and creates no transition notification', async () => { + const body = buildStaleBranchSyncComment({ + owner: 'org', repository: 'repo', dependency, + comparison: { aheadBy: 1, behindBy: 2 }, + messages: resolveStaticBranchSyncCatalog('en-US'), + }); + const context = setup({ comments: [{ id: 8, body, user: { login: 'vypbot' } }] }); + + await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + + expect(context.notifications.updateComment).not.toHaveBeenCalled(); + expect(context.notifications.addComment).not.toHaveBeenCalled(); + }); + + it('updates a changed stale projection without creating another timeline notification', async () => { + const body = buildStaleBranchSyncComment({ + owner: 'org', repository: 'repo', dependency, + comparison: { aheadBy: 0, behindBy: 1 }, + messages: resolveStaticBranchSyncCatalog('en-US'), + }); + const context = setup({ comments: [{ id: 8, body, user: { login: 'vypbot' } }] }); + + await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + + expect(context.notifications.updateComment).toHaveBeenCalledTimes(1); + expect(context.notifications.addComment).not.toHaveBeenCalled(); + }); + + it('publishes one short notification when an aligned card becomes stale', async () => { + const aligned = buildAlignedBranchSyncComment(dependency, resolveStaticBranchSyncCatalog('en-US')); + const context = setup({ comments: [{ id: 8, body: aligned, user: { login: 'vypbot' } }] }); + + const results = await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + + expect(context.notifications.updateComment).toHaveBeenCalledWith(42, 8, expect.stringContaining(BRANCH_SYNC_STALE_MARKER)); + expect(context.notifications.addComment).toHaveBeenCalledTimes(1); + expect(context.notifications.addComment).toHaveBeenCalledWith( + 42, + expect.stringContaining('[Open the current status](https://github.com/org/repo/issues/42#issuecomment-8)'), + ); + expect(results[0]).toMatchObject({ + success: true, + payload: { publicationTransition: { topic: 'branch-sync', target: 'issue:42', effect: 'created' } }, + }); + + await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + expect(context.notifications.addComment).toHaveBeenCalledTimes(1); + expect(context.notifications.updateComment).toHaveBeenCalledTimes(1); + }); + + it('updates aligned state without notifying when a trustworthy source head is unavailable', async () => { + const aligned = buildAlignedBranchSyncComment(dependency, resolveStaticBranchSyncCatalog('en-US')); + const context = setup({ comments: [{ id: 8, body: aligned, user: { login: 'vypbot' } }] }); + + await context.useCase.invoke(execution()); + + expect(context.notifications.updateComment).toHaveBeenCalledTimes(1); + expect(context.notifications.addComment).not.toHaveBeenCalled(); + }); + + it('reuses an exact transition and records compacted duplicate evidence', async () => { + const messages = resolveStaticBranchSyncCatalog('en-US'); + const aligned = buildAlignedBranchSyncComment(dependency, messages); + const intent = buildBranchSyncTransitionIntent(dependency, 'a'.repeat(40), 'en-US'); + const transition = renderTransitionNotification(intent, 'Existing notification.'); + const context = setup({ + comments: [ + { id: 8, body: aligned, user: { login: 'vypbot' } }, + { id: 9, body: transition, user: { login: 'vypbot' } }, + { id: 10, body: transition, user: { login: 'VYPBOT' } }, + ], + removal: 'compaction-required', + }); + + const results = await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + + expect(context.notifications.addComment).not.toHaveBeenCalled(); + expect(context.notifications.removeComment).toHaveBeenCalledWith(42, 10); + expect(context.notifications.updateComment).toHaveBeenCalledWith( + 42, 10, expect.stringContaining('[View the original notification]'), + ); + expect(results[0]).toMatchObject({ + success: true, + payload: { + publicationTransition: { effect: 'unchanged' }, + publicationCleanup: { compactedCommentIds: [10], compactedCount: 1 }, + }, + }); + }); + + it('preserves the updated stale card and reports a publication-only failure', async () => { + const aligned = buildAlignedBranchSyncComment(dependency, resolveStaticBranchSyncCatalog('en-US')); + const context = setup({ comments: [{ id: 8, body: aligned, user: { login: 'vypbot' } }] }); + context.notifications.addComment.mockRejectedValueOnce(new Error('secret provider detail')); + + const first = await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + expect(first[0]).toMatchObject({ + success: false, + payload: { state: 'stale', statusUpdated: true }, + }); + expect(first[0].steps[0]).toContain('status was updated'); + expect(JSON.stringify(first)).not.toContain('secret provider detail'); + + const replay = await context.useCase.invoke(execution({ inputs: { after: 'a'.repeat(40) } })); + expect(replay[0]).toMatchObject({ success: true, payload: { state: 'stale' } }); + expect(context.notifications.addComment).toHaveBeenCalledTimes(1); + }); + it("resolves the prior warning once the branch is aligned", async () => { const context = setup({ behindBy: 0, diff --git a/src/application/usecases/actions/observe_branch_sync_use_case.ts b/src/application/usecases/actions/observe_branch_sync_use_case.ts index a89523367..642c4605b 100644 --- a/src/application/usecases/actions/observe_branch_sync_use_case.ts +++ b/src/application/usecases/actions/observe_branch_sync_use_case.ts @@ -1,6 +1,10 @@ import { Result } from "../../../data/model/result"; import { buildAlignedBranchSyncComment, + buildBranchSyncDuplicatePointer, + buildBranchSyncStatusCommentUrl, + buildBranchSyncTransitionIntent, + buildBranchSyncTransitionNotification, buildStaleBranchSyncComment, findLatestBranchSyncComment, isStaleBranchSyncComment, @@ -10,8 +14,8 @@ import type { BranchDependency, BoundBranchDependencyQueryPort, BoundBranchSyncComparisonPort, - BoundBranchSyncNotificationPort, } from "../../ports/branch_sync_ports"; +import type { BoundIssueCommentPublicationPort } from '../../ports/issue_lifecycle_ports'; import type { BranchObservationContext } from '../push_single_action_contexts'; import { logError, logInfo } from "../../ports/logging_ports"; import type { ParamUseCase } from "../base/param_usecase"; @@ -21,6 +25,11 @@ import { resolveBranchSyncCatalog, type BranchSyncMessageCatalog, } from '../../policies/branch_sync_message_catalog'; +import { reconcileTransitionNotification } from '../steps/common/transition_notification_workflow'; +import { + buildDuplicateCompactionPublicationPayload, + buildTransitionPublicationPayload, +} from '../../policies/publication_outcome_policy'; const TASK_ID = "ObserveBranchSyncUseCase"; @@ -34,7 +43,7 @@ export class ObserveBranchSyncUseCase implements ParamUseCase buildBranchSyncDuplicatePointer(messages, canonicalUrl), + }, this.notifications); + const cleanup = buildDuplicateCompactionPublicationPayload(transition.compactedCommentIds); + return success(dependency, comparison.behindBy, "stale", { + ...buildTransitionPublicationPayload(intent, transition.effect), + ...(cleanup ?? {}), + }); + } catch (cause) { + return failure( + `Branch status was updated for issue #${dependency.issueNumber}, but its action notification could not be published.`, + cause, + { ...dependency, behindBy: comparison.behindBy, state: 'stale', statusUpdated: true }, + ); + } } if (latest && isStaleBranchSyncComment(latest.body)) { @@ -130,6 +177,7 @@ function success( dependency: BranchDependency, behindBy: number, state: "stale" | "aligned", + evidence: Readonly> = {}, ): Result { return new Result({ id: TASK_ID, @@ -140,16 +188,17 @@ function success( ? `Issue #${dependency.issueNumber}: ${dependency.workingBranch} is ${behindBy} commit(s) behind ${dependency.parentBranch}.` : `Issue #${dependency.issueNumber}: ${dependency.workingBranch} is aligned with ${dependency.parentBranch}.`, ], - payload: { ...dependency, behindBy, state }, + payload: { ...dependency, behindBy, state, ...evidence }, }); } -function failure(message: string, cause: unknown): Result { +function failure(message: string, cause: unknown, payload?: Readonly>): Result { return new Result({ id: TASK_ID, success: false, executed: true, steps: [message], errors: [toApplicationError(cause, 'provider.unavailable', message)], + ...(payload ? { payload } : {}), }); } diff --git a/src/application/usecases/push_single_action_contexts.ts b/src/application/usecases/push_single_action_contexts.ts index 5ca76527a..d0423fd07 100644 --- a/src/application/usecases/push_single_action_contexts.ts +++ b/src/application/usecases/push_single_action_contexts.ts @@ -66,6 +66,7 @@ export interface InactivityContext { export interface BranchObservationContext { readonly pushedBranch: string; readonly deletedPush: boolean; + readonly sourceHeadSha?: string; readonly trustedBotLogin?: string; readonly repository: { readonly owner: string; readonly name: string }; readonly locale: string; @@ -320,9 +321,12 @@ export function projectInactivityContext(source: PushSingleActionContextSource): } export function projectBranchObservationContext(source: PushSingleActionContextSource): BranchObservationContext { + const deletedPush = typeof source.inputs?.after === 'string' && /^0+$/u.test(source.inputs.after); + const sourceHeadSha = deletedPush ? undefined : canonicalGitObjectId(source.inputs?.after); return Object.freeze({ pushedBranch: source.commit.branch.trim(), - deletedPush: typeof source.inputs?.after === 'string' && /^0+$/u.test(source.inputs.after), + deletedPush, + ...(sourceHeadSha ? { sourceHeadSha } : {}), ...(source.tokenUser ? { trustedBotLogin: source.tokenUser } : {}), repository: Object.freeze({ owner: source.owner, name: source.repo }), locale: source.locale?.issue ?? 'en-US', diff --git a/src/architecture/github_publication_mutation_baseline.json b/src/architecture/github_publication_mutation_baseline.json index 22d52c00e..f6a2db8de 100644 --- a/src/architecture/github_publication_mutation_baseline.json +++ b/src/architecture/github_publication_mutation_baseline.json @@ -1,7 +1,7 @@ { "entries": [ { "file": "src/application/usecases/actions/close_inactive_issues_workflow.ts", "addComment": 1, "reason": "Publishes the terminal inactivity policy after a successful native close." }, - { "file": "src/application/usecases/actions/observe_branch_sync_use_case.ts", "addComment": 1, "updateComment": 2, "reason": "Reconciles the feature-owned branch synchronization card." }, + { "file": "src/application/usecases/actions/observe_branch_sync_use_case.ts", "addComment": 1, "updateComment": 2, "reason": "Reconciles the feature-owned branch synchronization card and delegates actionable transitions to the shared publisher." }, { "file": "src/application/usecases/actions/publish_issue_comment_workflow.ts", "addComment": 1, "updateComment": 1, "reason": "Executes the explicitly requested caller-supplied comment mutation." }, { "file": "src/application/usecases/steps/commit/bugbot/publish_issue_finding_comment.ts", "addComment": 1, "updateComment": 1, "reason": "Creates or updates one Bugbot issue finding." }, { "file": "src/application/usecases/steps/commit/bugbot/publish_overflow_comment.ts", "addComment": 1, "reason": "Publishes the bounded Bugbot overflow record." }, diff --git a/src/infrastructure/composition/__tests__/push_single_action_capability_port_binding.test.ts b/src/infrastructure/composition/__tests__/push_single_action_capability_port_binding.test.ts index b81083d28..36e64109c 100644 --- a/src/infrastructure/composition/__tests__/push_single_action_capability_port_binding.test.ts +++ b/src/infrastructure/composition/__tests__/push_single_action_capability_port_binding.test.ts @@ -4,7 +4,6 @@ import { bindBranchComparison, bindBranchDependencies, bindBranchListQuery, - bindBranchSyncNotification, bindBranchSyncWorkspace, bindDeploymentContinuation, bindDeploymentGit, @@ -280,7 +279,6 @@ describe('push and single-action capability binding', () => { const issuePushPort = { openIssue: jest.fn() }; const inactivityPort = { listOpenIssuesByLabel: jest.fn(), getOpenIssue: jest.fn() }; const dependencyPort = { listOpenDependencies: jest.fn(), resolveTarget: jest.fn() }; - const notificationPort = { listIssueComments: jest.fn(), addComment: jest.fn(), updateComment: jest.fn() }; const defaultBranch = bindRepositoryDefaultBranch({ getDefaultBranch: jest.fn() } as never, binding); const issuePush = bindIssueReopen(issuePushPort as never, binding); const branches = bindBranchListQuery({ getListOfBranches: jest.fn() } as never, binding); @@ -290,7 +288,6 @@ describe('push and single-action capability binding', () => { const size = bindBranchChangeSize({ getSizeCategoryAndReason: jest.fn() } as never, binding); const dependencies = bindBranchDependencies(dependencyPort as never, binding); const comparison = bindBranchComparison({ compare: jest.fn() } as never, binding); - const notifications = bindBranchSyncNotification(notificationPort as never, binding); await defaultBranch.getDefaultBranch(); await issuePush.openIssue(42); @@ -303,13 +300,9 @@ describe('push and single-action capability binding', () => { await dependencies.listOpenDependencies(); await dependencies.resolveTarget(42); await comparison.compare('develop', 'feature/42'); - await notifications.listIssueComments(42); - await notifications.addComment(42, 'body'); - await notifications.updateComment(42, 7, 'updated'); expect(issuePushPort.openIssue).toHaveBeenCalledWith('owner', 'repo', 42, 'secret-token'); expect(dependencyPort.resolveTarget).toHaveBeenCalledWith('owner', 'repo', 42, 'secret-token'); - expect(notificationPort.updateComment).toHaveBeenCalledWith('owner', 'repo', 42, 7, 'updated', 'secret-token'); }); it('forwards setup provisioning and every credential-bearing workspace operation', async () => { diff --git a/src/infrastructure/composition/main_run_route_composition_root.ts b/src/infrastructure/composition/main_run_route_composition_root.ts index 293f927a8..b03b34fbc 100644 --- a/src/infrastructure/composition/main_run_route_composition_root.ts +++ b/src/infrastructure/composition/main_run_route_composition_root.ts @@ -82,7 +82,6 @@ import { bindBranchChangeSize, bindBranchComparison, bindBranchDependencies, - bindBranchSyncNotification, bindBranchSyncWorkspace, bindDeploymentContinuation, bindDeploymentGit, @@ -150,7 +149,7 @@ export function createSingleActionUseCaseCompositionRoot( new ObserveBranchSyncUseCase( bindBranchDependencies(new BranchDependencyRepository(createGraphqlTransportClient()), binding), bindBranchComparison(new BranchCompareRepository(createBranchComparisonClient()), binding), - bindBranchSyncNotification(issueDescriptionQueryPort, binding), + bindIssueCommentPublication(issueDescriptionQueryPort, binding), catalogResolver, ), deploymentOrchestration, diff --git a/src/infrastructure/composition/push_single_action_capability_port_binding.ts b/src/infrastructure/composition/push_single_action_capability_port_binding.ts index 771c5eb13..9038207d2 100644 --- a/src/infrastructure/composition/push_single_action_capability_port_binding.ts +++ b/src/infrastructure/composition/push_single_action_capability_port_binding.ts @@ -4,11 +4,9 @@ import type { BranchListQueryPort, BoundBranchListQueryPort } from '../../applic import type { BoundBranchDependencyQueryPort, BoundBranchSyncComparisonPort, - BoundBranchSyncNotificationPort, BoundBranchSyncWorkspacePort, BranchDependencyQueryPort, BranchSyncComparisonPort, - BranchSyncNotificationPort, BranchSyncWorkspacePort, } from '../../application/ports/branch_sync_ports'; import type { @@ -332,17 +330,6 @@ export function bindBranchComparison( }); } -export function bindBranchSyncNotification( - port: BranchSyncNotificationPort, - binding: RepositoryCredentialBinding, -): BoundBranchSyncNotificationPort { - return Object.freeze({ - listIssueComments: (issueNumber) => port.listIssueComments(binding.owner, binding.repository, issueNumber, binding.token), - addComment: (issueNumber, comment) => port.addComment(binding.owner, binding.repository, issueNumber, comment, binding.token), - updateComment: (issueNumber, commentId, comment) => port.updateComment(binding.owner, binding.repository, issueNumber, commentId, comment, binding.token), - }); -} - export function bindBranchSyncWorkspace( port: BranchSyncWorkspacePort, binding: RepositoryCredentialBinding, From cc24a224166508310f92035e15eb2d578377c41d Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Tue, 15 Sep 2026 11:24:30 +0200 Subject: [PATCH 2/2] codex-branch-sync-action-notifications: link rollout evidence --- specs/branch-synchronization-and-conflict-recovery.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/branch-synchronization-and-conflict-recovery.md b/specs/branch-synchronization-and-conflict-recovery.md index de70b1f22..12b2540f6 100644 --- a/specs/branch-synchronization-and-conflict-recovery.md +++ b/specs/branch-synchronization-and-conflict-recovery.md @@ -5,7 +5,7 @@ - Last verified: 2026-09-15 - Owners: Copilot maintainers - Scope: all-branch drift observation and authorized parent-to-working-branch synchronization -- Related issues/PRs: managed issue lifecycle, semantic GitHub publication, and agent runtime SDDs +- Related issues/PRs: [PR #387](https://github.com/vypdev/copilot/pull/387), managed issue lifecycle, semantic GitHub publication, and agent runtime SDDs - Required review gates: product UX, architecture, testing, documentation, security/operations - Open decisions blocking readiness: none