-
Notifications
You must be signed in to change notification settings - Fork 11
chore: remove event worker #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR removes the Redis/BullMQ-based event/queue system and the entire Event Worker app. It deletes Redis configs, clients, and instrumentation, removes related Docker and CI artifacts, drops Azure consent/scan routes, and rewires event dispatch to always use the Kafka dispatcher. API/router code paths are adjusted to avoid queue enqueues. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor API
participant Events as @ctrlplane/events
participant Kafka as Kafka Broker
Note over API,Events: New flow (Kafka-only)
API->>Events: eventDispatcher.dispatchX(...)
Events->>Kafka: Produce event
Kafka-->>Consumers: Deliver event
Note over Kafka,Consumers: No Redis/BullMQ queues or workers
sequenceDiagram
autonumber
actor API
participant Queues as BullMQ (removed)
participant Worker as Event Worker (removed)
Note over API,Queues: Old flow (removed)
API->>Queues: getQueue(Channel.X).add(...)
Queues->>Worker: Job pulled and processed
Worker-->>Queues: Enqueue follow-up jobs
Note over All: Entire BullMQ/Redis path deleted
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewersPoem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/api/package.json (1)
11-14: Remove or update './queues' export in package.json
The./queuesexport targets missing files (src/queues/index.ts,dist/queues.d.ts); remove it or adjust to a valid module.packages/api/src/router/resource/resource-provider.ts (1)
167-175: Breaking change:managed.syncis now a silent no-op, breaking existing callers
- UI components (
ProviderActionsDropdown.tsx:112,PermissionsButton.tsx:27) callsync.mutateAsync(...)then refresh/navigation, assuming a real sync—now they silently reload stale data.- No error, warning, or docs indicate the endpoint is deprecated or replaced.
Choose one remediation:
- Throw an informative error (e.g. “Resource sync deprecated; use Kafka-based mechanism”)
- Return a deprecation response object instead of
void- Rewire this mutation to trigger the new Kafka-based sync
Also update caller logic or user messaging accordingly.
🧹 Nitpick comments (6)
packages/api/package.json (1)
38-38: Deduplicate @octokit/types (deps vs devDeps).It’s listed in both. Keep it only in devDependencies (types-only).
"dependencies": { "@aws-sdk/client-iam": "^3.696.0", "@aws-sdk/client-sts": "^3.696.0", "@ctrlplane/auth": "workspace:*", "@ctrlplane/db": "workspace:*", "@ctrlplane/events": "workspace:*", "@ctrlplane/job-dispatch": "workspace:*", "@ctrlplane/logger": "workspace:*", "@ctrlplane/rule-engine": "workspace:*", "@ctrlplane/secrets": "workspace:*", "@ctrlplane/validators": "workspace:*", "@octokit/auth-app": "catalog:", "@octokit/rest": "catalog:", - "@octokit/types": "^13.5.0", "@t3-oss/env-core": "catalog:",Also applies to: 58-58
packages/api/src/router/policy/approval/add-record.ts (1)
64-67: Consider parallel dispatching for better performance and resilience.The sequential
awaitinside the loop can slow down the operation when many release targets are affected. Additionally, if one dispatch throws an error, the remaining targets won't be processed.Consider using
Promise.allSettledfor parallel dispatching with independent error handling:- for (const releaseTarget of affectedReleaseTargets) - await eventDispatcher.dispatchEvaluateReleaseTarget(releaseTarget, { - skipDuplicateCheck: true, - }); + await Promise.allSettled( + affectedReleaseTargets.map((releaseTarget) => + eventDispatcher.dispatchEvaluateReleaseTarget(releaseTarget, { + skipDuplicateCheck: true, + }), + ), + );This processes all dispatches concurrently and ensures one failure doesn't block others.
packages/api/src/router/policy/version-selector.ts (2)
134-139: Consider adding error handling for the dispatch call.The
await eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy)call lacks error handling. If the dispatch fails, the mutation will throw an unhandled error, potentially leaving the UI in an inconsistent state (the database update succeeded but the dispatch failed).Consider wrapping the dispatch in a try-catch or using
.catch()to handle failures gracefully:- await eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy); + try { + await eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy); + } catch (error) { + console.error("Failed to dispatch policy update:", error); + // Consider whether to throw or continue + }Alternatively, if dispatch failures should be treated as non-critical, you could fire-and-forget without
await.
100-100: Consider improving the error message for debugging.The generic error message "Policy not found" could be more helpful by including the policy ID that was not found.
Apply this diff to improve the error message in both mutations:
- if (fullPolicy == null) throw new Error("Policy not found"); + if (fullPolicy == null) throw new Error(`Policy not found: ${policyId}`);Also applies to: 138-138
packages/api/src/router/redeploy.ts (1)
46-66: Note the mixed dispatch strategy.The
handleDeploymentfunction now uses two different dispatch mechanisms:
- Force deployments (lines 51-56): Direct synchronous job creation via
createReleaseJob- Normal deployments (lines 59-65): Event-based dispatch via
eventDispatcher.dispatchEvaluateReleaseTargetThis appears intentional as part of removing the BullMQ queue system while retaining the Kafka-based event dispatcher for evaluation flows. Consider documenting this architectural decision if it's not already captured elsewhere.
packages/job-dispatch/src/config.ts (1)
5-5: The removal of REDIS_URL validation aligns with PR objectives; however, verify external package dependencies before removing theenvexport.The
envexport fromconfig.tsis not used internally within the codebase. However, since it is re-exported as part of the@ctrlplane/job-dispatchpublic API (viaindex.ts), removing it could be a breaking change for external consumers. Before removing the file or this export, confirm that no external packages depend on it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (89)
.env.example(0 hunks).github/workflows/apps-event-worker.yaml(0 hunks)apps/event-queue/src/instrumentation-node.ts(0 hunks)apps/event-worker/Dockerfile(0 hunks)apps/event-worker/README.md(0 hunks)apps/event-worker/eslint.config.js(0 hunks)apps/event-worker/package.json(0 hunks)apps/event-worker/src/config.ts(0 hunks)apps/event-worker/src/github-utils.ts(0 hunks)apps/event-worker/src/index.ts(0 hunks)apps/event-worker/src/instrumentation-node.ts(0 hunks)apps/event-worker/src/instrumentation.ts(0 hunks)apps/event-worker/src/redis.ts(0 hunks)apps/event-worker/src/utils/omit-null-undefined.ts(0 hunks)apps/event-worker/src/workers/compute-deployment-resource-selector.ts(0 hunks)apps/event-worker/src/workers/compute-environment-resource-selector.ts(0 hunks)apps/event-worker/src/workers/compute-policy-target-release-target-selector.ts(0 hunks)apps/event-worker/src/workers/compute-systems-release-targets.ts(0 hunks)apps/event-worker/src/workers/compute-workspace-policy-targets.ts(0 hunks)apps/event-worker/src/workers/delete-deployment.ts(0 hunks)apps/event-worker/src/workers/delete-environment.ts(0 hunks)apps/event-worker/src/workers/delete-resource.ts(0 hunks)apps/event-worker/src/workers/deleted-release-target.ts(0 hunks)apps/event-worker/src/workers/evaluate-release-target.ts(0 hunks)apps/event-worker/src/workers/index.ts(0 hunks)apps/event-worker/src/workers/job-dispatch/github.ts(0 hunks)apps/event-worker/src/workers/job-dispatch/index.ts(0 hunks)apps/event-worker/src/workers/job-update/concurrency.ts(0 hunks)apps/event-worker/src/workers/job-update/db-update-job.ts(0 hunks)apps/event-worker/src/workers/job-update/index.ts(0 hunks)apps/event-worker/src/workers/job-update/job-metadata.ts(0 hunks)apps/event-worker/src/workers/job-update/job-retry.ts(0 hunks)apps/event-worker/src/workers/job-update/matched-policies.ts(0 hunks)apps/event-worker/src/workers/job-update/trigger-dependent-targets.ts(0 hunks)apps/event-worker/src/workers/job-update/utils.ts(0 hunks)apps/event-worker/src/workers/new-deployment-version.ts(0 hunks)apps/event-worker/src/workers/new-deployment.ts(0 hunks)apps/event-worker/src/workers/new-environment.ts(0 hunks)apps/event-worker/src/workers/new-policy.ts(0 hunks)apps/event-worker/src/workers/new-resource.ts(0 hunks)apps/event-worker/src/workers/resource-scan/aws/aws.ts(0 hunks)apps/event-worker/src/workers/resource-scan/aws/eks.ts(0 hunks)apps/event-worker/src/workers/resource-scan/aws/vpc.ts(0 hunks)apps/event-worker/src/workers/resource-scan/azure/aks.ts(0 hunks)apps/event-worker/src/workers/resource-scan/azure/cluster-to-resource.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/client.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/cluster-to-resource.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/gke.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/kube.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/vcluster.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/vm.ts(0 hunks)apps/event-worker/src/workers/resource-scan/google/vpc.ts(0 hunks)apps/event-worker/src/workers/resource-scan/index.ts(0 hunks)apps/event-worker/src/workers/resource-scan/utils/extract-variables.ts(0 hunks)apps/event-worker/src/workers/update-deployment-variable.ts(0 hunks)apps/event-worker/src/workers/update-deployment.ts(0 hunks)apps/event-worker/src/workers/update-environment.ts(0 hunks)apps/event-worker/src/workers/update-policy.ts(0 hunks)apps/event-worker/src/workers/update-resource-variable.ts(0 hunks)apps/event-worker/src/workers/updated-resources/index.ts(0 hunks)apps/event-worker/src/workers/updated-resources/span.ts(0 hunks)apps/event-worker/tsconfig.json(0 hunks)apps/webservice/src/app/api/azure/[workspaceId]/[tenantId]/[subscriptionId]/[name]/route.ts(0 hunks)apps/webservice/src/app/api/azure/consent/route.ts(0 hunks)apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/evaluate/route.ts(0 hunks)apps/webservice/src/env.ts(0 hunks)apps/webservice/src/instrumentation-node.ts(0 hunks)docker-compose.dev.yaml(0 hunks)docker-compose.yaml(0 hunks)packages/api/package.json(1 hunks)packages/api/src/config.ts(0 hunks)packages/api/src/index.ts(0 hunks)packages/api/src/redis.ts(0 hunks)packages/api/src/router/policy/approval/add-record.ts(2 hunks)packages/api/src/router/policy/version-selector.ts(3 hunks)packages/api/src/router/redeploy.ts(2 hunks)packages/api/src/router/resource/resource-provider.ts(1 hunks)packages/db/package.json(0 hunks)packages/db/src/config.ts(0 hunks)packages/db/src/redis.ts(0 hunks)packages/events/package.json(0 hunks)packages/events/src/config.ts(0 hunks)packages/events/src/dispatch-jobs.ts(0 hunks)packages/events/src/index.ts(1 hunks)packages/events/src/redis.ts(0 hunks)packages/events/src/types.ts(0 hunks)packages/job-dispatch/src/config.ts(1 hunks)packages/job-dispatch/src/job-dispatch.ts(0 hunks)pnpm-workspace.yaml(0 hunks)
💤 Files with no reviewable changes (82)
- apps/event-worker/README.md
- docker-compose.dev.yaml
- apps/event-worker/src/workers/deleted-release-target.ts
- apps/event-worker/src/workers/compute-workspace-policy-targets.ts
- apps/event-worker/src/workers/delete-environment.ts
- apps/event-worker/src/workers/job-update/job-retry.ts
- apps/event-worker/src/workers/index.ts
- apps/webservice/src/app/api/azure/[workspaceId]/[tenantId]/[subscriptionId]/[name]/route.ts
- packages/job-dispatch/src/job-dispatch.ts
- apps/event-worker/src/workers/compute-deployment-resource-selector.ts
- pnpm-workspace.yaml
- apps/event-worker/src/index.ts
- apps/webservice/src/env.ts
- apps/event-worker/src/workers/update-policy.ts
- apps/event-worker/src/redis.ts
- apps/event-worker/src/workers/resource-scan/google/cluster-to-resource.ts
- apps/event-worker/src/workers/compute-environment-resource-selector.ts
- packages/db/package.json
- apps/event-worker/src/workers/updated-resources/index.ts
- apps/event-worker/src/workers/resource-scan/utils/extract-variables.ts
- apps/event-worker/src/workers/resource-scan/google/vpc.ts
- apps/event-worker/src/workers/resource-scan/google/vcluster.ts
- apps/event-worker/package.json
- apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/evaluate/route.ts
- apps/event-worker/src/workers/job-update/matched-policies.ts
- apps/event-worker/src/workers/new-resource.ts
- apps/event-worker/src/workers/resource-scan/google/gke.ts
- packages/db/src/config.ts
- packages/api/src/redis.ts
- apps/event-worker/src/workers/new-environment.ts
- apps/event-worker/src/workers/job-dispatch/index.ts
- packages/events/package.json
- apps/event-worker/src/workers/delete-resource.ts
- packages/events/src/redis.ts
- apps/event-worker/src/workers/delete-deployment.ts
- apps/event-worker/src/workers/job-update/utils.ts
- apps/event-worker/src/workers/update-deployment-variable.ts
- apps/event-worker/src/workers/job-update/db-update-job.ts
- apps/event-worker/src/workers/resource-scan/aws/eks.ts
- apps/event-worker/src/github-utils.ts
- apps/webservice/src/app/api/azure/consent/route.ts
- apps/event-worker/src/workers/resource-scan/aws/vpc.ts
- apps/event-worker/src/workers/new-deployment-version.ts
- apps/event-worker/src/workers/compute-systems-release-targets.ts
- apps/event-worker/src/instrumentation.ts
- .env.example
- packages/events/src/config.ts
- apps/event-worker/src/workers/resource-scan/google/vm.ts
- apps/event-worker/src/workers/update-deployment.ts
- apps/event-queue/src/instrumentation-node.ts
- apps/event-worker/src/utils/omit-null-undefined.ts
- apps/event-worker/src/workers/job-update/index.ts
- apps/event-worker/src/config.ts
- docker-compose.yaml
- apps/event-worker/src/workers/updated-resources/span.ts
- apps/event-worker/src/workers/update-environment.ts
- apps/event-worker/src/workers/resource-scan/aws/aws.ts
- packages/events/src/dispatch-jobs.ts
- apps/event-worker/src/workers/new-policy.ts
- .github/workflows/apps-event-worker.yaml
- apps/event-worker/src/workers/resource-scan/index.ts
- apps/webservice/src/instrumentation-node.ts
- apps/event-worker/src/workers/job-update/job-metadata.ts
- apps/event-worker/tsconfig.json
- apps/event-worker/Dockerfile
- packages/api/src/config.ts
- apps/event-worker/src/workers/job-update/trigger-dependent-targets.ts
- packages/api/src/index.ts
- apps/event-worker/src/workers/resource-scan/google/client.ts
- apps/event-worker/src/workers/resource-scan/google/kube.ts
- apps/event-worker/eslint.config.js
- packages/events/src/types.ts
- apps/event-worker/src/workers/resource-scan/azure/cluster-to-resource.ts
- packages/db/src/redis.ts
- apps/event-worker/src/workers/compute-policy-target-release-target-selector.ts
- apps/event-worker/src/workers/job-dispatch/github.ts
- apps/event-worker/src/workers/resource-scan/azure/aks.ts
- apps/event-worker/src/workers/evaluate-release-target.ts
- apps/event-worker/src/instrumentation-node.ts
- apps/event-worker/src/workers/update-resource-variable.ts
- apps/event-worker/src/workers/job-update/concurrency.ts
- apps/event-worker/src/workers/new-deployment.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
packages/job-dispatch/src/config.tspackages/api/src/router/redeploy.tspackages/api/src/router/resource/resource-provider.tspackages/api/src/router/policy/version-selector.tspackages/events/src/index.tspackages/api/src/router/policy/approval/add-record.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
packages/job-dispatch/src/config.tspackages/api/src/router/redeploy.tspackages/api/src/router/resource/resource-provider.tspackages/api/src/router/policy/version-selector.tspackages/events/src/index.tspackages/api/src/router/policy/approval/add-record.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
packages/job-dispatch/src/config.tspackages/api/src/router/redeploy.tspackages/api/src/router/resource/resource-provider.tspackages/api/src/router/policy/version-selector.tspackages/api/package.jsonpackages/events/src/index.tspackages/api/src/router/policy/approval/add-record.ts
🧬 Code graph analysis (4)
packages/api/src/router/redeploy.ts (2)
apps/event-queue/src/job-manager/job-manager.ts (1)
createReleaseJob(231-260)packages/db/src/queries/create-release-job.ts (1)
createReleaseJob(19-95)
packages/api/src/router/policy/version-selector.ts (2)
packages/db/src/utils/policies/index.ts (1)
rulesAndTargets(13-13)packages/events/src/index.ts (1)
eventDispatcher(7-7)
packages/events/src/index.ts (2)
packages/events/src/event-dispatcher.ts (1)
EventDispatcher(15-97)packages/events/src/kafka/event-dispatch/index.ts (1)
KafkaEventDispatcher(13-195)
packages/api/src/router/policy/approval/add-record.ts (2)
packages/db/src/schema/release.ts (1)
releaseTarget(21-47)packages/events/src/index.ts (1)
eventDispatcher(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (5)
packages/events/src/index.ts (1)
7-7: Clean refactor to Kafka-only dispatch successfully verified.The change correctly hardcodes the event dispatcher to Kafka. Verification confirms:
- No code references the removed BullMQ exports (
createWorker,getQueue,bullmqRedis)- No remaining
bullmqimports in the codebase- Existing exports (resource-provider-scan and kafka utilities) are preserved
The transition is safe and complete.
packages/api/src/router/policy/approval/add-record.ts (1)
5-5: LGTM! Import aligns with PR objective.The change from
dispatchQueueJobtoeventDispatchercorrectly aligns with removing the queue-based event worker system.packages/api/src/router/policy/version-selector.ts (1)
6-6: LGTM! Import changes align with the new dispatch pattern.The addition of
rulesAndTargetsand the switch fromdispatchQueueJobtoeventDispatcherare appropriate for the refactored policy update flow.Also applies to: 11-11
packages/api/src/router/redeploy.ts (2)
9-9: Import retention is correct.The
eventDispatcherimport is appropriately retained as it's still used in thehandleDeploymentfunction (line 61) for non-force deployment scenarios.
41-41: Benchmark synchronous job creation to avoid API latency issuesMoving from async queue dispatch to a direct
createReleaseJob(tx, …)call inredeploy.tscould increase request latency. Measure or monitor this endpoint under realistic load to ensure it remains within acceptable timeouts.
| }, | ||
| "dependencies": { | ||
| "@aws-sdk/client-iam": "^3.696.0", | ||
| "@aws-sdk/client-sts": "^3.696.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
STS client addition looks good; ensure repo‑wide consistency.
Aligned with IAM (^3.696.0). Please confirm other packages use the same STS version to avoid duplicate ranges.
🏁 Script executed:
#!/bin/bash
# Check repo for @aws-sdk/client-sts version consistency
fd package.json -t f \
| xargs -I{} jq -r '(.name // "UNKNOWN") + " -> " + ((.dependencies["\"@aws-sdk/client-sts\""] // .dependencies["@aws-sdk/client-sts"] // .devDependencies["@aws-sdk/client-sts"] // "N/A"))' {} \
| rg '@aws-sdk/client-sts' -n -C0Length of output: 111
🏁 Script executed:
#!/bin/bash
# Find all @aws-sdk/client-sts references with versions using ripgrep
echo "=== @aws-sdk/client-sts versions across monorepo ==="
rg '@aws-sdk/client-sts' --type json -n
echo ""
echo "=== @octokit/types duplicates check ==="
rg '@octokit/types' --type json -n packages/api/package.json
echo ""
echo "=== Verify export targets exist ==="
echo "Checking: packages/api/src/queues/index.ts"
fd -p 'src/queues/index.ts' packages/api
echo "Checking: packages/api/dist/queues.d.ts (or dist/queues/index.d.ts)"
fd -p 'queues' packages/api/dist 2>/dev/null || echo "(dist/ may not exist yet)"Length of output: 407
🏁 Script executed:
#!/bin/bash
# View the exports section of packages/api/package.json
echo "=== packages/api/package.json exports section ==="
jq '.exports' packages/api/package.json
echo ""
echo "=== Check if @octokit/types is imported in production code ==="
rg 'from.*@octokit/types|import.*@octokit/types' packages/api/src --type ts --type tsx
echo ""
echo "=== Check for queues references in package.json ==="
cat packages/api/package.json | jq '.exports | keys' 2>/dev/null || rg 'queues' packages/api/package.json -nLength of output: 396
Remove duplicate dependency and fix exports
- In packages/api/package.json, remove the
@octokit/typesentry under dependencies (keep the one in devDependencies). - Ensure the files
src/queues/index.tsanddist/queues.d.tsexist (or remove the./queuesexport mapping). - The new
@aws-sdk/client-stsentry is fine ([3.696.0] only appears here).
| const fullPolicy = await ctx.db.query.policy.findFirst({ | ||
| where: eq(schema.policy.id, policyId), | ||
| with: rulesAndTargets, | ||
| }); | ||
|
|
||
| if (fullPolicy == null) throw new Error("Policy not found"); | ||
| await eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the duplicate fullPolicy arguments.
The call eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy) passes the same policy object twice. If the method signature expects separate "old" and "new" state objects, this might not be the intended behavior for a create operation.
Run the following script to check the dispatchPolicyUpdated method signature:
🏁 Script executed:
#!/bin/bash
# Description: Verify the dispatchPolicyUpdated method signature to understand if both parameters should be the same
# Search for the dispatchPolicyUpdated method definition
ast-grep --pattern $'dispatchPolicyUpdated($_, $_) {
$$$
}'
# Also search for interface/type definitions
rg -nP --type=ts -A5 'dispatchPolicyUpdated.*\('Length of output: 4273
Pass the previous state for the previous parameter in dispatchPolicyUpdated calls.
The method signature expects dispatchPolicyUpdated(previous: FullPolicy, current: FullPolicy), but at lines 101 and 139 of version-selector.ts, the same fullPolicy object is passed for both parameters. Review the surrounding code to determine what the previous state should be (e.g., the policy before the version selector change was applied) and pass it as the first argument.
Compare with correct usage in packages/api/src/router/policy/router.ts:261:
await eventDispatcher.dispatchPolicyUpdated(prevPolicy, fullPolicy);🤖 Prompt for AI Agents
In packages/api/src/router/policy/version-selector.ts around lines 95–101 (and
similarly at line ~139), the code currently calls
eventDispatcher.dispatchPolicyUpdated(fullPolicy, fullPolicy); instead fetch or
capture the previous FullPolicy state before applying the version-selector
change (e.g., query the policy with the same where/with options or keep a
shallow/deep copy of the policy retrieved prior to the update) and pass that as
the first argument: await eventDispatcher.dispatchPolicyUpdated(prevPolicy,
fullPolicy); ensure prevPolicy contains the same shape (rulesAndTargets) as
fullPolicy and use that instead of reusing fullPolicy for both parameters.
Summary by CodeRabbit