From 86a1581d417c4a80a074fe055a486066f4d95e2f Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:06:50 +0000 Subject: [PATCH] fix(jira): queue busy turns async; resolve thread root for inline replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop short-circuiting busy threads with "already working" — always dispatch thread.turn.start so orchestration queues the message, and keep bridgeTurn fork-detached (async reply when the turn finishes). Make parentId threading reliable: Jira only accepts children under root comments (nested parentId → 400, then we used to fall flat). GET the trigger comment and use its parentId when set before posting. Accept comment.parentId on webhooks; ignore empty Automation parent.id. Live evidence SA-421: reply 71415 was flat because parent was nested 71413 (true root 71399); parentId=71399 succeeds. Co-authored-by: Patrick Roza <42661+patroza@users.noreply.github.com> --- apps/server/src/jira/JiraAppClient.ts | 136 +++++++++++++++++++-- apps/server/src/jira/JiraIssueBridge.ts | 44 ++----- apps/server/src/jira/JiraWebhook.test.ts | 39 ++++++ apps/server/src/jira/JiraWebhookPayload.ts | 24 +++- docs/user/jira-issue-conversations.md | 32 ++++- 5 files changed, 223 insertions(+), 52 deletions(-) diff --git a/apps/server/src/jira/JiraAppClient.ts b/apps/server/src/jira/JiraAppClient.ts index d065504d86c2..0afa3682767c 100644 --- a/apps/server/src/jira/JiraAppClient.ts +++ b/apps/server/src/jira/JiraAppClient.ts @@ -15,8 +15,8 @@ export class JiraAppClient extends Context.Service< readonly body: string; /** * When set, create a threaded **reply** under this comment (Jira `parentId`). - * Only top-level comments accept children; nest under the thread root when the - * user wrote inside an existing reply thread. + * Only top-level comments accept children; the client resolves nested ids to the + * thread root via GET before posting. */ readonly parentCommentId?: string | null; /** @@ -27,7 +27,7 @@ export class JiraAppClient extends Context.Service< /** @-mention this Jira user at the start of the reply (normal reply style). */ readonly mentionAccountId?: string | null; readonly mentionDisplayName?: string | null; - }) => Effect.Effect<{ readonly id: string } | null, never>; + }) => Effect.Effect<{ readonly id: string; readonly parentId: string | null } | null, never>; /** * Best-effort reaction on a comment (👀). Jira Cloud support varies; returns the emoji id * when the site accepted the reaction, otherwise null. @@ -47,6 +47,12 @@ export class JiraAppClient extends Context.Service< const CommentResponse = Schema.Struct({ id: Schema.Union([Schema.String, Schema.Number]), + parentId: Schema.optional(Schema.Union([Schema.String, Schema.Number, Schema.Null])), +}); + +const CommentGetResponse = Schema.Struct({ + id: Schema.Union([Schema.String, Schema.Number]), + parentId: Schema.optional(Schema.Union([Schema.String, Schema.Number, Schema.Null])), }); export const make = Effect.gen(function* () { @@ -62,6 +68,80 @@ export const make = Effect.gen(function* () { return request.pipe(HttpClientRequest.setHeader("authorization", `Basic ${token}`)); }; + /** + * Jira only allows children under **root** comments. Nesting under a reply returns 400 + * ("Parent comment not found, and no child comments exist"). Resolve any comment id to + * the thread root via GET `/rest/api/3/issue/{key}/comment/{id}` (`parentId` or self). + */ + const resolveThreadRootCommentId = ( + issueKey: string, + commentId: string, + ): Effect.Effect => + Effect.gen(function* () { + const trimmed = commentId.trim(); + if (trimmed.length === 0) return null; + if (!config.enabled) return trimmed; + + const url = `${config.baseUrl}/rest/api/3/issue/${encodeURIComponent(issueKey)}/comment/${encodeURIComponent(trimmed)}`; + const request = authorize( + HttpClientRequest.get(url).pipe( + HttpClientRequest.acceptJson, + HttpClientRequest.setHeader("user-agent", "t3-code-jira-bridge"), + ), + ); + const response = yield* httpClient.execute(request).pipe( + Effect.tapError((cause) => + Effect.logWarning("Jira comment GET for thread root failed", { + issueKey, + commentId: trimmed, + cause, + }), + ), + Effect.orElseSucceed(() => null), + ); + if (response === null) return trimmed; + + return yield* HttpClientResponse.matchStatus(response, { + "2xx": (success) => + HttpClientResponse.schemaBodyJson(CommentGetResponse)(success).pipe( + Effect.map((parsed) => { + const parentRaw = parsed.parentId; + if (parentRaw === undefined || parentRaw === null) return String(parsed.id); + const parent = String(parentRaw).trim(); + return parent.length > 0 ? parent : String(parsed.id); + }), + Effect.tap((rootId) => + rootId !== trimmed + ? Effect.logInfo("Resolved Jira nested comment to thread root", { + issueKey, + commentId: trimmed, + threadRootId: rootId, + }) + : Effect.void, + ), + Effect.tapError((cause) => + Effect.logWarning("Jira comment GET decode failed; using id as root candidate", { + issueKey, + commentId: trimmed, + cause, + }), + ), + Effect.orElseSucceed(() => trimmed), + ), + orElse: (failed) => + Effect.gen(function* () { + const detail = yield* failed.text.pipe(Effect.orElseSucceed(() => "")); + yield* Effect.logWarning("Jira comment GET rejected; using id as root candidate", { + issueKey, + commentId: trimmed, + status: failed.status, + detail: detail.slice(0, 300), + }); + return trimmed; + }), + }); + }); + const addIssueComment = Effect.fn("JiraAppClient.addIssueComment")(function* (input: { readonly issueKey: string; readonly body: string; @@ -111,7 +191,12 @@ export const make = Effect.gen(function* () { parentForLog: string | null, ) => HttpClientResponse.schemaBodyJson(CommentResponse)(success).pipe( - Effect.map((parsed) => ({ id: String(parsed.id) })), + Effect.map((parsed) => { + const parentRaw = parsed.parentId; + const parentId = + parentRaw === undefined || parentRaw === null ? null : String(parentRaw).trim() || null; + return { id: String(parsed.id), parentId }; + }), Effect.tapError((cause) => Effect.logWarning("Jira comment create response decode failed", { issueKey: input.issueKey, @@ -123,14 +208,14 @@ export const make = Effect.gen(function* () { ); type Attempt = - | { readonly _tag: "ok"; readonly id: string } + | { readonly _tag: "ok"; readonly id: string; readonly parentId: string | null } | { readonly _tag: "retry_next" } | { readonly _tag: "failed" }; const tryPost = (parentCommentId: string | null): Effect.Effect => Effect.gen(function* () { const payload: Record = { body: adfBody }; - // Undocumented but supported on Jira Cloud: parentId threads under a root comment. + // Jira Cloud: parentId threads under a **root** comment only. if (parentCommentId !== null) { payload.parentId = parentIdValue(parentCommentId); } @@ -143,7 +228,7 @@ export const make = Effect.gen(function* () { Effect.map((parsed) => parsed === null ? ({ _tag: "failed" } as const) - : ({ _tag: "ok", id: parsed.id } as const), + : ({ _tag: "ok", id: parsed.id, parentId: parsed.parentId } as const), ), ), orElse: (failed) => @@ -172,15 +257,40 @@ export const make = Effect.gen(function* () { const primary = input.parentCommentId?.trim() || null; const secondary = input.fallbackParentCommentId?.trim() || null; - // Prefer inline reply: primary parent → optional secondary → top-level last resort only. - const parents: Array = []; - if (primary !== null) parents.push(primary); - if (secondary !== null && secondary !== primary) parents.push(secondary); - parents.push(null); + + // Resolve nested mention/reply ids to thread roots before posting. Live probe on SA-421: + // parentId=child → 400; parentId=root → 200 with parentId set. + const resolvedRoots: string[] = []; + for (const candidate of [primary, secondary]) { + if (candidate === null) continue; + const root = yield* resolveThreadRootCommentId(input.issueKey, candidate); + if (root !== null && !resolvedRoots.includes(root)) { + resolvedRoots.push(root); + } + } + + // Prefer inline reply under resolved roots; top-level only as last resort. + const parents: Array = [...resolvedRoots, null]; for (const parent of parents) { const result = yield* tryPost(parent); - if (result._tag === "ok") return { id: result.id }; + if (result._tag === "ok") { + if (parent !== null && result.parentId === null) { + // API accepted body but ignored parentId (e.g. wrong shape). Loud so we notice. + yield* Effect.logError("Jira comment created without parentId despite request", { + issueKey: input.issueKey, + requestedParentId: parent, + createdCommentId: result.id, + }); + } else if (parent !== null) { + yield* Effect.logInfo("Posted Jira inline threaded reply", { + issueKey: input.issueKey, + parentId: result.parentId ?? parent, + createdCommentId: result.id, + }); + } + return { id: result.id, parentId: result.parentId }; + } if (result._tag === "failed") return null; // retry_next → continue } diff --git a/apps/server/src/jira/JiraIssueBridge.ts b/apps/server/src/jira/JiraIssueBridge.ts index ffae4c3363bb..24df2df4e865 100644 --- a/apps/server/src/jira/JiraIssueBridge.ts +++ b/apps/server/src/jira/JiraIssueBridge.ts @@ -56,8 +56,6 @@ const CREATE_DISABLED_RESPONSE = "not yet linked. Auto-create is disabled; link this issue from Chat or enable auto-create."; const AMBIGUOUS_RESPONSE = "Multiple chat threads are linked to this Jira issue, so the bot could not pick which one to use."; -const BUSY_RESPONSE = - "This chat thread is already working. Try again after the current turn finishes."; const FAILED_RESPONSE = "Could not complete this request. Check the linked chat thread for details."; const EMPTY_PROMPT_RESPONSE = @@ -95,16 +93,6 @@ export function formatJiraComment(body: string): string { return `${trimmed.slice(0, MAX_JIRA_COMMENT_LENGTH - 20)}\n\n…(truncated)`; } -function isThreadBusy(thread: OrchestrationThread): boolean { - const latest = thread.latestTurn; - if (latest?.state === "running") return true; - const session = thread.session; - if (session === null) return false; - return ( - session.activeTurnId !== null && (session.status === "running" || session.status === "starting") - ); -} - export class JiraIssueBridge extends Context.Service< JiraIssueBridge, { @@ -141,30 +129,26 @@ const make = Effect.gen(function* () { /** * Post a bridge response as an **inline threaded reply** under the user's mention. * - * Parent order: - * 1. `replyToCommentId` — thread root when the mention is a child reply (Jira only - * allows nesting under roots), or the mention itself when top-level - * 2. `sourceCommentId` — the triggering mention (always try to answer the user inline) + * Parent candidates (JiraAppClient resolves each to the **thread root** via GET — + * Jira rejects nesting under a child comment with 400): + * 1. `sourceCommentId` — the triggering mention (reply next to the user) + * 2. `replyToCommentId` — webhook parent / root when present and different * * Never intentionally posts a bare top-level comment first; flat fallback is only - * if Jira rejects every parentId (see JiraAppClient). + * if every resolved parentId is rejected (see JiraAppClient). */ const postComment = (delivery: StoredJiraDelivery, body: string) => { - const rootParent = delivery.replyToCommentId.trim(); const mentionParent = delivery.sourceCommentId.trim(); - // Prefer the resolved reply parent, then the mention comment itself. - const parentCommentId = - rootParent.length > 0 ? rootParent : mentionParent.length > 0 ? mentionParent : null; + const replyParent = delivery.replyToCommentId.trim(); + // Prefer the mention itself so we always answer that comment's thread; client + // walks parentId up to the root when the mention is already a nested reply. + const parentCommentId = mentionParent.length > 0 ? mentionParent : null; return jira.addIssueComment({ issueKey: delivery.issueKey, body: formatJiraComment(body), parentCommentId, - // If parent was a nested reply id that Jira rejects, client retries with the - // mention id when it differs (still inline to the user), then top-level last. fallbackParentCommentId: - rootParent.length > 0 && mentionParent.length > 0 && rootParent !== mentionParent - ? mentionParent - : null, + replyParent.length > 0 && replyParent !== mentionParent ? replyParent : null, // Normal Jira reply style: @ the human who triggered the bot. mentionAccountId: delivery.actorAccountId, mentionDisplayName: delivery.actorDisplayName, @@ -649,11 +633,9 @@ const make = Effect.gen(function* () { }) .pipe(Effect.ignore); - if (isThreadBusy(thread)) { - yield* finishDelivery({ ...acknowledged, threadId: thread.id }, BUSY_RESPONSE, "completed"); - return; - } - + // Always dispatch. When the thread is mid-turn, orchestration queues the + // message (thread.message-queued) — do not short-circuit with a busy reply. + // Response posting stays async via forkDetach(bridgeTurn) below. const commandId = CommandId.make(yield* crypto.randomUUIDv4); const messageId = MessageId.make(yield* crypto.randomUUIDv4); const processing: StoredJiraDelivery = { diff --git a/apps/server/src/jira/JiraWebhook.test.ts b/apps/server/src/jira/JiraWebhook.test.ts index 3cc10af69dfc..78a82364705e 100644 --- a/apps/server/src/jira/JiraWebhook.test.ts +++ b/apps/server/src/jira/JiraWebhook.test.ts @@ -222,6 +222,45 @@ describe("parseJiraCommentInvocation", () => { }); }); + it("accepts REST-style comment.parentId for threaded replies", () => { + const invocation = parseJiraCommentInvocation( + webhook("@omegent continue", { + comment: { + id: "10800", + body: "@omegent continue", + parentId: 71399, + author: { accountId: "user-1", displayName: "Ada", accountType: "atlassian" }, + }, + }), + "omegent", + ); + expect(invocation).toMatchObject({ + commentId: "10800", + replyToCommentId: "71399", + commentSurface: "reply", + }); + }); + + it("ignores empty Automation parent.id and treats the mention as top-level", () => { + const invocation = parseJiraCommentInvocation( + webhook("@omegent investigate packing", { + comment: { + id: "71413", + body: "@omegent investigate packing", + parent: { id: "" }, + parentId: null, + author: { accountId: "user-1", displayName: "Ada", accountType: "atlassian" }, + }, + }), + "omegent", + ); + expect(invocation).toMatchObject({ + commentId: "71413", + replyToCommentId: "71413", + commentSurface: "issue", + }); + }); + it("uses the mention itself as replyTo when the comment is top-level", () => { const invocation = parseJiraCommentInvocation( webhook("@omegent investigate packing"), diff --git a/apps/server/src/jira/JiraWebhookPayload.ts b/apps/server/src/jira/JiraWebhookPayload.ts index 44a7ca0617ba..d39501420632 100644 --- a/apps/server/src/jira/JiraWebhookPayload.ts +++ b/apps/server/src/jira/JiraWebhookPayload.ts @@ -29,15 +29,24 @@ export const JiraCommentWebhook = Schema.Struct({ updated: Schema.optional(Schema.String), /** * Present when the comment is a reply in a threaded discussion (when Jira provides it). - * String or number depending on payload shape. + * String or number depending on payload shape. Empty `id` (Automation blank fields) + * is treated as missing by `parentCommentIdFromPayload`. */ parent: Schema.optional( Schema.Union([ - Schema.Struct({ id: Schema.Union([Schema.String, Schema.Number]) }), + Schema.Struct({ + id: Schema.optional(Schema.Union([Schema.String, Schema.Number, Schema.Null])), + }), Schema.String, Schema.Number, + Schema.Null, ]), ), + /** + * REST list/get shape uses top-level `parentId` (number|string|null). Accept it on + * webhooks/Automation so nested mentions resolve without an extra GET when present. + */ + parentId: Schema.optional(Schema.Union([Schema.String, Schema.Number, Schema.Null])), jsdPublic: Schema.optional(Schema.Boolean), }), issue: Schema.Struct({ @@ -310,12 +319,17 @@ export function projectKeyFromIssueKey(issueKey: string): string { return match?.[1] ?? issueKey.split("-")[0]?.toUpperCase() ?? ""; } -function parentCommentId(parent: JiraCommentWebhook["comment"]["parent"]): string | null { +function parentCommentIdFromPayload(comment: JiraCommentWebhook["comment"]): string | null { + // Prefer REST-style parentId (what GET /comment returns and Automation can mirror). + const fromParentId = asStringId(comment.parentId ?? null); + if (fromParentId !== null) return fromParentId; + + const parent = comment.parent; if (parent === undefined || parent === null) return null; if (typeof parent === "string" || typeof parent === "number") { return asStringId(parent); } - return asStringId(parent.id); + return asStringId(parent.id ?? null); } function isBotAuthor(author: JiraWebhookUser | undefined): boolean { @@ -356,7 +370,7 @@ export function parseJiraCommentInvocation( const commentId = asStringId(payload.comment.id); if (commentId === null) return null; - const parentId = parentCommentId(payload.comment.parent); + const parentId = parentCommentIdFromPayload(payload.comment); const replyToCommentId = parentId ?? commentId; const commentSurface: JiraCommentSurface = parentId !== null ? "reply" : "issue"; const projectKey = diff --git a/docs/user/jira-issue-conversations.md b/docs/user/jira-issue-conversations.md index 5688d9681caa..35cd68e887bb 100644 --- a/docs/user/jira-issue-conversations.md +++ b/docs/user/jira-issue-conversations.md @@ -181,9 +181,35 @@ are logged and never block the turn. Responses are posted as issue comments authored by the service account, preferably as a **threaded reply** under the triggering mention (REST body field `parentId` — supported on Jira -Cloud even though it is lightly documented). When the user mentioned the bot inside an existing -reply thread, the bridge parents under that thread’s **root** (Jira rejects nesting under a child). -If threading is rejected (invalid parent), the bridge falls back once to a top-level comment. +Cloud even though it is lightly documented). The body **@-mentions** the human requester via an ADF `mention` node: + +```json +{ + "type": "mention", + "attrs": { + "id": "", + "text": "@Display Name", + "accessLevel": "" + } +} +``` + +`attrs.id` is the **bare** account id from the webhook author (e.g. `6331c323…` or +`712020:uuid`) — the same shape returned by GET comment bodies on this site — not the wiki +`[~accountid:…]` form and not plain `@Name` text alone. + +**Inline threading (required for reliable replies):** Jira Cloud only accepts children under a +**root** comment. POST with `parentId` set to a nested reply id returns **400** +(`Parent comment not found, and no child comments exist`) — the bridge used to fall back flat. +Before posting, `JiraAppClient` GETs `/rest/api/3/issue/{key}/comment/{id}` and uses that +comment’s `parentId` when set (else the id itself). Webhooks should send either `comment.parentId` +(REST shape) or `comment.parent.id` when known; empty Automation fields are ignored. Only if every +resolved root is rejected does the bridge fall back to a top-level comment (logged as error when +the create response lacks `parentId`). + +**Busy threads:** follow-up mentions always dispatch `thread.turn.start`. Orchestration queues the +message while a turn is running; the bridge does **not** post an “already working” short-circuit. +The reply still posts asynchronously when the turn completes (`bridgeTurn` is fork-detached). Prefer Markdown converted to a minimal ADF document for API v3. Do not @-spam watchers unless the agent explicitly mentions users.