diff --git a/cdk/src/handlers/jira-webhook-processor.ts b/cdk/src/handlers/jira-webhook-processor.ts index 9ee28c82..cee4dfe6 100644 --- a/cdk/src/handlers/jira-webhook-processor.ts +++ b/cdk/src/handlers/jira-webhook-processor.ts @@ -455,22 +455,18 @@ export async function handler(event: ProcessorEvent): Promise { } let orchestrationChildren: readonly SubIssueNode[] | undefined; + let existingOrchestration = false; if (ORCHESTRATION_TABLE && resolvedJira) { - // Layer #574 deliberately freezes an orchestration after its first seed. - // Additive re-discovery is introduced by #578; until then, a re-trigger is - // an idempotent no-op and must not create a parent task. + // Re-read an existing orchestration's authored graph so genuinely-new Jira + // subtasks can be appended. The flag also prevents parent attachments from + // being uploaded again below: the meta row already pins the first seed's S3 + // versions, and replacing them would eventually expire those pinned objects. const existing = await loadOrchestration( ddb, ORCHESTRATION_TABLE, deriveOrchestrationId(issue.key), ); - if (existing) { - logger.info('Jira orchestration already exists — skipping re-trigger', { - issue_key: issue.key, - orchestration_id: existing.meta.orchestration_id, - }); - return; - } + existingOrchestration = existing !== null; const graphResult = await jiraGraphSource( resolvedJira.accessToken, @@ -485,6 +481,12 @@ export async function handler(event: ProcessorEvent): Promise { ); return; } + if (graphResult.kind === 'no_children' && existingOrchestration) { + logger.info('Jira orchestration re-trigger has no current subtasks — no-op', { + issue_key: issue.key, + }); + return; + } if (graphResult.kind === 'ok') { const routed = await routeJiraOrchestrationChildren({ cloudId, @@ -515,7 +517,7 @@ export async function handler(event: ProcessorEvent): Promise { // fail-closed (a selected-but-unscreenable attachment rejects the task). let comments: RenderedComment[] = []; let preScreenedAttachments: PassedAttachmentRecord[] = []; - if (WORKSPACE_REGISTRY_TABLE) { + if (WORKSPACE_REGISTRY_TABLE && !existingOrchestration) { const tenantCtx = { cloudId, registryTableName: WORKSPACE_REGISTRY_TABLE }; // Recent human comments — advisory context, never gate task creation. @@ -612,9 +614,9 @@ export async function handler(event: ProcessorEvent): Promise { } // A concurrent replay can win between the preflight read and the seed - // condition. Do not create a parent task or retain duplicate attachments. - if (discovery.kind === 'extended' - || (discovery.kind === 'seeded' && discovery.alreadyExisted)) { + // condition. The shared discovery path returns that race as an empty extend; + // any attachment objects uploaded by this invocation are duplicates. + if (discovery.kind === 'seeded' && discovery.alreadyExisted) { if (preScreenedAttachments.length > 0 && s3Client && ATTACHMENTS_BUCKET) { await cleanupPreScreenedAttachments(s3Client, ATTACHMENTS_BUCKET, preScreenedAttachments); } @@ -682,6 +684,85 @@ export async function handler(event: ProcessorEvent): Promise { }); return; } + if (discovery.kind === 'extended') { + if (discovery.addedSubIssueIds.length === 0) { + logger.info('Jira orchestration re-trigger added no new subtasks', { + issue_key: issue.key, + orchestration_id: discovery.orchestrationId, + }); + return; + } + + const snapshot = await loadOrchestration(ddb, ORCHESTRATION_TABLE, discovery.orchestrationId); + if (snapshot) { + const releasableRows = snapshot.children.filter( + (child) => discovery.releasableSubIssueIds.includes(child.sub_issue_id) + && child.child_status === 'ready', + ); + if (releasableRows.length > 0) { + const now = new Date().toISOString(); + const results = await releaseReadyChildren( + ddb, + ORCHESTRATION_TABLE, + releasableRows, + snapshot.meta.release_context, + createTaskCore, + now, + snapshot.children, + ); + await applyTerminalCreateFailures( + ddb, + ORCHESTRATION_TABLE, + discovery.orchestrationId, + snapshot.children, + results, + now, + ); + } + + if (WORKSPACE_REGISTRY_TABLE) { + try { + const fresh = await loadOrchestration( + ddb, + ORCHESTRATION_TABLE, + discovery.orchestrationId, + ); + const panelSnapshot = fresh ?? snapshot; + const commentId = await upsertEpicPanel({ + channel: makeJiraChannel(WORKSPACE_REGISTRY_TABLE), + parent: { issueId: issue.key, credentialsRef: cloudId }, + ...(panelSnapshot.meta.status_comment_id && { + statusCommentId: panelSnapshot.meta.status_comment_id, + }), + children: panelSnapshot.children, + inProgress: true, + labelFilter, + }); + if (commentId && !panelSnapshot.meta.status_comment_id) { + await setStatusCommentId( + ddb, + ORCHESTRATION_TABLE, + discovery.orchestrationId, + commentId, + ); + } + } catch (err) { + logger.warn('Failed to refresh Jira orchestration panel on extend (non-fatal)', { + issue_key: issue.key, + orchestration_id: discovery.orchestrationId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } + logger.info('Jira orchestration extended with new subtasks', { + issue_key: issue.key, + orchestration_id: discovery.orchestrationId, + added_count: discovery.addedSubIssueIds.length, + releasable_count: discovery.releasableSubIssueIds.length, + }); + return; + } } const requestId = crypto.randomUUID(); diff --git a/cdk/test/handlers/jira-webhook-processor-orchestration.test.ts b/cdk/test/handlers/jira-webhook-processor-orchestration.test.ts index 4d035c6d..47a4ee37 100644 --- a/cdk/test/handlers/jira-webhook-processor-orchestration.test.ts +++ b/cdk/test/handlers/jira-webhook-processor-orchestration.test.ts @@ -318,13 +318,122 @@ describe('jira-webhook-processor orchestration adapter', () => { ); }); - test('makes an existing Jira orchestration re-trigger inert in the first layer', async () => { + test('treats an existing orchestration with no new node IDs as a no-op', async () => { loadOrchestrationMock.mockReset(); loadOrchestrationMock.mockResolvedValueOnce(snapshot); + discoverOrchestrationMock.mockResolvedValueOnce({ + kind: 'extended', + orchestrationId: 'orch-ENG-1', + addedSubIssueIds: [], + releasableSubIssueIds: [], + }); + + await handler(event()); + + expect(jiraGraphSourceMock).toHaveBeenCalledTimes(1); + expect(discoverOrchestrationMock).toHaveBeenCalledTimes(1); + expect(releaseReadyChildrenMock).not.toHaveBeenCalled(); + expect(upsertEpicPanelMock).not.toHaveBeenCalled(); + expect(createTaskCoreMock).not.toHaveBeenCalled(); + }); + + test('releases only a newly-added root and reopens the existing panel', async () => { + const extendedSnapshot = { + ...snapshot, + meta: { ...snapshot.meta, child_count: 2, status_comment_id: 'panel-1' }, + children: [ + { ...snapshot.children[0], child_status: 'succeeded' }, + { + ...snapshot.children[0], + sub_issue_id: 'ENG-3', + child_status: 'ready', + }, + ], + }; + jiraGraphSourceMock.mockReturnValueOnce(jest.fn().mockResolvedValue({ + kind: 'ok', + children: [child(), child('ENG-3')], + })); + discoverOrchestrationMock.mockResolvedValueOnce({ + kind: 'extended', + orchestrationId: 'orch-ENG-1', + addedSubIssueIds: ['ENG-3'], + releasableSubIssueIds: ['ENG-3'], + }); + loadOrchestrationMock.mockReset(); + loadOrchestrationMock + .mockResolvedValueOnce(snapshot) + .mockResolvedValueOnce(extendedSnapshot) + .mockResolvedValueOnce(extendedSnapshot); + + await handler(event()); + + expect(releaseReadyChildrenMock).toHaveBeenCalledTimes(1); + expect(releaseReadyChildrenMock.mock.calls[0][2]).toEqual([ + expect.objectContaining({ sub_issue_id: 'ENG-3', child_status: 'ready' }), + ]); + expect(releaseReadyChildrenMock.mock.calls[0][6]).toBe(extendedSnapshot.children); + expect(upsertEpicPanelMock).toHaveBeenCalledWith(expect.objectContaining({ + statusCommentId: 'panel-1', + inProgress: true, + children: extendedSnapshot.children, + })); + expect(setStatusCommentIdMock).not.toHaveBeenCalled(); + }); + + test('leaves a newly-added blocked child for the reconciler but refreshes the panel', async () => { + const extendedSnapshot = { + ...snapshot, + meta: { ...snapshot.meta, child_count: 2 }, + children: [ + snapshot.children[0], + { + ...snapshot.children[0], + sub_issue_id: 'ENG-3', + depends_on: ['ENG-2'], + child_status: 'blocked', + }, + ], + }; + jiraGraphSourceMock.mockReturnValueOnce(jest.fn().mockResolvedValue({ + kind: 'ok', + children: [child(), { ...child('ENG-3'), depends_on: ['ENG-2'] }], + })); + discoverOrchestrationMock.mockResolvedValueOnce({ + kind: 'extended', + orchestrationId: 'orch-ENG-1', + addedSubIssueIds: ['ENG-3'], + releasableSubIssueIds: [], + }); + loadOrchestrationMock.mockReset(); + loadOrchestrationMock + .mockResolvedValueOnce(snapshot) + .mockResolvedValueOnce(extendedSnapshot) + .mockResolvedValueOnce(extendedSnapshot); + upsertEpicPanelMock.mockResolvedValueOnce('new-panel'); + + await handler(event()); + + expect(releaseReadyChildrenMock).not.toHaveBeenCalled(); + expect(upsertEpicPanelMock).toHaveBeenCalledWith(expect.objectContaining({ + inProgress: true, + children: extendedSnapshot.children, + })); + expect(setStatusCommentIdMock).toHaveBeenCalledWith( + expect.anything(), + 'Orchestrations', + 'orch-ENG-1', + 'new-panel', + ); + }); + + test('does not create a parent task when an existing graph currently returns no children', async () => { + loadOrchestrationMock.mockReset(); + loadOrchestrationMock.mockResolvedValueOnce(snapshot); + jiraGraphSourceMock.mockReturnValueOnce(jest.fn().mockResolvedValue({ kind: 'no_children' })); await handler(event()); - expect(jiraGraphSourceMock).not.toHaveBeenCalled(); expect(discoverOrchestrationMock).not.toHaveBeenCalled(); expect(createTaskCoreMock).not.toHaveBeenCalled(); }); diff --git a/cdk/test/handlers/shared/orchestration-store.test.ts b/cdk/test/handlers/shared/orchestration-store.test.ts index 71232521..ef469a82 100644 --- a/cdk/test/handlers/shared/orchestration-store.test.ts +++ b/cdk/test/handlers/shared/orchestration-store.test.ts @@ -1015,6 +1015,36 @@ describe('extendOrchestration — add nodes to an already-seeded epic', () => { expect(written.child_status).toBe('ready'); }); + test('persists adapter metadata on a newly-added node', async () => { + const ddb = makeDdb(); + ddb.send + .mockResolvedValueOnce(existing([{ id: 'A', status: 'succeeded' }])) + .mockResolvedValueOnce({}) + .mockResolvedValueOnce({}); + + await extendOrchestration({ + ddb: ddb as never, + ...extendParams([ + child('A'), + child('B', ['A'], { + channel_metadata: { + jira_cloud_id: 'cloud-1', + jira_issue_key: 'ENG-2', + }, + }), + ]), + }); + + const bw = ddb.send.mock.calls.find((call) => call[0] instanceof BatchWriteCommand)![0]; + const written = (bw.input.RequestItems[TABLE] as Array<{ + PutRequest: { Item: Record }; + }>)[0].PutRequest.Item; + expect(written.channel_metadata).toEqual({ + jira_cloud_id: 'cloud-1', + jira_issue_key: 'ENG-2', + }); + }); + test('adds a NEW node whose predecessor is NOT yet done → blocked, not releasable', async () => { const ddb = makeDdb(); ddb.send diff --git a/docs/guides/JIRA_SETUP_GUIDE.md b/docs/guides/JIRA_SETUP_GUIDE.md index 0e3f6d32..e01a7128 100644 --- a/docs/guides/JIRA_SETUP_GUIDE.md +++ b/docs/guides/JIRA_SETUP_GUIDE.md @@ -323,7 +323,9 @@ All executable subtasks must belong to active Jira project mappings that resolve The parent receives orchestration progress and the terminal rollup. Parallel leaves converge through an internal integration task so the orchestration produces one combined pull request; that internal task does not address a nonexistent Jira issue. An `@bgagent` comment on a real child updates that child's pull request and restacks dependent pull requests through the shared orchestration reconciler. -In this version the authored graph is frozen at the first successful seed. Re-applying the trigger label to an already-seeded parent is an idempotent no-op; it does not rerun completed children or add newly created subtasks. +To extend an existing orchestration, add Jira subtasks and re-apply the trigger label. ABCA appends only genuinely new issue keys: existing tasks, branches, statuses, and dependencies are preserved. A new child starts immediately when all of its declared predecessors have already succeeded; otherwise it remains blocked for the reconciler. A new child with no explicit blocker stacks on the existing epic tip rather than bare `main`. + +Re-applying the label without adding a child is an idempotent no-op. Changes only to blocker links between existing children are also ignored; dependency edits do not rewrite work that may already be running or complete. Extending a terminal orchestration reopens the parent progress panel and settles it again when the added work finishes. ## Issue context: attachments and comments diff --git a/docs/src/content/docs/using/Jira-setup-guide.md b/docs/src/content/docs/using/Jira-setup-guide.md index ff91e724..bf322c82 100644 --- a/docs/src/content/docs/using/Jira-setup-guide.md +++ b/docs/src/content/docs/using/Jira-setup-guide.md @@ -327,7 +327,9 @@ All executable subtasks must belong to active Jira project mappings that resolve The parent receives orchestration progress and the terminal rollup. Parallel leaves converge through an internal integration task so the orchestration produces one combined pull request; that internal task does not address a nonexistent Jira issue. An `@bgagent` comment on a real child updates that child's pull request and restacks dependent pull requests through the shared orchestration reconciler. -In this version the authored graph is frozen at the first successful seed. Re-applying the trigger label to an already-seeded parent is an idempotent no-op; it does not rerun completed children or add newly created subtasks. +To extend an existing orchestration, add Jira subtasks and re-apply the trigger label. ABCA appends only genuinely new issue keys: existing tasks, branches, statuses, and dependencies are preserved. A new child starts immediately when all of its declared predecessors have already succeeded; otherwise it remains blocked for the reconciler. A new child with no explicit blocker stacks on the existing epic tip rather than bare `main`. + +Re-applying the label without adding a child is an idempotent no-op. Changes only to blocker links between existing children are also ignored; dependency edits do not rewrite work that may already be running or complete. Extending a terminal orchestration reopens the parent progress panel and settles it again when the added work finishes. ## Issue context: attachments and comments