feat(jira): support @bgagent comment-triggered PR iteration

[ayushtr-aws]

Adds Jira reviewer follow-up comments that iterate on an existing ABCA pull request, matching the standalone Linear @bgagent <instruction> flow.

Area

• cdk - infrastructure, handlers, constructs
• agent - Python runtime / Docker image
• cli - bgagent client
• docs - guides or design sources (docs/guides/, docs/design/)
• tooling - root mise.toml, scripts, CI workflows

Related

Closes #576

Parent tracker: #580. Orchestrated child restacking remains scoped to #574, per the parent implementation note.

Changes

• Accept and deduplicate Jira comment_created webhooks using stable comment IDs.
• Parse token-bounded @bgagent mentions from plain text and ADF, while ignoring app and ABCA-authored comments.
• Add a sparse tenant-and-issue GSI that resolves the newest prior PR-producing Jira task.
• Admit coding/pr-iteration-v1 tasks against the existing PR with linked-reviewer attribution and original-owner fallback.
• Post immediate acknowledgement or actionable no-PR/admission feedback, while retaining the existing terminal Jira fanout.
• Document the required Jira webhook event and reviewer workflow, with focused handler, parser, resolver, construct, and persistence tests.

Validation

• mise run build passed, including 129 CDK suites / 2,438 tests, agent quality, CLI tests, docs build, synth, lint, and contract-sync checks.
• MISE_EXPERIMENTAL=1 mise //cdk:eslint and mise //cli:eslint passed after syncing with current origin/main.
• Branch-range gitleaks, Semgrep SAST, Grype, Retire.js, and Bandit passed.
• End-to-end on backgroundagent-dev2: Jira SCRUM-27 opened ayushtr-aws/abca-testing#16; a later @bgagent comment was acknowledged and completed through coding/pr-iteration-v1, adding a second commit to the same PR with no duplicate PR.
• Pre-commit hooks and pre-push package tests passed. The aggregate security hook also reports current repository/environment baselines unrelated to this diff: historical leaks on non-main refs, three lockfile advisories, existing custom masking findings, one existing workflow warning, and Docker unavailable on this Finch-only host.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[ayushtr-aws]

Self-review

Verdict: Approve. Faithful implementation of #576 that mirrors the Linear @bgagent flow. CDK wiring, IAM grants, idempotency, and self-trigger loop guards all check out. No blocking bugs.

Vision alignment

Extends fire-and-forget with policy-gated escalation — reviewers iterate on a PR from Jira instead of dropping into live pair-programming. Blast radius stays bounded: the comment path reuses createTaskCore (admission, cost limits, guardrails all apply), pins coding/pr-iteration-v1, and targets the existing PR. No tenet traded, so no ADR needed.

Two items worth an explicit sign-off (both intentional)

1. Authorization asymmetry vs. the label path. When the commenter isn't a linked ABCA user, handleCommentTrigger falls back to priorTask.user_id, whereas the label path rejects unlinked users. Net effect: any human in the tenant who can comment can drive a coding run on the owner's repo, attributed to and consuming the owner's quota. Documented as deliberate ("so a useful reviewer request is not dropped") and tested — flagging only so it's a conscious choice.
2. Transient OAuth-resolution failure is dropped silently. resolveJiraOauthToken → null covers both permanent (not onboarded, invalid_grant) and transient (DDB/Secrets throttle, refresh blip) conditions; the handler returns without feedback or a throw. Since the idempotency key is already computed, throwing on transient errors would be replay-safe and buy the two async Lambda retries. Mirrors the pre-existing label-path behavior, so it's a robustness improvement rather than a regression.

Documentation

Guide + Starlight mirror regenerated and in sync; CLI setup prompt now lists Comment: created. #576 is approved.

Tests & CI

Strong coverage — pure helpers thoroughly unit-tested with negatives and error propagation, GSI projection asserted exactly, processor happy path + most failure branches covered, pagination subtlety pinned. Gaps worth a follow-up: the OAuth-unavailable branch and the "PR but no repo" branch are untested, and the idempotency key is asserted by shape (/^jira-iterate-[a-f0-9]{64}$/) rather than by its determinism guarantee.

Bootstrap synth-coverage: N/A — the only infra change is addGlobalSecondaryIndex on the existing AWS::DynamoDB::Table (no new CFN resource type), so ADR-002 updates aren't required. The one red check (osv-scanner) is pre-existing baseline noise: 3 High advisories in fast-uri / fast-xml-parser / svgo, all in yarn.lock, unrelated to this diff.

Review agents run

code-reviewer, silent-failure-hunter, pr-test-analyzer, type-design-analyzer, comment-analyzer all ran. /security-review omitted: no IAM policy change (existing grantReadWriteData covers the new GSI), no Cedar/network/secrets change; untrusted webhook JSON is handled via parse-don't-trust guards.

[scottschreckengaust]

Verdict: Approve

Faithful, well-bounded implementation of #576 (approved, correct feat/576-* branch). It mirrors the Linear @bgagent flow, reuses createTaskCore (so admission, cost limits, and guardrails all still apply), pins coding/pr-iteration-v1, and targets the existing PR. Idempotency, self-trigger loop guards, the multi-tenant cloudId trust boundary, and the sparse GSI are all handled carefully and thoroughly tested. No blocking issues. Two intentional-design items and a few nits below.

Vision alignment

Advances "fire-and-forget default; escalate by policy." Reviewers iterate on a PR from Jira instead of dropping into live pair-programming, and blast radius stays bounded: the comment path routes through createTaskCore (admission/cost/guardrail gates intact), pins the coding/pr-iteration workflow, and never widens scope beyond the resolved PR. No tenet traded away, so no ADR required. Outcomes remain reviewable (PR commit + Jira ack/terminal comments).

Blocking issues

None.

Non-blocking suggestions / nits

1. Authorization asymmetry vs. the label path (intentional; worth a conscious sign-off). jira-webhook-processor.ts:645 — when the commenter isn't a linked ABCA user, handleCommentTrigger falls back to priorTask.user_id, whereas the label path rejects unlinked users. Net effect: any tenant user who can comment on an issue that already produced an ABCA PR can drive a coding run on the owner's repo, attributed to and consuming the owner's quota. Bounded (existing PR only, createTaskCore gates apply, pinned iteration workflow) and documented/tested, so not blocking — flagging so it's a deliberate choice, not an accident.

2. Transient OAuth-resolution failure is dropped silently (pre-existing pattern). jira-webhook-processor.ts:662-670 — resolveJiraOauthToken returns null for both permanent (not onboarded / revoked) and transient (DDB/Secrets throttle) conditions (see the nosemgrep annotations in jira-oauth-resolver.ts:269,327), and the handler then returns with no Jira feedback. Since buildCommentIdempotencyKey is deterministic on commentId, throwing on transient errors would be replay-safe and buy the two async-Lambda retries. This mirrors the label path (processor:425-436), so it's a latent robustness improvement rather than a regression — a getOauthSecretStrict-style strict resolver here would be a clean follow-up.

3. Coverage gaps for two failure branches. The OAuth-unavailable branch (:663) and the "found a task but !priorTask.repo" branch (:633) have no direct processor test; and the comment idempotency key is asserted by shape (jira-iterate-<hex>) rather than by its determinism guarantee (same cloudId/issueKey/commentId → same key). Minor, given the strong overall suite.

Documentation

Good. docs/guides/JIRA_SETUP_GUIDE.md documents the new required Comment: created webhook event and the reviewer workflow, and the Starlight mirror (docs/src/content/docs/using/Jira-setup-guide.md) was regenerated and is in sync (verified diff-equal — no hand-edit, no "Fail build on mutation" risk). CLI setup prompt (cli/src/commands/jira.ts:675) now lists Comment: created so operators subscribe the event. #576 is approved.

Tests & CI

Strong coverage: pure helpers (comment-trigger, jira-task-by-issue) unit-tested with negatives and error propagation; the GSI projection is asserted exactly (task-table.test.ts:107 — INCLUDE + NonKeyAttributes); pagination past a PR-less first page is pinned (jira-task-by-issue.test.ts:77); processor happy path, ADF mention-node parsing, self-trigger/app-author skips, and no-PR fallback are all covered. CI: 7/8 checks green including build (agentcore) (full CDK suite).

Bootstrap synth-coverage: N/A. The only infra change is addGlobalSecondaryIndex (JiraIssueIndex) on the existing AWS::DynamoDB::Table — no new CFN resource type, and the existing grantReadWriteData already covers GSI queries. So ADR-002 bootstrap-policy/version/artifact updates are not required.

Secrets scan (the one red check) — NOT a blocker, NOT a secret. I scoped-scanned in the worktree: gitleaks git . --log-opts=238eb30..HEAD → "no leaks found." The CI job's own log confirms the same (its gitleaks range step reported "no leaks found"); the job actually fails at the //:security:deps (osv-scanner) step with exit code 1 on 3 High advisories in yarn.lock — fast-uri (GHSA-v2hh-gcrm-f6hx), fast-xml-parser (GHSA-8r6m-32jq-jx6q), svgo (GHSA-2p49-hgcm-8545). This PR does not touch yarn.lock; these are pre-existing transitive-dependency advisories already being remediated on a sibling branch (origin/fix/svgo-osv-bump). Merge behind (or rebase on) the dep bump; do not block this feature on it. The only ARN in the diff is a test fixture with fake account 123.

Review agents run

Applied the review-agent lens over the full diff (agent subprocesses are not separately invocable in this harness; findings folded in):

• code-reviewer — routing correct (all handler/construct logic in cdk/, CLI prompt in cli/); L2 DynamoDB GSI; no hardcoded ARNs; channel_source === 'jira' guard on the hoisted jira_issue_identity.
• silent-failure-hunter — see nit #2 (OAuth-null drop, pre-existing) and the deliberate fail-open comment-screening / fail-closed attachment paths, which are correctly annotated. Webhook receiver rolls back the dedup row on invoke failure so retries aren't lost (jira-webhook.ts:257).
• type-design-analyzer — jira_issue_identity? added to TaskRecord (types.ts) is an internal persistence field, not a public API shape → cli/src/types.ts correctly NOT changed (verified TaskRecord is not mirrored to the CLI). The AdfNode.attrs.text addition shadows node-level text but is scoped and documented.
• comment-analyzer — comments accurate; the jira-integration.ts dedup-key doc comment was correctly updated to reflect comment-ID keying.
• pr-test-analyzer — see nit #3.
• /security-review — webhook/input/secrets change in scope: HMAC-verified receiver, per-tenant vs stack-wide cloudId trust boundary (stack-wide deliveries can't steer at a chosen tenant), parse-don't-trust on untrusted webhook JSON, sparse tenant-scoped GSI key. No IAM/Cedar/network change. No new secrets; the OAuth secret ARN is only referenced, never logged.

Human heuristics

• Proportionality — pass. New surface (helper + resolver + sparse GSI) matches the feature; no speculative abstraction. Largest file (jira-webhook-processor.ts) grew ~250 lines for a cohesive second trigger path.
• Coherence — pass. Same terms as the existing Jira/Linear paths (channel_source, channelMetadata, idempotencyKey); the comment path deliberately reuses createTaskCore rather than forking task creation.
• Clarity — concern (minor). Names communicate intent well; the only clarity gap is the OAuth-null path collapsing transient and permanent failures (nit #2), which is documented via nosemgrep.
• Appropriateness — pass. Integration verified against real Jira ADF mention-node shape (attrs.text = '@bgagent', jira-webhook-processor.test.ts:278) and real Atlassian label-diff string format, not just self-written mocks; the author reports an end-to-end run on backgroundagent-dev2.