feat: harden poll logic

[kasin-it]

Summary by CodeRabbit

• Bug Fixes

  • Validate ticket project key and AI column before acting; skip and clean up with clear skip reasons when mismatched. Preserve runs if live verification shows ticket still in the AI column.

• Webhooks

  • When payload status differs from AI column, perform live verification and either dispatch or cancel tracked runs accordingly.

• Adapters

  • Improved issue-tracker error handling for not-found cases and added project-key extraction.

• Tests

  • Added environment-mocked tests covering project/key and AI-column control flows.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds runtime guards that verify a ticket's Jira project key and AI column before starting workflows; unregisters or cancels runs when mismatches are detected. Normalizes and deduplicates discovered AI-column ticket keys during cron polling. Reconciliation and webhook flows now perform live issue-tracker verification (including 404 handling) before cancelling or cleaning runs.

Changes

Cohort / File(s)	Summary
Dispatch logic & types src/lib/dispatch.ts	Read env into expectedProjectKey and expectedAiStatus; added extractProjectKey(); verify ticket.trackerStatus and derived project key after fetchTicket; on mismatch call runRegistry.unregister() (best-effort) and return `{ started: false, reason: "not_in_ai_column"
Dispatch tests src/lib/dispatch.test.ts	Mocked ../../env.js; tightened existing dispatch test to require matching project and AI column; added tests for early-exit cases (not-in-AI-column, wrong-project-key) asserting unregister is called and workflow start is not invoked.
Cron polling & discovery src/routes/cron/poll.get.ts	Quoted JQL project value; normalize/trim/uppercase/filter/dedupe discovered ticket keys and return normalized list; warn when normalized count differs from raw results; pass adapters.issueTracker into reconcileRuns.
Reconcile runtime & tests src/lib/reconcile.ts, src/lib/reconcile.test.ts	reconcileRuns accepts optional issueTracker and threads it into reconciliation; added verifyTicketLeftAiColumn, resolveTicketProjectKey, and error-code handling; orphan cancellation/cleanup gated on verification (404 treated as left, other errors skip cancellation); tests exercise verification branches and error handling.
Webhook handler src/routes/webhooks/jira.post.ts	When payload status differs from env.COLUMN_AI, perform live adapters.issueTracker.fetchTicket to verify current AI-column membership; if still in AI column, dispatch; if left, attempt to cancel tracked run via new cancelTrackedRun flow and return early with left_ai_column reason.
Issue-tracker adapter & types src/adapters/issue-tracker/jira.ts, src/adapters/issue-tracker/types.ts, src/adapters/issue-tracker/jira.test.ts	fetchTicket now requests project field, populates projectKey (with fallback extraction), and throws IssueTrackerNotFoundError on 404; added IssueTrackerNotFoundError type and optional projectKey on TicketContent; tests added for 404 behavior.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Dispatch as dispatchTicket
  participant Jira as issueTracker.fetchTicket
  participant Registry as runRegistry.unregister
  participant Workflow as agentWorkflow.start

  Caller->>Dispatch: dispatchTicket(ticketKey)
  Dispatch->>Jira: fetchTicket(ticketKey)
  Jira-->>Dispatch: ticket { identifier, trackerStatus, ... }
  alt project key mismatch
    Dispatch->>Registry: unregister(ticketKey) (best-effort)
    Dispatch-->>Caller: { started: false, reason: "wrong_project_key" }
  else trackerStatus mismatch
    Dispatch->>Registry: unregister(ticketKey) (best-effort)
    Dispatch-->>Caller: { started: false, reason: "not_in_ai_column" }
  else proceed
    Dispatch->>Workflow: start(...)
    Workflow-->>Dispatch: started result
    Dispatch-->>Caller: { started: true, ... }
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Aiw 16 cicd info #38: Modifies dispatch/reconcile control flow and expands DispatchResult.reason — strong overlap with these dispatch/reconcile changes.
• Aiw 25 safeguard #37: Changes the same dispatch/reconcile paths and related tests — likely related at the code level.

Poem

🐰
I hop through keys both trimmed and neat,
I check the project and the AI seat.
If status or prefix goes awry,
I nudge the run and let it fly.
Tiny paws keep workflows sweet! 🥕

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: harden poll logic' is partially related to the changeset. While poll hardening is one aspect (ticket normalization in discoverAiColumnTickets), the core changes span multiple layers: dispatch validation, reconciliation verification, webhook handling, and error modeling—much broader than just poll logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/dispatch.ts`:
- Around line 7-9: Module-level access to env (EXPECTED_PROJECT_KEY,
EXPECTED_AI_STATUS) runs at import time and breaks tests; move the env
validation/trim/transform logic into a lazy initializer or inside dispatchTicket
so env is read only when the function is called (or create helper functions like
getExpectedProjectKey() and getExpectedAiStatus() that compute and cache the
values on first call). Update all references to EXPECTED_PROJECT_KEY and
EXPECTED_AI_STATUS to call those helpers (or compute inside dispatchTicket) so
tests can mock env before dispatch.js reads it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eafe7897-9ade-400e-9d8e-a9a726fa7257

📥 Commits

Reviewing files that changed from the base of the PR and between 9a77c51 and b1d219c.

📒 Files selected for processing (3)

• src/lib/dispatch.test.ts
• src/lib/dispatch.ts
• src/routes/cron/poll.get.ts

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/routes/cron/poll.get.ts (1)

73-85: Canonicalize keys before deduping.

startsWith(expectedPrefix) is case-insensitive, but Set dedup is not. PROJ-123 and proj-123 both survive today, so this normalization step can still dispatch/reconcile the same ticket twice if mixed-case keys ever appear.

♻️ Proposed change

 function normalizeTicketKeys(ticketKeys: string[]): string[] {
   const expectedPrefix = `${env.JIRA_PROJECT_KEY.trim().toUpperCase()}-`;
   const unique = new Set<string>();

   for (const rawKey of ticketKeys) {
-    const key = typeof rawKey === "string" ? rawKey.trim() : "";
+    const key = typeof rawKey === "string" ? rawKey.trim().toUpperCase() : "";
     if (!key) continue;
-    if (!key.toUpperCase().startsWith(expectedPrefix)) continue;
+    if (!key.startsWith(expectedPrefix)) continue;
     unique.add(key);
   }

   return [...unique];
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/routes/cron/poll.get.ts` around lines 73 - 85, In normalizeTicketKeys,
canonicalize keys to a consistent case before deduping so mixed-case duplicates
(e.g., "PROJ-123" vs "proj-123") collapse. Specifically, after trimming the
rawKey produce a normalizedKey (e.g., key.toUpperCase()) and use that for the
startsWith check against expectedPrefix and for adding into the unique Set
(instead of adding the original key); return the array of these canonicalized
keys so deduping is case-insensitive. Ensure the function still trims input and
skips empty values; update uses of expectedPrefix, key, and unique within
normalizeTicketKeys accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/reconcile.ts`:
- Around line 69-99: verifyTicketLeftAiColumn currently only checks
trackerStatus and string-parses errors for "404", which can let tickets that
moved projects slip through and is fragile; update the flow so
verifyTicketLeftAiColumn uses the project key from the fetched ticket (e.g.,
ticket.projectKey or TicketContent.projectKey) and confirms both status !==
env.COLUMN_AI and projectKey === env.JIRA_PROJECT_KEY, and stop relying on
/\b404\b/.test(message) by changing the IssueTrackerAdapter.fetchTicket contract
to surface a typed NotFoundError or a typed result (or an error.code property)
and then detect that explicit type/property in verifyTicketLeftAiColumn (use
instanceof NotFoundError or err.code === 'NOT_FOUND') to return true for missing
tickets and log other errors as before.

In `@src/routes/webhooks/jira.post.ts`:
- Around line 47-59: The code currently cancels based solely on the webhook
payload; instead fetch the current ticket state from adapters.issueTracker
(e.g., call the issue fetch method for ticketKey) or call the existing
reconciliation validator before invoking cancelTrackedRun so you only cancel
when the live status is still not an AI column. Concretely: after computing
ticketStatus from extractTicketStatus and before calling
cancelTrackedRun(ticketKey, adapters.runRegistry), query adapters.issueTracker
for the current issue status (or call the reconciliation validator function used
elsewhere), re-evaluate with isAiColumnStatus, and only call cancelTrackedRun if
the live status confirms the ticket remains outside the AI column; update the
logged fields accordingly.

---

Nitpick comments:
In `@src/routes/cron/poll.get.ts`:
- Around line 73-85: In normalizeTicketKeys, canonicalize keys to a consistent
case before deduping so mixed-case duplicates (e.g., "PROJ-123" vs "proj-123")
collapse. Specifically, after trimming the rawKey produce a normalizedKey (e.g.,
key.toUpperCase()) and use that for the startsWith check against expectedPrefix
and for adding into the unique Set (instead of adding the original key); return
the array of these canonicalized keys so deduping is case-insensitive. Ensure
the function still trims input and skips empty values; update uses of
expectedPrefix, key, and unique within normalizeTicketKeys accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1c214b85-ff68-4679-9a2b-efa26a02a5b9

📥 Commits

Reviewing files that changed from the base of the PR and between 40e4077 and f3bfafc.

📒 Files selected for processing (5)

• src/lib/dispatch.ts
• src/lib/reconcile.test.ts
• src/lib/reconcile.ts
• src/routes/cron/poll.get.ts
• src/routes/webhooks/jira.post.ts

✅ Files skipped from review due to trivial changes (1)

• src/lib/dispatch.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/reconcile.test.ts

[coderabbitai]

🧹 Nitpick comments (2)

src/routes/webhooks/jira.post.ts (1)

211-217: Consider extracting shared project-key extraction logic.

extractProjectKeyFromIdentifier duplicates logic in jira.ts:extractProjectKey and reconcile.ts:resolveTicketProjectKey. While each has minor differences, consolidating to a shared utility would reduce maintenance burden.

This is a minor refactor that could be deferred—the current implementations are correct and the slight variations (return type, input handling) may justify keeping them separate.

Also applies to: 108-121

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/routes/webhooks/jira.post.ts` around lines 211 - 217, Consolidate
duplicate project-key extraction logic by creating a shared utility (e.g.,
extractProjectKey) that implements the canonical behavior: trim the input,
return null for empty or missing dash or dash at position 0, and return the
segment before the first dash upper-cased; then replace usages of
extractProjectKeyFromIdentifier (in jira.post.ts), extractProjectKey (in
jira.ts) and resolveTicketProjectKey (in reconcile.ts) to call the new shared
function and update imports accordingly so all three call the single
implementation with the same return type and input handling.

src/adapters/issue-tracker/jira.test.ts (1)

53-64: Good test coverage for 404 handling.

The test correctly verifies that a 404 response throws IssueTrackerNotFoundError. Consider additionally asserting on the error's code property to ensure the duck-typed detection will work:

✨ Optional: Assert error code property

       const adapter = jiraAdapter();
-      await expect(adapter.fetchTicket("10001")).rejects.toBeInstanceOf(
-        IssueTrackerNotFoundError,
-      );
+      await expect(adapter.fetchTicket("10001")).rejects.toSatisfy(
+        (err: IssueTrackerNotFoundError) =>
+          err instanceof IssueTrackerNotFoundError && err.code === "NOT_FOUND",
+      );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/adapters/issue-tracker/jira.test.ts` around lines 53 - 64, Update the
test for jiraAdapter.fetchTicket to not only expect an IssueTrackerNotFoundError
but also assert the thrown error's code property used for duck-typing; catch or
await the rejection (e.g., with try/catch or using rejects.toEqual/async
assertion helpers) and assert error.code is defined and equals the adapter's
404-identifying value so that IssueTrackerNotFoundError detection is verified in
addition to the class type.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/adapters/issue-tracker/jira.test.ts`:
- Around line 53-64: Update the test for jiraAdapter.fetchTicket to not only
expect an IssueTrackerNotFoundError but also assert the thrown error's code
property used for duck-typing; catch or await the rejection (e.g., with
try/catch or using rejects.toEqual/async assertion helpers) and assert
error.code is defined and equals the adapter's 404-identifying value so that
IssueTrackerNotFoundError detection is verified in addition to the class type.

In `@src/routes/webhooks/jira.post.ts`:
- Around line 211-217: Consolidate duplicate project-key extraction logic by
creating a shared utility (e.g., extractProjectKey) that implements the
canonical behavior: trim the input, return null for empty or missing dash or
dash at position 0, and return the segment before the first dash upper-cased;
then replace usages of extractProjectKeyFromIdentifier (in jira.post.ts),
extractProjectKey (in jira.ts) and resolveTicketProjectKey (in reconcile.ts) to
call the new shared function and update imports accordingly so all three call
the single implementation with the same return type and input handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9f3e023-cfa0-47e0-85a2-2974fba74764

📥 Commits

Reviewing files that changed from the base of the PR and between f3bfafc and fc3ae08.

📒 Files selected for processing (7)

• src/adapters/issue-tracker/jira.test.ts
• src/adapters/issue-tracker/jira.ts
• src/adapters/issue-tracker/types.ts
• src/lib/reconcile.test.ts
• src/lib/reconcile.ts
• src/routes/cron/poll.get.ts
• src/routes/webhooks/jira.post.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/routes/cron/poll.get.ts