chore(webapp): admin endpoint to backfill Vercel deployment external ids - #4770
Conversation
|
WalkthroughAdds an authenticated admin POST endpoint for Vercel external-ID backfills. The endpoint validates batching, concurrency, recency, cursor, and dry-run parameters. The service paginates eligible environments, discovers current and recent deployments, derives normalized external IDs, and updates missing values with guarded writes. It reports errors, counts, cursor continuation, and completion status. Integration tests cover filtering, dry runs, idempotency, pagination, and duplicate project integrations. Merge Risk: ⚪ Minimal · up to This admin backfill adds scoped remediation for older Vercel deployments without a demonstrated production-impacting defect at the current head. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the purpose and implementation, but it does not follow the repository template. It omits the required issue-closing line, checklist, Testing section, Changelog section, and Screenshots section. Resolution Update the description to include the required template sections. Add a valid "Closes #<issue>" entry, complete the checklist, describe the test steps and results, add a short changelog entry, and include screenshots or state that screenshots are not applicable. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Observability mapAs of 19/100 over 449 measured of 467 entry points (base 19, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
e758fbf to
d166ded
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Skew protection resolves a run's worker by (environmentId, externalId, status=DEPLOYED). A miss parks the run and then expires it, so deployments predating the feature — which already carry the same value in commitSHA — need externalId populated to stay reachable. Vercel instant-rollback is the sharpest case, which is why the scope is the current promotion plus a recent window rather than current alone. Follows the existing backfill shape: admin PAT, keyset cursor over environments, per-environment action results, pMap, dryRun defaulting to true. Reuses normalizeExternalDeploymentId so a backfilled id is byte-identical to what a build writes, and the update re-checks externalId IS NULL so a deploy landing mid-backfill keeps its own id. Refs TRI-13464.
d166ded to
1c05d5e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/test/vercelExternalIdBackfill.test.ts (1)
388-401: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm the Proxy stub is an accepted exception to the no-mocking rule.
The repository guideline states "Never mock anything - use testcontainers instead." This Proxy replaces the
runtimeEnvironmentdelegate to force a lookup failure. The covered path is valuable and testcontainers cannot reproduce a replica error deterministically, so keeping the stub is reasonable. Please confirm the exception with the maintainers, or move the fault injection behind an injectable read function inBackfillOptions.As per coding guidelines, "We use vitest exclusively. Never mock anything - use testcontainers instead."
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e5e43ea-3885-4d97-941f-439f3e0153d4
📒 Files selected for processing (2)
apps/webapp/app/v3/services/vercelExternalIdBackfill.server.tsapps/webapp/test/vercelExternalIdBackfill.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (33)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: typecheck / typecheck
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: runops-guard / runops-guard
- GitHub Check: report
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (14)
New code must target Run Engine V2 through the singleton in `app/v3/runEngine.server.ts`; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.ts
Test files must not import `app/env.server.ts`; pass configuration as options instead.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/test/vercelExternalIdBackfill.test.tsapps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts
🧠 Learnings (1)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/test/vercelExternalIdBackfill.test.ts
🔇 Additional comments (9)
apps/webapp/app/v3/services/vercelExternalIdBackfill.server.ts (5)
88-117: Add crumb instrumentation for page selection and batch completion.The coding guidelines require crumbs while developing this path. Mark page selection and per-project completion with
//@Crumbsor a `// `#region` `@crumbsblock.As per coding guidelines, "Add crumbs as you write code — not just when debugging."
Source: Coding guidelines
1-37: LGTM!
50-129: LGTM!
153-205: LGTM!
213-273: LGTM!apps/webapp/test/vercelExternalIdBackfill.test.ts (4)
6-140: LGTM!
143-321: LGTM!
323-380: LGTM!
1-4: 📐 Maintainability & Code QualityNo change needed.
vercelExternalIdBackfill.server.tsimports~/services/logger.server, which does not import~/env.server; its transitive local imports also do not import it.
2 participants
Skew protection resolves a run's worker by (environmentId, externalId,
status=DEPLOYED). A miss parks the run and then expires it, so deployments
predating the feature — which already carry the same value in commitSHA — need
externalId populated to stay reachable. Vercel instant-rollback is the sharpest
case, which is why the scope is the current promotion plus a recent window
rather than current alone.
Follows the existing backfill shape: admin PAT, keyset cursor over environments,
per-environment action results, pMap, dryRun defaulting to true. Reuses
normalizeExternalDeploymentId so a backfilled id is byte-identical to what a
build writes, and the update re-checks externalId IS NULL so a deploy landing
mid-backfill keeps its own id.
Refs TRI-13464.