feat: add spacecat-shared-ticket-client package with Jira Cloud integration (SITES-44690)

[prithipalpatwal]

Jira: SITES-44690

Summary

Adds the spacecat-shared-ticket-client package — a provider-agnostic ticket management client with Jira Cloud as the first implementation.

Package Structure

src/
├── adf/
│   └── markdown-to-adf.js              # Markdown → ADF conversion (marked.lexer)
├── clients/
│   ├── base-ticket-client.js           # Abstract base (createTicket contract)
│   └── jira-cloud-client.js            # Jira REST API v3 provider
├── credentials/
│   ├── credential-manager-factory.js   # Auth strategy dispatch
│   └── oauth-credential-manager.js     # OAuth 2.0 3LO token lifecycle
├── http/
│   └── rate-limit-aware-http-client.js # 429 retry with backoff + quota fail-fast
├── ticket-client-factory.js            # Composition root (wires everything)
├── index.js                            # Barrel exports
└── index.d.ts                          # TypeScript declarations

Key Capabilities

• JiraCloudClient

  • Targets Jira REST API v3
  • Markdown-to-ADF description conversion
  • 255-character summary truncation
  • uploadAttachment() with:
    • MIME type whitelist
    • Magic-byte validation
    • Filename sanitization
  • listProjects() and listIssueTypes() with pagination support

• RateLimitAwareHttpClient

  • Automatic HTTP 429 retry with exponential backoff
  • Retry-After header support
  • Quota-exhaustion fail-fast (no retry on hour-long quota windows)

• OAuthCredentialManager

  • OAuth 2.0 3LO token lifecycle management
  • Concurrent-safe refresh via Secret Manager pre-read
  • Race recovery on Atlassian 401 responses
  • Secret Manager write retry with idempotency tokens

• Security

  • SSRF protection
    • All requests routed through the fixed Atlassian gateway (api.atlassian.com)
    • cloudId validated as a UUID
    • siteUrl validated as https://*.atlassian.net
    • siteUrl used for display only, never as a request target
  • Path traversal protection
    • organizationId and connectionId validated as UUIDs before Secret Manager path interpolation
    • Attachment filenames sanitized

Design

• SOLID-compliant

  • SRP: ADF conversion extracted into its own module
  • OCP: Map-based provider dispatch
  • ISP: Provider-specific methods implemented only on subclasses
  • DIP: All dependencies injected via constructors

• Composition Root

  • TicketClientFactory.create() wires together:
    • Credential manager
    • Rate-limited HTTP client
    • Provider client
  • Consumers use a single client.createTicket() entry point

• Single-level inheritance

  • JiraCloudClient extends BaseTicketClient for shared constructor logic
  • No deep inheritance hierarchy

Alignment

Implements the v1 scope from mysticat-architecture PR #150.

Coverage includes:

• All API endpoints
• Field mappings
• Error handling
• Retry flows

Secret path pattern:

/mysticat/task-management/{orgId}/{connectionId}

Test Plan

• 143 unit tests
• 100% statement, branch, function, and line coverage across all files

Covered Scenarios

• Constructor validation
• createTicket()
  • ADF conversion
  • Summary truncation
  • Field pass-through
• listProjects()
  • Pagination
• listIssueTypes()
  • Hierarchy handling
• uploadAttachment()
  • MIME validation
  • Magic-byte validation
  • Size validation
  • Path traversal protection
• OAuthCredentialManager
  • Token lifecycle
  • Concurrent refresh
  • Secret Manager write retry/exhaustion
  • Race recovery
• RateLimitAwareHttpClient
  • HTTP 429 retry
  • Retry-After handling
  • Exponential backoff
  • Quota fail-fast (case-insensitive)
• Auth retry (#withAuthRetry)
  • Concurrent refresh detection
  • Same-token rejection
  • REQUIRES_REAUTH propagation

[github-actions]

This PR will trigger a minor release when merged.

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Request changes - solid scaffolding with a few security and correctness gaps to close before merge.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (23 files).

Must fix before merge

1. [Critical] Secret path injection - buildSecretPath interpolates organizationId/connectionId without validation; a malformed DB record with ../ can traverse the Secrets Manager namespace - src/ticket-client-factory.js:44 (details inline)
2. [Important] Unused production dependencies - @adobe/fetch and @adobe/spacecat-shared-utils are declared in dependencies but never imported anywhere in src/; ships dead weight to consumers - package.json:41 (details inline)
3. [Important] Uncapped Retry-After - #resolveWaitMs trusts the server-sent Retry-After header without an upper bound; a value of 86400 blocks the Lambda for a full day - src/http/rate-limit-aware-http-client.js:62 (details inline)
4. [Important] ClientRequestToken misuse - access_token.slice(0, 32) may collide across refreshes or contain invalid characters for SM's idempotency token; use a proper UUID or deterministic hash instead - src/credentials/oauth-credential-manager.js:90 (details inline)

Non-blocking (5): minor issues and suggestions

• nit: content?.length works for Buffer/Uint8Array but will silently under-count multi-byte strings if the type contract ever loosens - consider Buffer.byteLength or a type guard - src/clients/jira-cloud-client.js:203
• nit: c8, mocha, mocha-multi-reporters are used in scripts/config but not in devDependencies - works via workspace hoisting but explicit is better - package.json:44
• suggestion: add jitter to exponential backoff (waitMs * (0.5 + Math.random() * 0.5)) to avoid thundering herd on shared rate limits - src/http/rate-limit-aware-http-client.js:60
• nit: URL_REGEX greedily captures trailing punctuation (e.g. ) or . at end of sentence) - consider a negative lookbehind or trimming trailing non-URL chars - src/clients/jira-cloud-client.js:32
• nit: sanitizeFilename strips ../ sequences but not standalone .. (no trailing slash) - edge case for directory traversal on Windows-style paths - src/clients/jira-cloud-client.js:124

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 10m 30s | Cost: $8.16 | Commit: 2541c41333939018ad13d14c8cdba405e63ee6a8
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all four prior blocking findings are correctly fixed with adequate tests.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (23 files).

Previously flagged, now resolved

• Secret path injection: buildSecretPath now validates both IDs as UUIDs before interpolation
• Unused production dependencies: @adobe/fetch and @adobe/spacecat-shared-utils removed; c8, mocha, mocha-multi-reporters added to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token (64-char hex) replaces naive .slice(0, 32)

Non-blocking (4): minor issues and suggestions

• nit: sanitizeFilename .replace(/\.\./g, '') strips ALL .. sequences including legitimate ones in filenames like report..final.pdf - stripping path separators (/ and \) instead would be more targeted - src/clients/jira-cloud-client.js:125
• nit: UUID_REGEX comment says "Matches lowercase UUIDs" but the regex uses the /i flag accepting uppercase - align the comment with the code or remove the flag - src/ticket-client-factory.js:34
• suggestion: validate process.env.NODE_ENV against an allowlist or /^[a-z]+$/ pattern in buildSecretPath for defense-in-depth - the other two path segments are UUID-validated but the env segment is not - src/ticket-client-factory.js:56
• nit: URL_REGEX lookbehind will strip trailing ) from legitimate URLs with balanced parens like Wikipedia links https://en.wikipedia.org/wiki/Foo_(bar) - acceptable tradeoff for the Jira description use case but worth a code comment - src/clients/jira-cloud-client.js:30

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 5s | Cost: $6.08 | Commit: 0303efc11adae7029511b667a03e529fcdd7d5dc
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":6,"duration_api_ms":1560718,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"b18831f0-22a7-4862-a39b-cdbc7351939d","total_cost_usd":10.4506784,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_input_tok

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Request changes - prior blocking findings are fixed; a few remaining correctness issues in filename sanitization, environment validation, and token lifecycle handling.
Complexity: HIGH - large diff, new package scaffolding across 24 files.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Must fix before merge

1. [Important] sanitizeFilename strips all .. sequences and allows path separators through - src/clients/jira-cloud-client.js:127 (details inline)
2. [Important] process.env.NODE_ENV interpolated into SM secret path without validation - src/ticket-client-factory.js:56 (details inline)
3. [Important] Token refresh 401 from Atlassian never triggers setRequiresReauth - src/credentials/oauth-credential-manager.js:84 (details inline)
4. [Important] siteUrl validation uses string suffix matching instead of hostname parsing - src/clients/jira-cloud-client.js:97 (details inline)

Non-blocking (5): minor issues and suggestions

• nit: index.test.js does not verify RateLimitAwareHttpClient export despite it being exported from src/index.js - test/index.test.js:22
• nit: CLOUD_ID_REGEX is lowercase-only (no /i) while UUID_REGEX in the factory has /i - inconsistent casing contract - src/clients/jira-cloud-client.js:19 vs src/ticket-client-factory.js:34
• nit: #buildAdfDescription JSDoc says "URLs are converted to clickable link nodes" but implementation emits plain-text nodes only - src/clients/jira-cloud-client.js:282
• suggestion: add ticket-client to the Package Catalog's API clients row in the root CLAUDE.md so the discovery index stays current
• nit: test/setup-env.js copyright header says 2026, rest of files say 2025 - test/setup-env.js:2

Previously flagged, now resolved

• Secret path injection: buildSecretPath now validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed from dependencies; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all four prior blocking findings are correctly fixed with appropriate test coverage.
Complexity: HIGH - large diff; new package scaffolding with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Previously flagged, now resolved

• sanitizeFilename now strips path separators (/, \) instead of all .. sequences - preserves legitimate filenames while neutralizing traversal
• NODE_ENV removed from SM secret path entirely - account boundary provides env isolation, eliminating the unvalidated interpolation
• Token refresh 401 now triggers setRequiresReauth via try/catch on #fetchNewTokens with .status property on the thrown error
• siteUrl validation upgraded to new URL() hostname parsing - blocks path-based and subdomain-based spoofing attempts

Non-blocking (2): minor issues and suggestions

• nit: UUID_REGEX comment says "Matches lowercase UUIDs" but the regex uses the /i flag accepting uppercase - align the comment or remove the flag for consistency with CLOUD_ID_REGEX - src/ticket-client-factory.js:34
• nit: sanitizeFilename comment "Strip path separators" is on the line above the eslint-disable, making it visually ambiguous which .replace it describes - consider moving it directly above the regex - src/clients/jira-cloud-client.js:120

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 1s | Cost: $3.07 | Commit: 15094660ec79f5d55cbd5652798e3ee31763514f
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Request changes - TypeScript declarations are out of sync with the implementation after the optional-fields commit.
Complexity: HIGH - large diff; new package scaffolding across 24 files.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Must fix before merge

1. [Important] TicketData interface missing priority, dueDate, components, parent fields - TypeScript consumers will get compile errors - src/index.d.ts:22 (details inline)

Non-blocking (6): minor issues and suggestions

• suggestion: validate dueDate format (/^\d{4}-\d{2}-\d{2}$/) before passing to Jira - consistent with how cloudId, ticketKey, and organizationId are validated - src/clients/jira-cloud-client.js:160
• suggestion: add a max-pages safety bound to listProjects pagination loop to prevent unbounded requests if Jira API misbehaves - src/clients/jira-cloud-client.js:112
• nit: .nycrc.json sets branches: 100 but project standard is 97% branches per CLAUDE.md - stricter than required but diverges from other packages - packages/spacecat-shared-ticket-client/.nycrc.json:8
• suggestion: update root CLAUDE.md and AGENTS.md Package Catalog to include ticket-client in the API clients row and increment the package count from 22 to 23 - both catalogs are now stale
• nit: CLOUD_ID_REGEX uses /i (accepts uppercase) while UUID_REGEX in the factory does not - intentional per comments but a one-line note on CLOUD_ID_REGEX explaining why would prevent future fixes - src/clients/jira-cloud-client.js:20
• nit: test/setup-env.js copyright header says 2026 while all other files say 2025 - test/setup-env.js:2

Previously flagged, now resolved

• Secret path injection: buildSecretPath validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)
• sanitizeFilename: now strips path separators instead of all .. sequences
• NODE_ENV: removed from SM secret path entirely
• Token refresh 401: now triggers setRequiresReauth
• siteUrl validation: upgraded to new URL() hostname parsing

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 6s | Cost: $9.66 | Commit: 0eb2ca8c8d6e49ca99458f3aeb10a92a97ad9c09
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[prithipalpatwal]

Addressed review 4572484771 in 73f62962:

• Added priority, dueDate, components, parent to TicketData in src/index.d.ts
• Made description optional (matches implementation)
• Added dueDate YYYY-MM-DD validation + test
• Added listProjects max-pages safety bound (100)
• Documented CLOUD_ID_REGEX /i rationale

79 tests passing, 100% coverage. CI green.

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all prior blocking findings are correctly fixed; only non-blocking nits remain.
Complexity: HIGH - large diff; new package scaffolding across 24 files.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Non-blocking (3): minor issues and suggestions

• suggestion: root CLAUDE.md and AGENTS.md Package Catalog still say "22 packages" and omit ticket-client from the API clients row - both are now factually stale after this PR - AGENTS.md:47
• nit: dueDate validation uses an IIFE-inside-spread pattern that is inconsistent with the other optional fields (priority, components, parent all use plain conditional spread) - hoisting the guard before body construction would be more readable - src/clients/jira-cloud-client.js:214
• suggestion: MAX_PAGES safety bound has no dedicated test proving the loop terminates at 100 when isLast is perpetually false - a small stub test would prevent accidental removal - src/clients/jira-cloud-client.js:252

Previously flagged, now resolved

• TypeScript TicketData interface now includes priority, dueDate, components, parent; description made optional
• dueDate format validated via regex before passing to Jira
• listProjects pagination bounded by MAX_PAGES = 100
• CLOUD_ID_REGEX /i flag rationale documented in comment

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 19s | Cost: $4.23 | Commit: 73f6296206f4286f1d8a3838fa33be2019ef5f66
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all prior blocking findings are correctly fixed; only non-blocking suggestions remain.
Complexity: HIGH - large diff; new package scaffolding across 25 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (25 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (5): minor issues and suggestions

• suggestion: validate href scheme in ADF link nodes (reject javascript:, data:) for defense-in-depth - Jira sanitizes server-side but this hardens against re-rendering in other contexts - src/adf/markdown-to-adf.js:523
• suggestion: tighten siteUrl validation to reject bare atlassian.net (require subdomain: hostname !== 'atlassian.net') - display-only field but tighter matches the *.atlassian.net contract - src/clients/jira-cloud-client.js:97
• suggestion: pass the unwrapped httpClient to CredentialManagerFactory.create() instead of the rate-limited wrapper - auth endpoint has different rate-limit semantics than the Jira API and does not emit RateLimit-Reason headers - src/ticket-client-factory.js:87
• nit: validate projectKey is truthy in createTicket before sending to Jira, consistent with the projectId check in listIssueTypes - src/clients/jira-cloud-client.js:925
• nit: TypeScript declarations do not expose refreshAuthHeaders or forceRefreshAuthHeaders - if consuming services call these directly, they will lack type support - src/index.d.ts:55

Previously flagged, now resolved

• Secret path injection: buildSecretPath validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)
• sanitizeFilename: now strips path separators instead of all .. sequences
• NODE_ENV: removed from SM secret path entirely
• Token refresh 401: now triggers setRequiresReauth with concurrent-race recovery
• siteUrl validation: upgraded to new URL() hostname parsing
• TypeScript TicketData interface: includes priority, dueDate, components, parent; description made optional
• dueDate validation: format validated via regex before passing to Jira
• listProjects pagination: bounded by MAX_PAGES = 100
• AGENTS.md: updated to 23 packages with ticket-client in API clients row

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 14s | Cost: $7.69 | Commit: 6c58d5d92c5867cc9e6d0d3e40cfc102cd525196
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - well-engineered package with solid security posture; only non-blocking suggestions remain after 7+ review rounds.
Complexity: HIGH - large diff; new package scaffolding across 26 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (26 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (7): minor issues and suggestions

• suggestion: TypeScript declarations do not expose refreshAuthHeaders or forceRefreshAuthHeaders on OAuthCredentialManager - consuming services calling these directly will lack type support - src/index.d.ts:57
• suggestion: pass the unwrapped httpClient to CredentialManagerFactory.create() instead of the rate-limited wrapper - the auth endpoint (auth.atlassian.com) has different rate-limit semantics than the Jira API and does not emit RateLimit-Reason headers - src/ticket-client-factory.js:87
• nit: siteUrl validation permits bare atlassian.net (no subdomain) - display-only so no security impact, but tighter matches the *.atlassian.net contract: add || siteUrlParsed.hostname === 'atlassian.net' rejection - src/clients/jira-cloud-client.js:97
• suggestion: setRequiresReauth and #writeReauthFlag perform a bare SM putSecretValue without retry - if the write fails transiently the connection is left in a zombie state unable to refresh but not flagged for reauth - src/credentials/oauth-credential-manager.js:135
• nit: validate parent field against TICKET_KEY_REGEX before sending to Jira, consistent with the defensive validation on ticketKey in uploadAttachment - src/clients/jira-cloud-client.js:220
• nit: listProjects pagination safety bound MAX_PAGES = 100 has no test exercising the break condition - a loop-logic refactor could silently remove the safety bound - src/clients/jira-cloud-client.js:252
• nit: concurrent refresh 200ms wait in #recoverFromAtlassian401 is not tested with fake timers - removing the setTimeout would not break the current test suite - src/credentials/oauth-credential-manager.js:176

Previously flagged, now resolved

• Secret path injection: buildSecretPath validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)
• sanitizeFilename: now strips path separators instead of all .. sequences
• NODE_ENV: removed from SM secret path entirely
• Token refresh 401: now triggers setRequiresReauth with concurrent-race recovery
• siteUrl validation: upgraded to new URL() hostname parsing
• TypeScript TicketData interface: includes priority, dueDate, components, parent; description made optional
• dueDate validation: format validated via regex before passing to Jira
• listProjects pagination: bounded by MAX_PAGES = 100
• AGENTS.md: updated to 23 packages with ticket-client in API clients row

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 24s | Cost: $8.24 | Commit: f6f3daff53742e15a103f61db234593a3f40010b
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all prior non-blocking suggestions are correctly addressed; only minor nits remain.
Complexity: HIGH - large diff; new package scaffolding across 26 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (26 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (5): minor issues and suggestions

• suggestion: CLAUDE.md Package Catalog is still missing ticket-client in the API clients row - AGENTS.md was updated but CLAUDE.md is now inconsistent - CLAUDE.md
• nit: #writeReauthFlag reads the secret once outside the retry loop - a concurrent writer between retries could have its tokens clobbered with the stale snapshot plus requiresReauth: true; re-reading inside the loop would be strictly more correct as a read-modify-write pattern - src/credentials/oauth-credential-manager.js:370
• nit: clock.restore() calls in oauth-credential-manager tests are in the test body rather than afterEach - if an assertion fails before restore, fake timer leaks into subsequent tests - test/oauth-credential-manager.test.js
• nit: setRequiresReauth test uses clock.tickAsync(500) which assumes the total backoff is under 500ms - if constants change this test could silently hang; consider calculating from the constants - test/oauth-credential-manager.test.js
• suggestion: validate href scheme in ADF link nodes (reject javascript:, data:) for defense-in-depth - Jira sanitizes server-side but this hardens against re-rendering in other contexts - src/adf/markdown-to-adf.js:523

Previously flagged, now resolved

• TypeScript declarations now expose refreshAuthHeaders and forceRefreshAuthHeaders on OAuthCredentialManager
• Unwrapped httpClient passed to CredentialManagerFactory - auth.atlassian.com does not emit Jira rate-limit headers
• #writeReauthFlag now has 3-attempt SM write retry with exponential backoff; setRequiresReauth delegates to it (eliminates duplication)
• parent field validated against TICKET_KEY_REGEX in createTicket, consistent with uploadAttachment
• MAX_PAGES = 100 pagination safety bound now has a dedicated test
• #recoverFromAtlassian401 tests use sinon fake timers - no more real 200ms wall-clock delays

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 4m 4s | Cost: $5.68 | Commit: 7dd234120b92241488aa20751e87b18879b8262b
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all prior non-blocking suggestions are correctly addressed; no new issues introduced.
Complexity: HIGH - large diff; new package scaffolding across 26 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (26 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (1): minor issues and suggestions

• nit: XSS link-scheme test covers only javascript: - a second case (e.g. data:text/html,...) would document that the stripping logic is scheme-agnostic rather than specifically targeting one scheme - test/jira-cloud-client.test.js:677

Previously flagged, now resolved

• CLAUDE.md Package Catalog now includes ticket-client in the API clients row
• #writeReauthFlag re-reads secret inside retry loop on every attempt, avoiding stale-snapshot clobbering
• clock.restore() moved to afterEach in both refreshAuthHeaders and setRequiresReauth describe blocks
• tickAsync uses calculated value (300ms) with explanatory comment documenting the backoff math
• ADF link nodes validate href scheme via allowlist, rendering unsafe schemes as plain text

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 56s | Cost: $3.89 | Commit: 7d3975ed288d129271ee52cdbc423b9230985572
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all prior findings addressed; only non-blocking nits remain after 8+ review rounds.
Complexity: HIGH - large diff; new package scaffolding across 26 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (26 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (2): minor issues and suggestions

• nit: XSS test uses a for-loop inside a single it() - a failure on the first scheme short-circuits evaluation of the second; separate it() blocks would give independent pass/fail, though the assertion labels make diagnosis trivial as-is - test/jira-cloud-client.test.js:679
• suggestion: consider adding a mixed-case scheme test (JaVaScRiPt:) to confirm case-insensitive stripping - the implementation's regex already handles this via the i flag on the allowlist check, but a test would document the contract - test/jira-cloud-client.test.js:679

Previously flagged, now resolved

• XSS link-scheme test now covers both javascript: and data: URI schemes (addresses prior nit about single-scheme coverage)
• Secret path injection: buildSecretPath validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)
• sanitizeFilename: now strips path separators instead of all .. sequences
• NODE_ENV: removed from SM secret path entirely
• Token refresh 401: now triggers setRequiresReauth with concurrent-race recovery
• siteUrl validation: upgraded to new URL() hostname parsing
• TypeScript TicketData interface: includes priority, dueDate, components, parent; description made optional
• dueDate validation: format validated via regex before passing to Jira
• listProjects pagination: bounded by MAX_PAGES = 100
• AGENTS.md and CLAUDE.md: updated to 23 packages with ticket-client in API clients row
• ADF link nodes validate href scheme via allowlist, rendering unsafe schemes as plain text
• #writeReauthFlag re-reads secret inside retry loop; clock.restore() moved to afterEach
• parent field validated against TICKET_KEY_REGEX; MAX_PAGES has a dedicated test
• Unwrapped httpClient passed to CredentialManagerFactory (auth endpoint has different rate-limit semantics)

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 59s | Cost: $3.31 | Commit: cc0295c7071159272771db75f67440ca2de8f409
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - merge from main only; no source changes to the ticket-client package since the last approval.
Complexity: HIGH - large diff; new package scaffolding across 26 files with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (26 files).
Note: Recommend a human read before merge - this change modifies the root AGENTS.md (architectural significance). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (1): minor issues and suggestions

• nit: AGENTS.md Package Catalog still says "22 packages" in the Architecture Overview header while the catalog table and CLAUDE.md both say 23 - AGENTS.md:44

Previously flagged, now resolved

• Secret path injection: buildSecretPath validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)
• sanitizeFilename: now strips path separators instead of all .. sequences
• NODE_ENV: removed from SM secret path entirely
• Token refresh 401: now triggers setRequiresReauth with concurrent-race recovery
• siteUrl validation: upgraded to new URL() hostname parsing
• TypeScript TicketData interface: includes priority, dueDate, components, parent; description made optional
• dueDate validation: format validated via regex before passing to Jira
• listProjects pagination: bounded by MAX_PAGES = 100
• AGENTS.md and CLAUDE.md: updated to 23 packages with ticket-client in API clients row
• ADF link nodes validate href scheme via allowlist, rendering unsafe schemes as plain text
• #writeReauthFlag re-reads secret inside retry loop; clock.restore() moved to afterEach
• parent field validated against TICKET_KEY_REGEX; MAX_PAGES has a dedicated test
• Unwrapped httpClient passed to CredentialManagerFactory (auth endpoint has different rate-limit semantics)
• TypeScript declarations expose refreshAuthHeaders and forceRefreshAuthHeaders
• XSS link-scheme test covers javascript: and data: schemes with mixed-case coverage

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 9m 37s | Cost: $2.48 | Commit: 7a8433cf82773ef4ac93b754d7aac149ac80437b
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[ravverma]

Hey @prithipalpatwal,

This is a follow-up independent review after the automated review rounds. The security-critical work already done (SSRF gateway pinning, secret-path UUID validation, Retry-After capping, secret hygiene) holds up well. The findings below are gaps that the prior rounds did not surface, several of which are attacker-triggerable via the untrusted description field or affect token-lifecycle availability. Note: the CI Test check is currently pending.

Strengths

• The concurrent-refresh design for Atlassian's rotating single-use refresh tokens (pre-read collapse + #recoverFromAtlassian401 two-phase re-read) is the correct model for a stateless multi-Lambda deployment. oauth-credential-manager.js
• #fetchNewTokens validates the shape of a 200 token response before persisting, preventing a malformed success from corrupting Secret Manager. oauth-credential-manager.js
• No secret/token leakage into logs, thrown errors, or ticketUrl - confirmed across all operations. #requireOk never logs response bodies. jira-cloud-client.js:520
• marked is pinned to 18.0.5 with an integrity hash and adds zero transitive prod dependencies. Clean supply-chain surface.
• 429 handling is well-built: quota-vs-burst branching, jitter in [0.7, 1.3) to spread thundering herds, Retry-After honored. rate-limit-aware-http-client.js:150

Issues

Critical (Must Fix)

1. #isExpired treats a missing or NaN expiresAt as never-expiring. oauth-credential-manager.js:218 - Date.now() >= (secret.expiresAt - BUFFER) evaluates to false when expiresAt is undefined/null/non-numeric (comparison against NaN). A partial write, manual SM edit, or schema drift then silently disables all refresh logic: getAuthHeaders serves a possibly-dead Bearer token forever, and the refresh/recovery paths all early-exit treating the stale secret as valid. #fetchNewTokens validates Atlassian's response shape but nothing validates SM's own shape on read. Fix: in #readSecret, require Number.isFinite(secret.expiresAt) and presence of accessToken/refreshToken; treat missing/NaN as expired (fail-safe).

2. A live rotated refresh token is discarded if the SM write fails, bricking the connection. oauth-credential-manager.js:129 - Atlassian refresh tokens are single-use: once #fetchNewTokens returns 200, the old token in SM is dead and only the new refreshed.refresh_token can refresh again. If #writeTokens then exhausts its retries, it re-reads SM (dead old token), sees it expired, and force-marks the connection requires_reauth, throwing away the still-valid new token it held microseconds earlier. A transient SM blip becomes a mandatory human re-authorization, and under SM throttling this fires for every connection mid-refresh. Fix: persist the in-hand new token bundle more aggressively before falling to reauth; at minimum log at error that a live rotated token was dropped.

3. Unbounded recursion in the ADF converter crashes on untrusted input (DoS). markdown-to-adf.js - tokensToAdf/inlineToAdf recurse per nesting level with no depth guard, and markdownToAdf wraps lexer() + conversion in no try/catch. A description of roughly 3,000 > characters (or deeply nested lists) overflows the stack with an uncaught RangeError (verified locally against marked 18.0.5 - the conversion recursion overflows below marked's own lexer limit). Since description is untrusted suggestion-derived content, a ~3 KB payload reliably crashes createTicket as an uncategorized 500. Fix: cap input length, thread a depth counter through the recursion, and wrap conversion in try/catch that falls back to a single plain-text paragraph rather than throwing.

Important (Should Fix)

4. createTicket is a non-idempotent POST sitting under two retry layers - duplicate-ticket risk. jira-cloud-client.js - the create POST has no dedup key. If Jira commits the issue but the response is lost (connection reset, or Lambda timeout per item 5), the caller retries and files a second identical ticket. Jira Cloud offers no create-issue idempotency token, so mitigation is application-level (caller-supplied correlation key stored in a label, checked via JQL before create) or, at minimum, document loudly that createTicket is at-least-once, not exactly-once. Today nothing warns the caller.

5. No overall retry deadline, and the auth-retry layer nests a full 429 backoff loop inside itself. jira-cloud-client.js #withAuthRetry wraps requestFn, which is the RateLimitAwareHttpClient.fetch containing the 429 loop. A 401 re-invokes the entire MAX_RETRIES-deep 429 backoff again, so a single createTicket can accumulate ~75-80s of backoff sleep before the status fetch even runs. There is no aggregate deadline or Lambda-time awareness, so a caller on a common 30-60s timeout is killed mid-backoff (and per item 4, may have already created the ticket). Relatedly, MAX_WAIT_MS is applied only to the Retry-After branch, not the exponential fallback (rate-limit-aware-http-client.js:153), so the "hard ceiling" silently stops holding if MAX_RETRIES/BASE_BACKOFF_MS grow. Fix: thread a single deadline/AbortSignal through both layers and clamp the exponential fallback with Math.min(..., MAX_WAIT_MS).

6. Plain-text-only security contract is contradicted by the code, and HTML tokens break the documented invariant. jira-cloud-client.js createTicket JSDoc says "Plain-text description (no ADF markup)... untrusted content is never rendered as structured markup (spec 13)," but the implementation calls markdownToAdf(description) and the README encourages full markdown. Separately, markdown-to-adf.js never handles marked's html tokens, so <script>/<img onerror=...> fall through the default branch and are emitted verbatim into an ADF text node. Jira renders these as literal text (not stored XSS in Jira itself), but the file's own comment claims defense-in-depth "if ADF is consumed by other renderers" - which is only true for links, not raw HTML. Fix: reconcile the JSDoc/README/code on whether markdown is intended, and add an explicit case 'html': in both switch functions that strips tags so the "no active markup leaves this function" invariant is real.

7. Empty fenced code block produces ADF that Jira rejects, failing ticket creation. markdown-to-adf.js code branch builds content: [{ type: 'text', text: '' }] for an empty code fence. The file's own comment notes Jira rejects empty text nodes, and inlineToAdf filters them, but the code branch bypasses that filter. Attacker-influenced description content can deterministically break createTicket. Fix: content: token.text ? [{ type: 'text', text: token.text }] : [].

8. Atlassian OAuth failures are collapsed to { status } and only 401 triggers reauth. oauth-credential-manager.js:248 - the error body is never inspected, so invalid_client (misconfigured app credentials, a fleet-wide operational issue) is indistinguishable from invalid_grant (this user's token revoked). A client-credential rotation would stampede every connection into requires_reauth. Also, RFC 6749 5.2 mandates 400 invalid_grant for a bad refresh token and Atlassian often returns 403, but the code only branches on status 401, so the reauth path may rarely fire in practice. Fix: parse the JSON error body, route only invalid_grant to reauth, and trigger on 400/403 as well as 401.

9. Summary truncation splits multibyte characters. jira-cloud-client.js:337 - plain.slice(0, 255) operates on UTF-16 code units; a 255-unit slice of emoji/multibyte content can end on a lone high surrogate that JSON-serializes to a replacement char. The only truncation test uses 'A'.repeat(300) (pure ASCII) so it structurally cannot catch this. Fix: make truncation code-point aware and add boundary tests at 255/256 with multibyte content.

10. Only 429 is retried; 5xx, network errors, and timeouts are not, and no fetch timeout is enforced. rate-limit-aware-http-client.js:66 returns immediately for any non-429. Transient 502/503/504 (common during Atlassian incidents) fail the user request, and since the underlying fetch has no timeout, a hung Atlassian connection hangs the Lambda until it is killed. Fix: add bounded retry for 5xx and network errors (respecting the item 5 deadline, and not on the non-idempotent POST without the item 4 guard), and enforce or document a required per-request timeout.

11. The re-auth handshake is undocumented and unreachable for factory consumers. refreshAuthHeaders/forceRefreshAuthHeaders are the only methods that actually consume a refresh token, but nothing in the package calls them - #withAuthRetry only throws TOKEN_REFRESH_REQUIRED. Yet TicketClientFactory.create returns a BaseTicketClient with no accessor to its credentialManager, so a factory-based consumer literally cannot call the refresh methods this package exports, and the README's Error Handling section never explains the loop. Fix: document the catch-TOKEN_REFRESH_REQUIRED then refresh then retry handshake, and either expose the manager or move the reactive refresh inside the client.

12. Thin observability on the failure paths that matter. There is no log when all retries are exhausted and the final 429 is returned (rate-limit-aware-http-client.js:79 returns silently), reauth/refresh events are only at debug, and there are no metrics for retry-exhausted / quota-exhausted / reauth-triggered. In prod these failure modes are a black box. Fix: warn/error on retry exhaustion and reauth transitions, and emit structured metric fields.

13. Secret Manager read amplification: getAuthHeaders reads SM on every API call. oauth-credential-manager.js - a single user action (paginated list + list types + create + status) is easily 6-10+ GetSecretValue calls, and paginated listProjects reads per page. At scale this contends against account-wide SM throttling and manifests as auth failures unrelated to the tokens. Fix: cache the decoded secret in-memory with a short TTL on the read path (keep race-recovery reads uncached).

14. Composition and shared-primitive divergence from the rest of the monorepo. Every other client exposes static createFrom(context) and pulls env/log/fetch from context; this package requires callers to hand-assemble smClient/httpClient/log. It also hand-rolls UUID/URL validation twice instead of using @adobe/spacecat-shared-utils (isValidUUID, isValidUrl, tracingFetch), and index.d.ts types smClient as bare object while the code assumes the AWS SDK v2 getSecretValue()/putSecretValue() call shape (v3 uses client.send(command)). Fix: add createFrom(context), adopt the shared utils, and define a minimal SecretsManagerClient interface in the types (or file tracked fast-follows).

15. Field/payload mappings are exercised but never asserted, so regressions stay green under 100% coverage. labels.map(String) output (jira-cloud-client.js:448), the attachment Blob's MIME type and byte content (597), and sanitizeFilename's control-char/null-byte/over-length handling (350) are all covered by tests that assert only call counts or partial results. Add assertions on the serialized request payloads.

Minor (Nice to Have)

• README states 429 fallback backoff is "1s, 2s, 4s, 8s" but the code uses BASE_BACKOFF_MS = 2000 producing 2s, 4s, 8s, 16s. On-call responders reading this during an incident will trust wrong numbers. README.md
• Set redirect: 'error' (or 'manual') on credential-bearing fetches in #fetchNewTokens and Jira calls. The SSRF fix pins the initial URL but the injected fetch follows redirects by default, so a redirect could carry the Bearer token off the pinned host. oauth-credential-manager.js
• Magic-byte validation is signature-presence only: text/* types skip checks entirely, and the WebP check is satisfiable by any RIFF container. Consider aligning the filename extension to the validated MIME type. jira-cloud-client.js:292
• listProjects return type disagrees across JSDoc ({key, name}), code ({id, key, name}), and README - and id is exactly what listIssueTypes(projectId) needs, so dropping it misleads consumers on how to chain the calls. jira-cloud-client.js:239
• #buildTicketResult host-mismatch guard is a security invariant marked c8 ignore with zero coverage; make it testable rather than ignored. jira-cloud-client.js:743
• Inline code inside **bold** silently drops the strong mark (hard-coded codeMarks), and nested lists / ordered-list start values are untested. markdown-to-adf.js:50
• summary is marked required in the types but never validated present, so an omitted summary is sent as "" and fails opaquely at Jira; labels/components throw a raw TypeError on non-array input. Fail fast locally like projectKey does. jira-cloud-client.js

Recommendations

• The two highest-leverage fixes are: (a) validate the SM secret schema on read with the same rigor already applied to Atlassian's response, and (b) thread a single deadline/AbortSignal through both retry layers. Together these resolve items 1, 5, and 10 at the root.
• Add a targeted test that feeds <script>, <img onerror>, deeply nested >, and empty code fences to markdownToAdf and asserts either a clean result or a categorized error - the current Jira test file does not cover the html-token or deep-nesting paths, which is why they were missed.

Assessment

Ready to merge? No, with fixes. The prior review rounds handled the classic security checklist well, but the approval rests on invariants (never-crashing plain-text conversion, non-expiring-token detection, retry ceilings) that are not actually enforced. Items 1-3 are attacker-triggerable or availability-critical and no existing test exercises them, so they should block merge. Items 4-15 are important robustness and contract gaps that are far cheaper to address now than after consumers wire against the current shape.

Next Steps

1. Fix the 3 critical items first (SM secret-shape validation, rotated-token-drop, ADF recursion/crash guard).
2. Address the important items - the idempotency/retry-deadline pair (items 4, 5, 10) and the plain-text-contract reconciliation (items 6, 7) are the most consequential.
3. Minor items are optional polish.

---

Out of scope, worth tracking: the DB task_management_provider enum, connection-validation schemas, and the Vault-to-Lambda env-var injection for the OAuth client credentials all live outside this repo but are hard dependencies of this package's correctness.