feat(jira): extend existing orchestrations (#578)

[ayushtr-aws]

Extends an existing Jira-authored orchestration with newly added children through the shared additive extension contract. This is PR 2 of 3 for #580 and is based on #725; #727 builds on this branch.

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 #578

Parent: #580

Stack: #725 -> #726 -> #727

Changes

• Handle the shared extended discovery result when a Jira parent is retriggered.
• Create and release only genuinely new child nodes while preserving every existing orchestration row and completed result.
• Reopen terminal orchestrations for added work, keep blocked additions pending, and treat no-new-node retriggers as no-ops.
• Add webhook and store coverage for release, metadata propagation, terminal reopen, and invalid additions.

Verification

• mise run build on the stack head
• Focused and full CDK suites (178 suites, 3,554 tests on the stack head)
• GITLEAKS_RANGE=origin/main..HEAD mise run security:secrets:range

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]

(Submitted as COMMENT — GitHub blocks self-approval; substantive verdict is Approve.)

Verdict: Approve

Wiring-only, well-tested, and coherent. This PR (2/3 in the #580 stack, based on #725) connects the Jira webhook processor to the shared additive-extension contract that already exists in the base branch. It handles the extended discovery result: releases only genuinely-new releasable children, leaves blocked additions for the reconciler, reopens a terminal parent panel, and treats no-new-node / no-current-subtask re-triggers as no-ops. The extension primitives (extendOrchestration, epic-tip stacking, the extended discriminant) come from the base/parent work and were correctly excluded from this review.

Vision alignment

Advances the orchestration arc (epic #580) while preserving fire-and-forget: a re-trigger is unattended, bounded (only new keys appended; existing rows/branches/statuses/results preserved), and reviewable (panel reopened, structured logs). Blast radius is unchanged — no new IAM, resources, or network surface. Consistent with the bounded-blast-radius tenet; no tenet trade, so no ADR required.

Blocking issues

None.

Non-blocking suggestions / nits

1. Extend-path double read of the orchestration (jira-webhook-processor.ts ~714). When releasableRows.length === 0 (a blocked-only addition), no release mutates the table, yet the panel-refresh block still issues a second loadOrchestration and falls back to snapshot anyway (fresh ?? snapshot). You could reuse snapshot directly in that case and save a DDB read. Trivial; leaving it keeps the seed/extend paths structurally parallel.
2. Seed vs. extend panel-upsert divergence. The seed path guards if (fresh) before upserting; the extend path uses fresh ?? snapshot and always upserts. Both are defensible (extend intentionally refreshes even from a stale snapshot), but the asymmetry is worth a one-line comment so a future reader doesn't "fix" it into inconsistency.
3. No direct assertion that context enrichment is skipped on extend. The new !existingOrchestration gate at line 500 (skip comments/attachments on a re-trigger) is sound and the comment explains the S3-version-pinning rationale, but no test asserts fetchRecentHumanComments / attachment download are NOT called on the extend path. Consider a .not.toHaveBeenCalled() to lock the intent.

Documentation

docs/guides/JIRA_SETUP_GUIDE.md was updated to replace the "frozen at first seed" wording with the new extend semantics (append-only, immediate release when predecessors succeeded, blocked otherwise, epic-tip stacking, terminal reopen). The Starlight mirror docs/src/content/docs/using/Jira-setup-guide.md is byte-identical for the changed block — mirror sync is satisfied, no stale-mirror CI risk. Backing issue #578 (approved, assigned) matches the delivered behavior.

Tests & CI

Good coverage of the new branches and failure/edge paths: newly-added releasable root + panel reopen; blocked addition left for reconciler with panel refresh + setStatusCommentId; no-new-node extend no-op; existing graph returning no_children no-op; and store-level metadata propagation on a newly-added node. Tests correctly exercise the multi-loadOrchestration call sequence via ordered mocks. Bootstrap synth-coverage: not applicable (no new CFN resource types, constructs, or stacks). Test performance: no new App()/Template.fromStack per-test and no re-enabled bundling-stacks in the changed suites. Note: I could not execute the focused Jest suites — the review worktree has no installed node_modules and npx resolved a mismatched global jest; I relied on static analysis plus the author's stated full-suite pass (178 suites / 3,554 tests on the stack head).

Review agents run

• code-reviewer (applied): style/guidelines clean; routing correct (handler change lands in cdk/).
• silent-failure-hunter (applied): panel-refresh failures are caught and logged warn as non-fatal, consistent with the seed path and the advisory nature of the panel; no swallowed control-flow errors.
• comment-analyzer (applied): the rewritten comments (attachment S3-version pinning, race-as-empty-extend) accurately match the code.
• pr-test-analyzer (applied): coverage assessment above; one minor gap noted (nit 3).
• type-design-analyzer (omitted): no new types introduced here — the extended discriminant and extend result types originate in the base branch.
• security-review (omitted): no IAM, Cedar, network, secrets, or input-gateway changes in this diff; shared types (types.ts) untouched, so CLI type-sync is N/A.

Human heuristics

• Proportionality (pass): ~120 handler lines mirroring the existing seed path; no new abstraction for a one-off.
• Coherence (pass): reuses releaseReadyChildren / applyTerminalCreateFailures / upsertEpicPanel with the same conventions as seed; terminology ("extend", "releasable", "epic tip") is consistent across handler, store, and docs.
• Clarity (pass): names communicate intent; existingOrchestration gating is explicit and commented. Minor divergence noted in nit 2.
• Appropriateness (pass): additive extension is verified against the real store contract (extendOrchestration) and the store test asserts persisted channel_metadata, not just a self-written mock echo.

[ayushtr-aws]

Addressed the review nits in 5348759:

• blocked-only graph extensions reuse the durable orchestration snapshot and avoid the redundant post-release read;
• released extensions still refresh after mutation, with a comment documenting the intentional seed/extension asymmetry;
• the extension regression test now includes an attachment and proves both comment fetching and attachment processing are skipped.

Focused extension suite: 13/13 passing.

[scottschreckengaust]

Verdict: Approve

Wiring-only PR (2/3 in the #580 stack, based on #725) that connects the Jira webhook processor to the shared additive-extension contract living in the base branch. It handles the extended discovery result, releases only genuinely-new releasable children, leaves blocked additions for the reconciler, reopens a terminal parent panel, and treats no-new-node / no-current-subtask re-triggers as no-ops. The extension primitives (extendOrchestration, epic-tip stacking, the extended discriminant) originate in the base/parent work and were correctly excluded from this review. Well-tested, coherent, docs + Starlight mirror in sync. One non-blocking coherence gap noted below.

Vision alignment

Advances the orchestration arc (epic #580) while preserving fire-and-forget: a re-trigger is unattended, bounded (only new keys appended; existing rows/branches/statuses/results preserved), and reviewable (panel reopened, structured logs). No new IAM, resources, or network surface — blast radius unchanged. Consistent with the bounded-blast-radius tenet; no tenet trade, so no ADR required.

Blocking issues

None.

Non-blocking suggestions / nits

1. Extend release path omits the per-user concurrency budget that seed and the Linear extend path enforce (cdk/src/handlers/jira-webhook-processor.ts:691). The seed path immediately above (:599-617) reads readConcurrencyBudget(...) and threads it as the maxToRelease arg to releaseReadyChildren; the Linear extend path does the same (linear-webhook-processor.ts:1191-1205). The Jira extend path calls releaseReadyChildren(..., snapshot.children) with no defaultBranch/maxToRelease, so it releases ALL newly-added ready children unthrottled. Functionally this degrades gracefully — admission control is the documented hard ceiling and over-released children that lose the race are left ready for the reconciler (see the readConcurrencyBudget doc-comment) — so it is not a correctness/fail-open bug. But a user who adds many unblocked subtasks in one re-trigger can mass-release into admission failures, which is exactly the churn the budget throttle exists to avoid. For coherence with seed + Linear, thread the budget here too. Note the self-review's "Coherence (pass): reuses releaseReadyChildren with the same conventions as seed" is slightly overstated on this point — the throttle convention diverges.
2. Extend-path double read when releasableRows.length === 0. On a blocked-only addition no release mutates the table, yet the panel-refresh reuses panelSnapshot = snapshot (correct) — but note the code already avoids the extra read in that branch (panelSnapshot only re-loads inside the releasableRows.length > 0 block). This is fine as written; flagging only to confirm it was checked.
3. Seed vs. extend panel-upsert asymmetry. Seed guards if (fresh) before upserting; extend uses panelSnapshot = fresh ?? snapshot and always upserts (intentional — extension already has a durable snapshot, per the inline comment at :717). Defensible and commented; no change needed.

Documentation

docs/guides/JIRA_SETUP_GUIDE.md replaces the "frozen at first seed" wording with the extend semantics (append-only, immediate release when predecessors succeeded, blocked otherwise, epic-tip stacking, terminal reopen, no-op on no-new-child, ignored edge-only edits). The Starlight mirror docs/src/content/docs/using/Jira-setup-guide.md is byte-identical for the changed block — mirror sync satisfied, no stale-mirror CI risk. Backing issue #578 (approved, assigned to author) matches the delivered behavior.

Tests & CI

Strong coverage of the new branches and edge/failure paths: newly-added releasable root + panel reopen with statusCommentId/stateOverrides; blocked addition left for reconciler with panel refresh + setStatusCommentId; no-new-node extend no-op; existing graph returning no_children no-op; and store-level channel_metadata propagation on a newly-added node. The multi-loadOrchestration call sequence is exercised via ordered mocks. The new tests also lock the !existingOrchestration context-skip intent (fetchRecentHumanComments / attachment download .not.toHaveBeenCalled()), closing the gap the author self-noted. Bootstrap synth-coverage: not applicable — no new CFN resource types, constructs, or stacks. Test performance: no new App() / Template.fromStack per-test and no re-enabled aws:cdk:bundling-stacks in the changed suites (#366 clean). I did not execute the Jest suites in the review worktree (no installed node_modules); relied on static analysis plus the author's stated full-suite pass (178 suites / 3,554 tests on the stack head).

Review agents run

• code-reviewer (applied): style/guidelines clean; routing correct — handler + tests land in cdk/ per AGENTS.md.
• silent-failure-hunter (applied): panel-refresh failures caught and logged warn as non-fatal, consistent with the seed path and the advisory nature of the panel; no swallowed control-flow errors. Graph error/attachment JiraAttachmentError still fail-closed via safeReportIssueFailure.
• comment-analyzer (applied): rewritten comments (attachment S3-version pinning at :427-430, race-as-empty-extend at :585-587, extend-refresh-from-snapshot at :717) accurately match the code.
• pr-test-analyzer (applied): coverage assessment above; the one prior gap (context-skip assertion) is now covered. No unthrottled-large-extend test exists, matching nit 1.
• type-design-analyzer (omitted): no new types introduced here — the extended discriminant and result types originate in the base branch.
• security-review (omitted): no IAM, Cedar, network, secrets, or input-gateway changes in this diff; cdk/src/handlers/shared/types.ts untouched, so cli/src/types.ts sync is N/A; no Cedar pin moved.

Human heuristics

• Proportionality (pass): ~120 handler lines mirroring the seed path; no new abstraction for a one-off.
• Coherence (concern, non-blocking): reuses releaseReadyChildren / applyTerminalCreateFailures / upsertEpicPanel with seed conventions, but the concurrency-budget throttle diverges from both seed and the Linear extend path (nit 1, jira-webhook-processor.ts:691). Terminology ("extend", "releasable", "epic tip") is consistent across handler, store, and docs.
• Clarity (pass): names communicate intent; existingOrchestration gating is explicit and commented with the S3-version-pinning rationale.
• Appropriateness (pass): additive extension verified against the real store contract (extendOrchestration), and the store test asserts persisted channel_metadata rather than a self-written mock echo.

The base branch was changed.