fix: keep transient evaluation failures alert-free, timeout-aware and idempotent across redeliveries (lfx-cdp-insights-ops#215)

[ulemons]

Summary

Follow-up to #5008, addressing the four review findings on it (Copilot and Cursor Bugbot) that landed after the merge.

• No public-API alert on a transient 503. The error handler alerts on every 5xx, so each transient evaluator failure would have paged even when the worker retried successfully. The evaluation endpoint now answers the ServiceUnavailableError itself with the 503 body and a warn log, without going through the error handler. Test added.
• Schedule budget follows the retry policy. The 04:00 schedule budgeted 6 minutes per project (2 attempts × 3 min); with 3 attempts and the 30s/60s backoff the worst case is 10.5 minutes. The retry constants live in evaluator/retry.ts and the schedule derives the workflow execution timeout from them (11 minutes per project). The worker reconciles the schedule action at startup.
• Timeouts count as transient. The fetch to the evaluation API has a 150-second deadline (under the 3-minute activity timeout) so a stalled API becomes the typed transient failure, and the workflow also treats a Temporal TimeoutFailure in the cause chain as transient instead of recording a terminal error on the first run.
• Deferral is idempotent per run. The workflow passes the row it fetched at the start of the run; deferProjectCatalogEvaluation is guarded by "evaluationAttempts" = expected, so a redelivered markEvaluationFailed finds the counter already advanced, reports deferred and changes nothing. One transient failure can no longer consume two runs or finalize a fresh row as error. DAL test covers the redelivery.

No migration. Deploy api and projects-evaluation-worker (no evaluateProjects run in flight).

Refs linuxfoundation/lfx-cdp-insights-ops#215

Test plan

• pnpm tsc-check, pnpm lint --deny-warnings
• Unit tests: evaluation endpoint 503 path, evaluator client (fetch signal, typed failures), DAL deferral redelivery (CI), worker suites (82 tests)
• Prod: after a transient failure, no "public API" alert in the alerts channel; the run log shows deferred=N and the rows keep evaluationAttempts = 1

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Touches evaluation API error handling, Temporal retry/timeouts, and guarded DB updates for failure deferral; misconfiguration could still mark rows failed or under-alert, but changes are narrowly scoped with tests.

Overview
Hardens project evaluation when Bedrock/API calls fail temporarily: the worker can retry without paging on every 503, timeouts are treated as retryable, and duplicate activity deliveries no longer double-count deferrals.

The public project-evaluation endpoint now catches ServiceUnavailableError and returns 503 with a warn log locally, bypassing the global 5xx handler that would trigger public-API alerts on retried successes.

The evaluation worker client adds a 150s fetch deadline; stalled JSON reads become typed transient failures. The workflow also treats Temporal TimeoutFailure as transient. Retry attempt counts, backoff, and per-attempt timeouts are centralized in evaluator/retry.ts, and the scheduled workflow execution timeout is derived from that worst-case budget (~11 min/project vs the old 6 min assumption).

Catalog deferral/failure writes are guarded by evaluationAttempts = expectedAttempts, so redelivered markEvaluationFailed calls are idempotent (still report deferred without burning an extra run or marking stale rows as error).

^{Reviewed by Cursor Bugbot for commit b45d5f5. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

🟡 Changes recommended

Body-read timeouts and stale failure deliveries can still prematurely finalize retryable evaluations.

2 open findings

• Prevent stale deliveries from overwriting fresh evaluation retries · New
• Treat body-read timeouts as transient evaluation failures · New

What changed in this PR

This follow-up to #5008 improves transient-failure handling across the evaluation API, worker, and catalog persistence.

Changes:

• Returns transient 503 responses without public-API alerts.
• Adds request deadlines, recognizes Temporal timeouts, and derives schedule budgets from retry settings.
• Guards deferrals against duplicate delivery and adds regression coverage.

File	Description
services/​libs/​data-access-layer/​src/​project-catalog/​projectCatalog.ts	Guards deferrals with the expected attempt count.
services/​libs/​data-access-layer/​src/​project-catalog/​projectCatalog.test.ts	Tests duplicate deferral delivery.
services/​apps/​projects_evaluation_worker/​src/​workflows/​evaluateProjects.ts	Shares retry settings and recognizes timeouts.
services/​apps/​projects_evaluation_worker/​src/​schedules/​scheduleProjectsEvaluation.ts	Derives the execution timeout from retry settings.
services/​apps/​projects_evaluation_worker/​src/​evaluator/​retry.ts	Defines shared retry and deadline constants.
services/​apps/​projects_evaluation_worker/​src/​evaluator/​evaluator.ts	Adds a request deadline.
services/​apps/​projects_evaluation_worker/​src/​evaluator/​evaluator.test.ts	Checks that fetch receives an abort signal.
services/​apps/​projects_evaluation_worker/​src/​activities/​activities.ts	Detects previously applied deferrals.
backend/​src/​api/​public/​v1/​projectEvaluation/​projectEvaluationExec.ts	Handles transient 503 responses locally.
backend/​src/​api/​public/​v1/​projectEvaluation/​projectEvaluationExec.test.ts	Tests the local 503 response path.

🧠 Review effort: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Discovery rows lack a terminal Slack notification when persistent transient failures exhaust their retries.

0 open findings

2 resolved since last review

• Treat body-read timeouts as transient evaluation failures
• Prevent stale deliveries from overwriting fresh evaluation retries

Previously missed (1)

In code that hasn't changed since last review

Notify Slack when discovery evaluation retries are exhausted

backend/​src/​api/​public/​v1/​projectEvaluation/​projectEvaluationExec.ts:55

Handling the 503 here suppresses the public-API alert even when a discovery row exhausts all three evaluation runs. The workflow then records error and calls notifyEvaluationError, but that activity returns without notifying for any provenance other than slack-bot or github-discussion (services/apps/projects_evaluation_worker/src/activities/activities.ts:471–475). Persistent upstream failures therefore leave discovery rows in error without a terminal Slack notification. Keep retries alert-free, but add worker-side notification for these rows when retries are exhausted.

🧠 Review effort: Balanced