feat: platform features — SSO, billing, plan enforcement, audit log, chaos tests, Redis cluster

[pahuldeepp]

Summary

• SSO & multi-tenancy: Auth0-backed SSO, team member management, tenant isolation via RLS
• Billing & plan enforcement: Stripe integration, usage metering, quota gating, Resend email via jobs-worker
• Security hardening: rate limiting, DB input validation, security audit fixes (critical → low priority)
• Redis cluster: BFF and read-model-builder upgraded to UniversalClient for cluster-mode compatibility
• Audit log: device create/fail events logged via logAuditEvent
• E2E tests: Auth0 replaced with mock auth fixture for CI; e2e skipped when secrets absent
• Chaos test suite: pod kill, Kafka pause, Redis outage, projection lag, network partition scenarios
• Migrations (read-model-builder): 000003 create telemetry history, 000004 add event_id PK, 000005 perf indexes — all sequential, no collisions
• CI/CD: Go 1.25 compat, security workflow hardened, compose startup order fixed

Migration sequence (read-model-builder)

#	Name
000001	create_projection_tables
000002	add_rls_tenant_isolation
000003	create_telemetry_history
000004	add_event_id_to_telemetry_history
000005	add_perf_indexes (CONCURRENTLY)

Test plan

• docker compose up — all services healthy, migrations run clean (migrate up logs no errors)
• GraphQL devicesConnection — returns tenant-scoped results with cursor pagination
• Billing webhook endpoint responds 200 to test Stripe event
• Chaos suite: npm run test:chaos — all scenarios pass or gracefully degrade
• CI green on this PR

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Tenant plan enforcement (subscription, quotas, feature gates) and audit logging for device creation; dashboard modal closes on backdrop click.

• Tests

  • Added Playwright E2E suites, a k6 performance-budget test, and a suite of Kubernetes chaos experiments; CI workflows to run E2E, chaos, perf, terraform, and security checks.

• Chores

  • CI/tooling updates (Go 1.25), Redis cluster/universal client support and Redis-backed circuit sync, Makefile/test targets, Terraform infra (prod/DR/modules/backend) and Kafka MirrorMaker config, DB migrations for SSO/bulk/import.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9ea9e031-91a0-42a4-b16a-44732d2a9bf9

📥 Commits

Reviewing files that changed from the base of the PR and between 3c997c6 and e18f67b.

📒 Files selected for processing (3)

• .github/workflows/perf.yml
• apps/gateway/src/services/device.ts
• scripts/load-tests/performance-budget.js

---

📝 Walkthrough

Walkthrough

Adds multiple CI workflows and chaos/e2e/perf test suites; introduces multi-region Terraform infra and S3/DynamoDB backend; adds Redis cluster/universal-client support across services; implements gateway plan-enforcement middleware and audit device events; hardens saga repository and timeouts; adds chaos scripts, k6 load test, Makefile updates, DB migrations, and Kafka MirrorMaker2 config.

Changes

Cohort / File(s)	Summary
GitHub Actions Workflows .github/workflows/... .github/workflows/cd.yml, .github/workflows/chaos.yml, .github/workflows/e2e.yml, .github/workflows/perf.yml, .github/workflows/security.yml, .github/workflows/terraform.yml	Added chaos, e2e, perf, terraform workflows; updated CD Go version to 1.25; minor quoting/format tweaks in security workflow.
Makefile & local targets Makefile	Introduced COMPOSE var; added ci target; split test targets (test-go, test-gateway, test-dashboard, test-e2e); added typecheck/lint-fix; improved service health checks and added logs-ingest.
BFF Redis & CircuitBreaker apps/bff/src/datasources/redis.ts, apps/bff/src/lib/circuitBreaker.ts	Redis client now supports cluster via REDIS_CLUSTER_NODES with single-node fallback; CircuitBreaker syncs state to/from Redis (best-effort) before transitions.
Gateway: plan enforcement, audit & device flow apps/gateway/src/middleware/planEnforcement.ts, apps/gateway/src/lib/audit.ts, apps/gateway/src/server.ts, apps/gateway/src/routes/teamMembers.ts, apps/gateway/src/services/device.ts	Added plan/feature/quota middleware with Redis-cached tenant plans/counters; extended AuditEventType with device/webhook events; audit logging added for device create success/failure; fixed proto path and added gRPC response validation; inviterName set to "GrainGuard".
E2E Playwright tests & fixture tests/e2e/*, tests/e2e/playwright.config.ts, .github/workflows/e2e.yml	Added Playwright config, mockAuth fixture, and tests (auth, billing, devices); CI job builds dashboard, serves it locally, runs Playwright (Chromium/Firefox), and uploads reports.
Chaos experiments & orchestration tests/chaos/*, .github/workflows/chaos.yml	Added chaos experiments (pod-kill, kafka-consumer-pause, redis-outage, projection-lag, network-partition), run-all.sh, per-experiment scripts/YAML, README, and workflow (manual + weekly).
Performance tests & workflow scripts/load-tests/performance-budget.js, .github/workflows/perf.yml	Added k6 performance-budget script with scenarios, thresholds, summary handling; perf workflow spins up services, runs k6, and uploads results artifact.
Redis client generalization (Go) apps/read-model-builder/..., apps/saga-orchestrator/..., libs/health/health.go	Replaced *redis.Client with redis.UniversalClient across services and health checkers; read-model-builder uses NewUniversalClient handling cluster/single-node addresses and pool sizing.
Saga orchestrator & repository robustness apps/saga-orchestrator/internal/...	Added IsValidStatus() helper; fail-fast JSON marshal checks for saga/command payloads; introduced dbQueryTimeout and withTimeout; FindByCorrelationID now orders by created_at DESC with LIMIT 1.
Jobs worker & search indexer apps/jobs-worker/src/connection.ts, apps/search-indexer/main.py	AMQP connection now sets heartbeat and logs error/close events; search-indexer now writes separate device upsert and telemetry upsert with deterministic telemetry doc IDs.
Database migrations apps/telemetry-service/migrations/*	Added tenant email and current_period_end columns; added SSO tenant columns, alert_rules, and bulk_import_jobs tables with indexes; corresponding down migrations updated/added.
Terraform: backend, environments, modules infra/terraform/..., infra/terraform/modules/...	Added S3+DynamoDB backend, prod and dr environment roots, and aurora-global / elasticache-global modules supporting primary/secondary roles and DR wiring.
Kafka replication config infra/kafka/mirrormaker2-connector.json	Added MirrorMaker2 connector config for primary→DR topic replication (SASL_SSL SCRAM-SHA-512, topics, replication/task settings).
Node/TS tooling changes apps/bff/package.json, apps/gateway/package.json	BFF: added ESLint/typecheck scripts and devDependencies; Gateway: bumped minor @types/* devDependencies.
Frontend minor behavior apps/dashboard/src/features/devices/components/RegisterDeviceModal.tsx	Added backdrop click handling to close Register Device modal.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• feat(audit): add immutable audit logging for device operations #1: overlaps gateway audit and device logging additions (device.created, device.creation_failed).
• feat: platform features — SSO, billing, plan enforcement, audit log, chaos tests, Redis cluster #14: touches chaos CI/workflow, Redis universal-client changes, and planEnforcement/audit changes—strongly related.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.61% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 accurately captures the main changes: platform features including SSO, billing, plan enforcement, audit logging, chaos tests, and Redis cluster support.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/elated-lamarr

---

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

[coderabbitai]

Actionable comments posted: 55

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (10)

apps/read-model-builder/internal/projection/telemetry_projection.go (1)

311-318: ⚠️ Potential issue | 🔴 Critical

Missing rows.Err() check after rows.Close() — potential silent data loss.

Per coding guidelines, rows.Err() must be checked after rows.Close(). If an error occurs during iteration (e.g., network failure mid-stream), rows.Next() returns false but the error is only accessible via rows.Err(). Without this check, the function proceeds with a partial newEventIDs map, potentially causing events to be silently skipped.

As per coding guidelines: "Check pgx rows.Err() after rows.Close()."

🐛 Proposed fix

 		newEventIDs := make(map[string]struct{})
 		for rows.Next() {
 			var id string
 			if scanErr := rows.Scan(&id); scanErr == nil {
 				newEventIDs[id] = struct{}{}
 			}
 		}
 		rows.Close()
+		if err := rows.Err(); err != nil {
+			observability.EventsRetry.Inc()
+			return err
+		}
 
 		newEvents := make([]parsedEvent, 0, len(events))

🤖 Prompt for AI Agents

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

In `@apps/read-model-builder/internal/projection/telemetry_projection.go` around
lines 311 - 318, After iterating and calling rows.Close(), check rows.Err() and
handle any iteration error instead of proceeding with the partial newEventIDs
map; specifically, after the existing rows.Close() call invoke rows.Err(), and
if it returns a non-nil error return or propagate that error (or log and abort)
so the code that builds newEventIDs (using rows.Next and rows.Scan) does not
silently continue on an incomplete result set.

apps/saga-orchestrator/internal/health/checker.go (1)

12-48: ⚠️ Potential issue | 🟡 Minor

Remove the unused apps/saga-orchestrator/internal/health package.

The saga-orchestrator already imports and uses libs/health (confirmed in cmd/main.go), making the internal/health directory with its duplicate Checker interface and checker implementations (postgresChecker, redisChecker, tcpChecker) dead code. Delete the entire directory.

🤖 Prompt for AI Agents

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

In `@apps/saga-orchestrator/internal/health/checker.go` around lines 12 - 48,
Delete the entire internal health package and its types/functions: remove the
file(s) defining Checker, postgresChecker, redisChecker, tcpChecker and
constructors NewPostgresChecker, NewRedisChecker, NewKafkaChecker; ensure no
other code imports apps/saga-orchestrator/internal/health (update any references
to use the existing libs/health package instead), run go build/tests to verify
no remaining references, and commit the deleted directory.

apps/saga-orchestrator/internal/repository/postgres_saga_repository.go (1)

102-119: ⚠️ Potential issue | 🟠 Major

Missing timeout wrapper on IsEventProcessed and MarkEventProcessed.

These methods lack the withTimeout wrapper applied to other DB methods. Under database connectivity issues, these queries could block indefinitely, stalling saga processing.

🛡️ Proposed fix

 // IsEventProcessed returns true if this event_id was already handled
 func (r *PostgresSagaRepository) IsEventProcessed(ctx context.Context, eventID string) (bool, error) {
+	ctx, cancel := withTimeout(ctx)
+	defer cancel()
 	var exists bool
 	err := r.pool.QueryRow(ctx, `
         SELECT EXISTS(SELECT 1 FROM saga_processed_events WHERE event_id = $1)
     `, eventID).Scan(&exists)
 	return exists, err
 }

 // MarkEventProcessed records that this event_id has been handled
 func (r *PostgresSagaRepository) MarkEventProcessed(ctx context.Context, eventID string, sagaID uuid.UUID) error {
+	ctx, cancel := withTimeout(ctx)
+	defer cancel()
 	_, err := r.pool.Exec(ctx, `
         INSERT INTO saga_processed_events (event_id, saga_id)
         VALUES ($1, $2)
         ON CONFLICT (event_id) DO NOTHING
     `, eventID, sagaID)
 	return err
 }

🤖 Prompt for AI Agents

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

In `@apps/saga-orchestrator/internal/repository/postgres_saga_repository.go`
around lines 102 - 119, IsEventProcessed and MarkEventProcessed lack the
standard withTimeout DB context wrapper used elsewhere; wrap their DB calls in
r.withTimeout(ctx) to create a cancellable context, use that context for
r.pool.QueryRow and r.pool.Exec respectively, ensure deferred cancel is called,
and return errors/results unchanged so the methods behave like other repository
functions that time out under connectivity issues.

apps/bff/src/lib/circuitBreaker.ts (1)

73-92: ⚠️ Potential issue | 🟠 Major

HALF_OPEN is still local, so recovery probes will stampede across pods.

After the timeout elapses, every pod can set HALF_OPEN locally and call fn() because this transition is neither written to Redis nor guarded by a distributed lock. That breaks the "one request allowed" contract and can hammer a recovering Postgres cluster. Reuse cache.acquireLock() here so exactly one pod runs the probe while the others keep failing fast.

🤖 Prompt for AI Agents

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

In `@apps/bff/src/lib/circuitBreaker.ts` around lines 73 - 92, The
OPEN-to-HALF_OPEN transition is only local, causing stampedes; change the block
in the circuit breaker (where it checks this.state === "OPEN", uses
this.lastFailureTime and this.timeout, and later calls fn()) to acquire a
distributed lock via cache.acquireLock(...) keyed by the circuit name (e.g.,
`lock:${this.name}`) before transitioning or running the probe; if the lock is
not acquired, keep throwing the same OPEN error so other pods fail fast; if the
lock is acquired, run the probe (fn()), then atomically update the shared
circuit state in Redis (set HALF_OPEN/OPEN/closed appropriately) and release the
lock so exactly one pod performs recovery probing.

apps/gateway/src/routes/teamMembers.ts (3)

178-197: ⚠️ Potential issue | 🔴 Critical

Bind invite acceptance to the invited identity.

This lookup only checks the token, then inserts req.user!.sub into tenant_users with the invite's email. Any authenticated user who obtains a valid token can accept someone else’s invite and join that tenant.

---

81-94: ⚠️ Potential issue | 🟠 Major

Don't return invited: true before the Auth0 invite actually succeeds.

The Auth0 org invite is fired-and-forgotten, and the code silently no-ops when auth0_org_id is missing. This can return 201 with invited: true even though no invite email was sent and the user was never provisioned into the org.

Also applies to: 106-111

🤖 Prompt for AI Agents

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

In `@apps/gateway/src/routes/teamMembers.ts` around lines 81 - 94, The current
flow fires inviteToOrg as fire-and-forget and can return invited: true even when
no invite was sent; change the logic around pool.query(...).then(...) to await
or return the inviteToOrg promise so the handler only responds after inviteToOrg
resolves, and when rows[0]?.auth0_org_id is missing or inviteToOrg rejects,
propagate/throw an error (or set invited: false) instead of silently no-op;
update the same pattern used around inviteToOrg at the other occurrence (the
similar block using tenantId/memberRole/email.trim().toLowerCase()) so both
places check rows[0]?.auth0_org_id, call inviteToOrg({orgId:
rows[0].auth0_org_id, email: email.trim().toLowerCase(), role: memberRole,
inviterName: "GrainGuard"}) and await its result, handling and logging errors
and returning an accurate invited flag only after success.

---

73-104: ⚠️ Potential issue | 🟠 Major

Make invite creation and audit persistence atomic.

This handler writes the invite and the audit entry as separate operations. If writeAuditLog fails after the invite insert succeeds, the request returns 500 but leaves a live pending invite behind, so the retry path immediately hits invite_already_pending. As per coding guidelines, apps/gateway/src/**/*.ts: Verify every mutating route uses a real transaction client instead of pool-level BEGIN/COMMIT.

🤖 Prompt for AI Agents

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

In `@apps/gateway/src/routes/teamMembers.ts` around lines 73 - 104, The insert
into tenant_invites is done directly on pool and writeAuditLog is a separate
call, so failures in writeAuditLog leave a pending invite; fix by using a real
DB transaction: acquire a client via pool.connect(), client.query('BEGIN'),
replace pool.query(...) INSERT into tenant_invites with the same transaction
client (referencing inviteId, tenantId, email.trim().toLowerCase(), memberRole),
then call writeAuditLog using that transaction client (add an optional client/db
param to writeAuditLog or perform the audit insert via the same client using the
same eventType/resourceId/meta), commit on success, and rollback + rethrow on
error; keep inviteToOrg as the non-fatal background call and execute it after
the transaction commits, and always release the client in a finally block.

.github/workflows/security.yml (1)

34-39: ⚠️ Potential issue | 🟠 Major

Scan the same image that CD builds.

This step now builds each Dockerfile from apps/${{ matrix.service }}, while .github/workflows/cd.yml still builds the same Dockerfile from repo root. That breaks artifact parity: Trivy can scan a different image than the one you actually ship, and monorepo Dockerfiles that copy shared/root files can fail here while CD still succeeds.

🔧 Align the scan build context with CD

       - name: Build image
         run: |
           docker build \
             -f apps/${{ matrix.service }}/Dockerfile \
             -t grainguard/${{ matrix.service }}:${{ github.sha }} \
-            apps/${{ matrix.service }}
+            .

🤖 Prompt for AI Agents

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

In @.github/workflows/security.yml around lines 34 - 39, The "Build image" step
in .github/workflows/security.yml currently builds each service using apps/${{
matrix.service }} as the build context, which can diverge from the CD build;
update this step to use the exact same docker build command and context as
.github/workflows/cd.yml so Trivy scans the identical image CD will ship (i.e.,
ensure the -f Dockerfile path and the build context match the CD workflow
instead of apps/${{ matrix.service }}), keeping references to the matrix.service
matrix variable and the "Build image" step when editing.

apps/gateway/src/lib/audit.ts (1)

26-39: ⚠️ Potential issue | 🟡 Minor

Remove duplicate event type literals from the union.

The following event types are declared twice in AuditEventType:

• "device.created" (lines 26 and 35)
• "device.creation_failed" (lines 27 and 36)
• "device.registered" (lines 28 and 34)
• "webhook_endpoint.created" (lines 29 and 39)

While TypeScript permits duplicate union members, this creates confusion and maintenance risk.

🧹 Proposed fix to remove duplicates

   | "billing.portal_accessed"
-  | "device.created"
-  | "device.creation_failed"
-  | "device.registered"
-  | "webhook_endpoint.created"
   // API keys
   | "api_key.created"
   | "api_key.revoked"
   // Devices
   | "device.registered"
   | "device.created"
   | "device.creation_failed"
   | "device.deleted"
   // Webhooks
   | "webhook_endpoint.created";

🤖 Prompt for AI Agents

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

In `@apps/gateway/src/lib/audit.ts` around lines 26 - 39, The AuditEventType union
contains duplicated string literals (e.g., "device.created",
"device.creation_failed", "device.registered", "webhook_endpoint.created"); edit
the AuditEventType declaration to remove the repeated entries so each event
string appears only once (locate the AuditEventType union and collapse duplicate
members into a single listing for each unique event literal) and ensure no other
event names are accidentally removed.

apps/gateway/src/server.ts (1)

168-216: ⚠️ Potential issue | 🔴 Critical

Add CSRF protection to the POST /devices endpoint.

This is a mutating route that requires CSRF protection per coding guidelines. The csrfProtection middleware exists but is not applied. Add it to the middleware chain:

app.post(
  "/devices",
  apiRateLimiter,
  authMiddleware,
  csrfProtection(),
  async (req: Request, res: Response) => {

Also apply CSRF protection to the POST /devices route in routes/devices.ts (lines 54-70) unless it is intentionally exempt for non-browser clients.

🤖 Prompt for AI Agents

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

In `@apps/gateway/src/server.ts` around lines 168 - 216, The POST /devices route
is missing CSRF protection; add the csrfProtection middleware into the route's
middleware chain (i.e., include csrfProtection() between authMiddleware and the
async handler in the app.post("/devices", ...) declaration) so the call becomes
app.post("/devices", apiRateLimiter, authMiddleware, csrfProtection(), async
(req: Request, res: Response) => { ... }); Also add the same csrfProtection()
middleware to the POST /devices route in routes/devices.ts (the route handling
lines around the existing handler) unless that route is intentionally exempt for
non-browser clients.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In @.github/workflows/chaos.yml:
- Around line 45-48: Quote the $HOME paths in the kubeconfig setup commands to
avoid word-splitting/globbing (match actionlint SC2086): update the run block so
the mkdir, redirection target and chmod use quoted paths (e.g., mkdir -p
"$HOME/.kube", write to "$HOME/.kube/config", chmod 600 "$HOME/.kube/config") so
the existing commands (mkdir, echo redirection, chmod) operate safely on paths
containing spaces or special characters.
- Line 3: The workflow currently relies on default token permissions under the
top-level "on:" key; add an explicit workflow-level "permissions:" block next to
"on:" that lists only the minimal scopes this workflow needs (e.g., contents:
read, actions: read, id-token: write) instead of using defaults—update the chaos
workflow to declare the exact permissions required by its jobs, removing any
unnecessary scopes so the GITHUB_TOKEN is least-privileged.

In @.github/workflows/e2e.yml:
- Around line 9-12: The Playwright E2E job ("e2e") is running on forked pull
requests where required repository secrets are not available; update the job to
skip when secrets would be missing by adding a conditional that prevents running
for forked PRs (for example, set the job-level if to check that the PR head repo
is not a fork: use github.event.pull_request.head.repo.fork to detect forks) so
the "e2e" job is skipped for forked PRs and only runs when secrets are available
in non-fork contexts.

In @.github/workflows/perf.yml:
- Around line 6-9: The perf workflow never actually exercises the BFF because
the performance-budget.js only runs device/GraphQL checks when JWT is present
and the BFF is listening on port 4000; update the job in perf.yml (both the
initial paths block and duplicate blocks around lines 53-87) to: start the BFF
before the budget step (e.g., run the project's BFF start command as a
background step and ensure it listens on port 4000) and export/set a valid JWT
environment variable (from secrets or a test token) so performance-budget.js
will execute the BFF checks; ensure the new steps run before invoking
performance-budget.js and keep the existing paths filter for apps/bff/** so PRs
touching BFF actually trigger this job.
- Around line 17-38: The workflow's Docker services for Postgres and Redis do
not publish ports to localhost, so the gateway cannot reach them; update the
postgres and redis service definitions (the postgres and redis service blocks)
to include explicit ports mappings (publish container ports 5432 and 6379 to the
host, e.g. host:container 5432:5432 and 6379:6379) for hosted runners, and apply
the same ports mapping for the duplicate redis/service block referenced around
the later section (the second redis/service block at the other services
section).

In @.github/workflows/terraform.yml:
- Around line 65-77: The "Post plan as PR comment" step using
actions/github-script@v7 currently calls github.rest.issues.createComment
without handling failures; wrap the script logic that reads 'plan' and calls
github.rest.issues.createComment in a try/catch so failures are caught, log the
error (including error.message) and either fail the step via rethrow or set an
output flag to handle downstream logic; alternatively, add continue-on-error:
true to the job step and explicitly check the result and log/notify on failure.
Ensure you reference the same identifiers ('plan', 'body', and
github.rest.issues.createComment) so the error handling captures and reports any
permission or API errors and preserves the truncated plan behavior.
- Around line 83-91: The workflow's existing Terraform apply job "apply" only
targets the dev environment; add explicit guidance by either (A) adding a new
job named "apply-prod" modeled on "apply" but with environment: prod, stricter
GitHub environment approvals, and working-directory set to
infra/terraform/environments/prod and an appropriate trigger/if condition for
prod pushes/tags, or (B) add a clear note in the repo docs (e.g.,
infra/terraform/README.md) that production deploys are performed
manually/out-of-band and describe the manual steps and who can approve them;
update the workflow/README referencing the "apply" job to clarify which path
(automated apply-prod vs manual) is used.
- Line 15: YAML lint fails due to an extra space after the colon for the
AWS_REGION key; update the value declaration for AWS_REGION (the "AWS_REGION:"
entry) to remove the extra space after the colon so the line reads a valid YAML
mapping (e.g., ensure it is formatted as AWS_REGION: "us-east-1" with no extra
space between the colon and the value).
- Around line 52-53: The Terraform fmt step "Terraform Format check" currently
runs `terraform fmt -check -recursive` from infra/terraform/environments/${{
matrix.env }}, which misses shared modules in ../../modules; update the workflow
to either add a new step that runs `terraform fmt -check -recursive` from the
repository root infra/terraform/ (e.g., a new job step named "Terraform Format
check - root") or replace the existing step so that the command is executed from
infra/terraform/ instead of the environment directory, ensuring shared modules
are validated as well.

In `@apps/bff/src/datasources/redis.ts`:
- Around line 7-25: getMany() is not cluster-safe because it builds a MULTI/EXEC
(multi().exec()) over arbitrary keys which can cause CROSSSLOT errors in Redis
Cluster; change getMany to issue independent GETs per key (e.g., map keys to
client.get(...) calls) and await them with Promise.all so each GET is routed to
the correct node, preserve the original ordering and null handling of values,
and remove the multi().exec() usage; update any code paths that expect
multi/exec results to instead consume the array of GET responses from getMany().
- Around line 8-14: The Redis cluster node objects created when
REDIS_CLUSTER_NODES is set are currently shaped as { host, port } which is
incompatible with node-redis v4's createCluster; update the nodes mapping in the
block that constructs nodes (the variable nodes used for createCluster) to
produce client config objects with a url property (e.g. `url:
'redis://HOST:PORT'`) so createCluster({ nodes }) receives the expected shape;
ensure you use the same REDIS_CLUSTER_NODES splitting logic and default port
fallback but emit objects like { url: `redis://...` } before calling
createCluster.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Around line 69-71: The execute method currently awaits syncFromRedis on the
hot path, making every protected call subject to Redis latency; change execute
(the async execute<T>(fn: () => Promise<T>) method) to avoid blocking: invoke
syncFromRedis but do not wait indefinitely—either fire-and-forget it or await it
with a very small bounded timeout (e.g., Promise.race pattern with a 20–100ms
timeout) and fall back to local circuit state if the sync times out; ensure the
rest of execute uses the local/shared-in-memory state when syncFromRedis fails
or times out so Redis latency does not delay protected calls.
- Around line 43-50: syncFromRedis currently only adopts shared state when
shared.failureCount > this.failureCount, which ignores newer records that reset
failureCount to 0 or have the same count but later lastFailureTime; change the
shared payload returned by cache.get to include a monotonic updatedAt (or
version) field (e.g., { state: State; failureCount: number; lastFailureTime:
number; updatedAt: number }) and in syncFromRedis compare shared.updatedAt to a
local this.updatedAt (or this.lastFailureTime if you prefer) and accept the
shared record whenever shared.updatedAt > this.updatedAt, updating this.state,
this.failureCount, this.lastFailureTime and this.updatedAt accordingly; also
ensure any code that writes the cache (the code that sets `cb:${this.name}`)
sets the updatedAt/version when updating Redis.

In `@apps/gateway/src/middleware/planEnforcement.ts`:
- Around line 145-147: The middleware requireActiveSubscription currently allows
requests with no req.user?.tenantId to pass; change it to fail closed by
validating tenantId the same way as the helper used at the other sites (the
helper referenced around Line 187/214/249/276) — i.e., immediately reject
requests missing req.user!.tenantId with the same response code/behavior the
helper enforces (401/403), and ensure the middleware performs the same tenantId,
RBAC and CSRF checks flow as that helper before calling next() so protected
routes cannot bypass plan enforcement.
- Around line 228-239: The device-quota check in planEnforcement.ts only
compares the current device count (getDeviceCount(tenantId)) against
limits.devices and ignores how many devices are in the incoming import batch;
update the middleware to compute the incoming batch size (e.g., inspect
req.body.devices?.length or req.body.batchSize used by the import endpoint), add
that to currentCount to form projectedCount = currentCount + incomingCount, and
reject with the same 403 JSON (error: "device_quota_exceeded", plan, limit,
current: projectedCount or include both current and incoming) when
projectedCount > limits.devices; keep the unlimited case (limits.devices === -1)
unchanged and reuse the existing response fields and upgradeUrl.
- Around line 274-280: The requireFeature function currently accepts any keyof
PlanLimits (including numeric fields like devices, alertRules, apiRateLimit)
which can never be false and leads to silent no-ops; narrow the parameter to
only boolean feature flags by introducing a type that extracts keys of
PlanLimits whose values are boolean (e.g., BooleanPlanFlags = { [K in keyof
PlanLimits]: PlanLimits[K] extends boolean ? K : never }[keyof PlanLimits]) and
change the requireFeature signature to accept that type, updating any call
sites; reference symbols: requireFeature, PlanLimits, devices, alertRules,
apiRateLimit.
- Around line 102-121: The quota check is raceable because getDeviceCount reads
a cached/pooled value outside any transaction; fix by performing the quota check
and the insert/update in the same DB transaction using a transaction client (not
pool.query) — e.g., acquire a client via pool.connect()/client.query('BEGIN'),
run a SELECT ... FOR UPDATE on a tenant counter row (or UPDATE tenants SET
device_count = device_count + 1 WHERE id=$1 AND device_count < $limit RETURNING
...) to atomically verify/increment the count, then perform the devices INSERT
and COMMIT; remove or bypass the 60s redis cache for these mutating paths
(getDeviceCount should not be used for enforcement), and ensure every mutating
route that creates devices uses this transaction-client pattern instead of
pool-level BEGIN/COMMIT so concurrent writes cannot exceed the cap.
- Around line 153-176: The defined middleware requireActiveSubscription is never
mounted in server.ts (so it does nothing) and should either be removed or
actually applied to the routes it was intended to protect; decide whether to
import and app.use()/router.use() requireActiveSubscription in server.ts or
delete the unused function to avoid dead code, and ensure its blocked-status
list matches the canonical statuses used elsewhere. Also fix the Stripe status
spelling mismatch in hasPaidAccess (services/planEnforcement.ts) by normalizing
to the expected "canceled" spelling (or ensure both "canceled"/"cancelled" are
handled) so status checks align between requireActiveSubscription,
checkDeviceQuota, and hasPaidAccess.

In `@apps/jobs-worker/src/connection.ts`:
- Around line 22-28: The current conn.on("error") and conn.on("close") handlers
only log and leave stale conn/ch in memory; update these handlers to perform
cleanup and trigger recovery: on "error"/"close" clear any stored channel
variable (ch) and connection reference, cancel or mark existing consumer tags as
invalid, and start a reconnection loop with exponential backoff that
re-establishes conn, recreates the channel, and re-subscribes all consumers
(re-run whatever subscription setup lives in your consumer-init function such as
channel.consume calls or an initConsumers helper). Ensure the reconnection
routine is idempotent (guard against concurrent reconnect attempts) and exposes
a hook or emits an event so higher-level supervisor logic can observe connection
state if present.

In `@apps/read-model-builder/cmd/main.go`:
- Around line 97-104: The Redis UniversalOptions lack a DialTimeout which can
cause cluster topology discovery to hang; update the redis.NewUniversalClient
call to set DialTimeout on redis.UniversalOptions (e.g., provide a reasonable
timeout value or read from config/env via getenvInt) alongside existing
PoolSize/ReadTimeout/WriteTimeout so cluster discovery won't block indefinitely;
locate the redis.NewUniversalClient(...) invocation and add DialTimeout:
<duration> to the options struct.
- Around line 87-91: When parsing REDIS_CLUSTER_NODES into addrs in main.go,
filtered empty entries must be dropped to avoid empty addresses; update the loop
that uses strings.Split(clusterNodes, ",") and strings.TrimSpace(a) to only
append when the trimmed value is non-empty (i.e., if trimmed :=
strings.TrimSpace(a); trimmed != "" { addrs = append(addrs, trimmed) }), leaving
log.Info().Int("nodes", len(addrs)).Msg("Redis cluster mode") unchanged so the
node count reflects only valid addresses.

In `@apps/saga-orchestrator/internal/orchestrator/provision_saga.go`:
- Around line 132-135: The error wrap for the JSON marshal in the quota
allocation branch uses the wrong context "tenant.detach_device"; update the
error message to accurately reflect the operation (e.g., "marshal
quota.allocate_device command") so failures point to quota.allocate_device;
locate the marshal call around cmdBytes, detachErr := json.Marshal(cmd) in
provision_saga.go (quota allocation path) and change the fmt.Errorf call to wrap
detachErr with the correct command name.

In `@apps/saga-orchestrator/internal/repository/postgres_saga_repository.go`:
- Around line 17-23: Add a nolint directive to the withTimeout function to
suppress gosec G118: annotate the withTimeout declaration with a comment like
"//nolint:gosec // G118 false positive: cancel returned to callers" so the
linter knows this is intentional; ensure the justification references that
withTimeout returns a cancel function and callers defer cancel(), and keep the
existing behavior using dbQueryTimeout unchanged.

In `@apps/search-indexer/main.py`:
- Around line 93-96: The Elasticsearch document IDs used in self.es.update for
DEVICE_INDEX currently use device_id and f"{device_id}:{recorded_at or ''}"
which are not tenant-scoped and allow cross-tenant collisions and a collapsing
ID when recorded_at is missing; change the ID scheme to include the tenant
identifier (e.g., prefix with tenant_id) and avoid empty timestamps by either
requiring/validating recorded_at or substituting a non-empty deterministic
placeholder (e.g., a monotonic counter or actual ingestion timestamp) so IDs
become like tenant_id:device_id:recorded_at (or tenant_id:device_id:<fallback>)
wherever device_id and recorded_at are used to build document IDs.

In `@apps/telemetry-service/migrations/000009_sso_alert_rules_bulk.up.sql`:
- Around line 18-33: Enable row-level security for the new tenant-owned tables
and add tenant-scoped policies: run ALTER TABLE ... ENABLE ROW LEVEL SECURITY
for both alert_rules and bulk_import_jobs, then create policies (e.g., CREATE
POLICY tenant_isolation ON alert_rules FOR ALL USING (tenant_id =
current_setting('request.jwt.claims.tenant_id')::uuid) WITH CHECK (tenant_id =
current_setting('request.jwt.claims.tenant_id')::uuid)) and equivalent policies
for bulk_import_jobs; also ensure any service/loader roles that need to bypass
checks are granted appropriate role-based policies or use a SECURITY DEFINER
function to set tenant_id on insert.

In `@infra/kafka/mirrormaker2-connector.json`:
- Around line 12-14: Add producer-side compression to the MirrorMaker2 connector
to reduce cross-region bandwidth; update the connector JSON to include keys such
as "producer.compression.type" (e.g., "lz4" or "snappy") and optionally
"producer.compression.level" or "producer.batch.size" to tune throughput,
placing them alongside the existing "topics" and "replication.factor" entries so
MirrorMaker2 uses compression for replicated topics.
- Around line 22-28: The connector currently injects the same credentials into
source.cluster.sasl.jaas.config and target.cluster.sasl.jaas.config using
${KAFKA_USERNAME}/${KAFKA_PASSWORD}; change these to use separate environment
variables (e.g. ${KAFKA_SOURCE_USERNAME}, ${KAFKA_SOURCE_PASSWORD} for
source.cluster.sasl.jaas.config and ${KAFKA_TARGET_USERNAME},
${KAFKA_TARGET_PASSWORD} for target.cluster.sasl.jaas.config), update any
deployment manifests/secrets to provide the new vars, and ensure any
documentation or secret provisioning scripts are updated to reflect the distinct
credentials for source and target clusters.

In `@infra/terraform/backend.tf`:
- Around line 26-34: Add a brief clarifying comment above the terraform backend
block explaining that this root-level backend (backend "s3" with key
"grainguard/terraform.tfstate") stores state for root-module resources, while
environment-specific backends use separate keys (e.g.,
"grainguard/prod/terraform.tfstate", "grainguard/dr/terraform.tfstate") so
states are isolated and do not conflict; reference the bucket, key, region and
dynamodb_table settings (bucket, key, region, dynamodb_table) in the comment to
make the relationship clear.

In `@infra/terraform/environments/dr/main.tf`:
- Around line 67-69: Rename or clarify the Redis output to avoid implying global
primary; change the output name from dr_redis_primary_endpoint to
dr_redis_endpoint (or keep the existing name and add a clear comment above
output "dr_redis_primary_endpoint" explaining it refers to the DR cluster's
primary node within the DR cluster, not the global primary). Update any
references to module outputs that consume dr_redis_primary_endpoint to use the
new dr_redis_endpoint identifier if you rename it.
- Around line 5-9: The variable declarations use non-idiomatic semicolon syntax
and one variable (aws_region) is unused; replace each declaration with standard
Terraform variable block syntax for aurora_global_cluster_id,
redis_global_datastore_id, db_password (keep sensitive = true), and project
(e.g., variable "name" { type = string [; default = ...] -> variable "name" {
type = string [default = "..."] }), remove the semicolons, and either remove
aws_region or reference it from your AWS provider configuration (provider "aws"
{ region = var.aws_region }) so the variable is actually used.

In `@infra/terraform/environments/dr/providers.tf`:
- Around line 11-17: State is currently stored in the S3 backend block (backend
"s3", bucket "grainguard-terraform-state", key
"grainguard/dr/terraform.tfstate", region "us-east-1", dynamodb_table
"grainguard-terraform-locks") which makes DR operations unavailable if the
primary region fails; enable cross-region protections by configuring S3
Cross-Region Replication for that bucket to a DR-region bucket (or create a
secondary state bucket in the DR region), ensure the DynamoDB lock mechanism is
resilient (use a global/replicated locks table or a secondary locks table in the
DR region), and add a documented failover procedure in runbooks describing how
to switch the Terraform backend to the DR bucket (update the backend
configuration to the DR bucket/key and locks table) during a primary-region
outage.

In `@infra/terraform/environments/prod/main.tf`:
- Around line 27-28: The VPC CIDR "10.1.0.0/16" is hardcoded in multiple module
calls causing duplication; add a locals block (e.g., locals { vpc_cidr =
"10.1.0.0/16" }) near the top of main.tf and replace all hardcoded CIDR
occurrences with references to local.vpc_cidr in the modules that currently use
the literal (the module blocks that use vpc_id, redis, and msk), ensuring each
module argument that accepted the string now uses local.vpc_cidr.

In `@infra/terraform/environments/prod/variables.tf`:
- Around line 1-3: The variables in variables.tf (variable "project", variable
"aws_region", variable "db_password") lack description attributes; add a
descriptive description = "..." for each variable to improve terraform plan
output and generated docs (e.g., project name, AWS region, and database
password), keeping db_password sensitive = true and its type unchanged; update
the variable blocks for project, aws_region, and db_password to include clear,
concise descriptions.

In `@infra/terraform/modules/aurora-global/main.tf`:
- Around line 54-63: The aws_rds_global_cluster resource
(aws_rds_global_cluster.this) hardcodes database_name = "grainguard"; make it
configurable by adding a module variable (e.g., var.database_name with a
sensible default "grainguard") and update the resource to use database_name =
var.database_name so the module can be reused for other DB names.
- Around line 18-19: Add a validation that enforces global_cluster_id is
non-empty when is_secondary is true: create a top-level check block (Terraform
1.5+) named e.g. "require_global_cluster_id_when_secondary" that uses condition
= !(var.is_secondary && var.global_cluster_id == "") and a clear error_message
like "global_cluster_id must be set when is_secondary = true"; reference the
existing variables is_secondary and global_cluster_id so apply-time errors are
caught early.
- Around line 83-84: The cluster protection logic currently only treats "prod"
specially; update the settings so the DR environment is also protected: change
the expressions for skip_final_snapshot and deletion_protection (referencing
skip_final_snapshot, deletion_protection and var.environment in this module) to
treat "dr" like "prod" (e.g., skip_final_snapshot = !(var.environment == "prod"
|| var.environment == "dr") and deletion_protection = var.environment == "prod"
|| var.environment == "dr"), or alternatively introduce a new boolean variable
(e.g., var.enable_deletion_protection) and use it to drive both
skip_final_snapshot and deletion_protection so DR can be explicitly enabled
without relying on environment name.

In `@infra/terraform/modules/elasticache-global/main.tf`:
- Around line 9-16: The variable block uses semicolons for attribute separators
(e.g., in variable "node_type" and "is_secondary") which is non-idiomatic HCL;
update each variable declaration (variable "project", "environment", "vpc_id",
"vpc_cidr", "private_subnet_ids", "node_type", "is_secondary",
"global_datastore_id") to use = for attributes and place attributes on separate
lines (for example: type = string and default = "..." on their own lines) so the
declarations follow idiomatic Terraform/HCL2 style and improve readability.

In `@Makefile`:
- Around line 117-118: Replace the current lint command that short-circuits
failures ("golangci-lint run ./... || echo ...") with logic that still prints
the helpful install hint but returns a non-zero exit on real lint failures:
first check for the golangci-lint binary (e.g., test -x "$(command -v
golangci-lint)") and print the install hint if missing, otherwise run
"golangci-lint run ./..." and let its exit code propagate so CI fails on lint
errors; update the Makefile target that currently echoes "=== Go lint ===" to
use this check-run pattern so missing binary and lint failures are handled
distinctly and lint failures fail the build.

In `@scripts/load-tests/performance-budget.js`:
- Around line 27-38: The thresholds currently rely on the "error_rate" metric
but `/health` failures (500/503) aren't contributing to that metric, so update
the load test where the `/health` request is made to explicitly increment the
same metric used by the threshold (named "error_rate") and the test's total
error counter (e.g., "totalErrors") whenever the `/health` response status is
>=500; ensure the metric names match the ones in the exported options object so
those failures are counted by the existing thresholds.

In `@tests/chaos/kafka-consumer-pause.sh`:
- Line 19: Remove the unused TOPIC variable declaration
(TOPIC="telemetry.events") from the kafka-consumer-pause.sh script; locate the
TOPIC symbol in the script and delete its assignment so ShellCheck SC2034 is
resolved and no unused variables remain.
- Around line 85-100: The timeout deadline is computed once (deadline) before
iterating CONSUMERS, so the second consumer may get less than 5 minutes; move
the deadline calculation inside the outer for loop so each consumer gets a fresh
deadline (compute deadline=$(( $(date +%s) + 300 ))) at the start of the loop
that iterates over CONSUMERS and keep the inner while logic (current_lag,
MAX_LAG check, sleep, fail) unchanged so each consumer has its own 5-minute
recovery window.
- Around line 54-80: The script pauses consumers by iterating over the CONSUMERS
array and calling scale_consumer, but it lacks a cleanup trap so a failure can
leave deployments scaled to 0; add a trap before the "Pausing consumers" loop
that registers a handler (e.g., restore_consumers or inline handler) to iterate
CONSUMERS and call scale_consumer "$consumer" 1 (and optionally wait with
kubectl rollout status) on EXIT/ERR, and remove or disable that trap after
successful resume so normal completion doesn't re-run it; reference the existing
scale_consumer function, CONSUMERS array, and the resume loop that uses kubectl
rollout status to mirror expected restore behavior.

In `@tests/chaos/network-partition.yaml`:
- Around line 76-80: The NetworkPolicy's egress currently allows only UDP/53
(DNS) which blocks all other outbound traffic for telemetry-service; update the
egress rules in the telemetry-service NetworkPolicy so they allow DNS plus only
the Kafka broker ports instead of everything—specifically add TCP port entries
for your Kafka listeners (e.g., 9092 and/or 29092 as used in your cluster)
alongside the existing UDP/53 rule, and scope those Kafka rules to the Kafka
podSelector/IPBlock as appropriate so only Kafka traffic is permitted while
other egress remains denied.
- Around line 60-64: Remove the invalid --stdin flag from the kubectl apply
argument list: locate the kubectl command args sequence that currently reads
"apply", "-f", "-", "--stdin" (followed by the stdin: | block) and delete the
"--stdin" token so the command uses "apply -f -" to read from stdin correctly;
leave the existing stdin: | content intact.

In `@tests/chaos/pod-kill.yaml`:
- Around line 65-79: The current chaos action named "kill-gateway-pod" (and the
similar entries for other services) runs kubectl delete pod -l app=... which
deletes all matching pods; change the action to target a single pod by first
resolving one pod name via kubectl get pods -l app=... -n "${namespace}" (e.g.,
take the first item) and then run kubectl delete pod <that-pod-name> -n
"${namespace}" --grace-period=0 --force; update the provider block for the
"kill-gateway-pod" action and the other similar action blocks so they resolve
and delete a single pod instead of deleting by label selector.

In `@tests/chaos/projection-lag.sh`:
- Around line 78-80: The script currently treats the ProjectionLagHigh check as
non-blocking by using the warn helper when (( alert_fired )) is false; change
this to a hard failure so the experiment blocks CI when the alert never fires:
replace the non-blocking warn invocation that references ALERT_WINDOW and
ProjectionLagHigh with a failing path (call the project's fail/error helper or
exit with a non-zero status) so that the absence of the ProjectionLagHigh alert
causes the job to fail rather than just warn.
- Around line 57-87: After scaling read-model-builder to 0 (after the kubectl
scale deployment/read-model-builder -n "$NAMESPACE" --replicas=0 line), add a
rollback trap that always restores the deployment to 1 replica on EXIT (or on
ERR) to avoid leaving the service down; implement the trap to call the same
restore commands (kubectl scale ... --replicas=1 and kubectl rollout status ...)
and ensure it runs regardless of failures during the alert-wait loop
(references: NAMESPACE, current_lag, alert_firing, and the restore commands used
later in the script).

In `@tests/chaos/README.md`:
- Around line 16-20: The experiment descriptions overstate guarantees; update
the README lines for pod-kill.yaml, kafka-consumer-pause.sh, redis-outage.sh,
projection-lag.sh and network-partition.yaml to reflect what the scripts
actually assert (rollout/hpa trigger and readiness gating, consumer lag
thresholds after resume rather than absolute “no messages lost”, BFF fallback
behavior as observed in tests and saga retry/backoff behavior as validated, lag
alert and catch-up within measured windows, and producer buffering/delivery
behavior as observed) — replace prescriptive claims like “HPA respawns within
30s” and “no messages lost” with conservative phrasing such as “validates
HPA-triggered rollout and readiness gating within expected window” and
“validates consumer lag ≤ X after resume (does not assert zero message loss)”,
and similarly tighten the other experiment descriptions to match the actual
asserted metrics and checks performed by the scripts.

In `@tests/chaos/redis-outage.sh`:
- Around line 26-40: In http_check(), currently only the HTTP status is
evaluated; update it to capture both the response body and the status (still
using curl with -w "%{http_code}") and then validate the GraphQL payload: ensure
the JSON contains a non-null "data" field and does NOT contain an "errors" field
before treating the probe as success. Use the existing symbols (http_check,
GATEWAY_URL, GRAPHQL_QUERY, TEST_JWT) to locate the function, parse the response
body with a JSON-safe check (jq or equivalent) to detect errors, and only log a
success when status is 200 and the payload has no errors; otherwise warn and
return 1 as currently done.
- Line 85: The current sleep invocation sleep "$(( OUTAGE_SECONDS - 15 ))" can
be passed a negative value when OUTAGE_SECONDS < 15; change the logic in
tests/chaos/redis-outage.sh to clamp or guard the delay before sleeping (e.g.,
compute a non-negative DELAY from OUTAGE_SECONDS - 15 or use an if ((
OUTAGE_SECONDS > 15 )) check) and only call sleep with that non-negative value
to avoid passing negatives to sleep.
- Around line 53-92: Add an EXIT trap that always restores Redis replicas if the
script exits unexpectedly: capture the target deployment name ($REDIS_DEPLOY)
and namespace ($NAMESPACE) and define a restore function (e.g., restore_redis)
that runs kubectl scale deployment "$REDIS_DEPLOY" -n "$NAMESPACE" --replicas=1
and optionally kubectl rollout status; register it with trap 'restore_redis'
EXIT before you scale to 0 (in the block around REDIS scaling and outage checks
using http_check, fail, log and OUTAGE_SECONDS), and ensure the trap is removed
or the function no-ops after successful restore to avoid double-restores.

In `@tests/chaos/run-all.sh`:
- Line 12: The color constant YELLOW defined in the color line
(YELLOW='\033[1;33m') is unused and triggers ShellCheck SC2034; remove the
YELLOW token from the constants assignment (the line defining RED, GREEN,
YELLOW, BOLD, NC) or alternatively use YELLOW where appropriate—e.g., delete the
YELLOW segment so the line only declares RED, GREEN, BOLD, and NC to eliminate
the unused-variable warning.

In `@tests/e2e/billing.spec.ts`:
- Around line 18-21: The test "shows plan prices" hardcodes "$49/mo" and
"$199/mo" which makes it fragile; update the test (the test block named "shows
plan prices" that uses page.getByText) to use either shared constants (e.g.,
PLANS.PRICE_BASIC, PLANS.PRICE_PRO) imported from a fixture/config or a flexible
matcher such as a regex/currency pattern (e.g., match /\$\d+\/mo/ or specific
plan labels combined with a number pattern) so price changes don't break the
assertions; prefer referencing stable identifiers (plan IDs or data-test
attributes) and assert the price format rather than exact literal strings.

In `@tests/e2e/devices.spec.ts`:
- Around line 30-35: The test "modal closes on backdrop click" uses a fixed
coordinate mouse.click(10, 10) which is brittle; change it to compute the dialog
element and click outside its bounds (or target a backdrop element if one
exists). Locate the dialog via page.getByRole("dialog") (assign to a variable
like dialog), call dialog.boundingBox() and, if non-null, invoke
page.mouse.click at coordinates just outside box.x/box.y (e.g., box.x - 10,
box.y - 10) or alternatively click the backdrop selector if available, then
assert the dialog is not visible.

In `@tests/e2e/fixtures/mockAuth.ts`:
- Around line 93-124: The current page.route GraphQL handler returns a silent
success for unknown operations which masks unmocked queries; instead, update the
default branch in the page.route callback (the anonymous handler using
route.request().postDataJSON()) to fail loudly: detect when the incoming query
string doesn't match the known handlers for "devices"/"Devices" or "me"/"tenant"
and respond with a non-2xx route.fulfill (e.g., status 400 or 500) and a JSON
body that includes an error message and the raw query (or alternatively call
route.abort()) so unmocked operations surface failures during tests; keep
existing mocked responses for MOCK_DEVICES and the me/tenant payload intact.

---

Outside diff comments:
In @.github/workflows/security.yml:
- Around line 34-39: The "Build image" step in .github/workflows/security.yml
currently builds each service using apps/${{ matrix.service }} as the build
context, which can diverge from the CD build; update this step to use the exact
same docker build command and context as .github/workflows/cd.yml so Trivy scans
the identical image CD will ship (i.e., ensure the -f Dockerfile path and the
build context match the CD workflow instead of apps/${{ matrix.service }}),
keeping references to the matrix.service matrix variable and the "Build image"
step when editing.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Around line 73-92: The OPEN-to-HALF_OPEN transition is only local, causing
stampedes; change the block in the circuit breaker (where it checks this.state
=== "OPEN", uses this.lastFailureTime and this.timeout, and later calls fn()) to
acquire a distributed lock via cache.acquireLock(...) keyed by the circuit name
(e.g., `lock:${this.name}`) before transitioning or running the probe; if the
lock is not acquired, keep throwing the same OPEN error so other pods fail fast;
if the lock is acquired, run the probe (fn()), then atomically update the shared
circuit state in Redis (set HALF_OPEN/OPEN/closed appropriately) and release the
lock so exactly one pod performs recovery probing.

In `@apps/gateway/src/lib/audit.ts`:
- Around line 26-39: The AuditEventType union contains duplicated string
literals (e.g., "device.created", "device.creation_failed", "device.registered",
"webhook_endpoint.created"); edit the AuditEventType declaration to remove the
repeated entries so each event string appears only once (locate the
AuditEventType union and collapse duplicate members into a single listing for
each unique event literal) and ensure no other event names are accidentally
removed.

In `@apps/gateway/src/routes/teamMembers.ts`:
- Around line 81-94: The current flow fires inviteToOrg as fire-and-forget and
can return invited: true even when no invite was sent; change the logic around
pool.query(...).then(...) to await or return the inviteToOrg promise so the
handler only responds after inviteToOrg resolves, and when rows[0]?.auth0_org_id
is missing or inviteToOrg rejects, propagate/throw an error (or set invited:
false) instead of silently no-op; update the same pattern used around
inviteToOrg at the other occurrence (the similar block using
tenantId/memberRole/email.trim().toLowerCase()) so both places check
rows[0]?.auth0_org_id, call inviteToOrg({orgId: rows[0].auth0_org_id, email:
email.trim().toLowerCase(), role: memberRole, inviterName: "GrainGuard"}) and
await its result, handling and logging errors and returning an accurate invited
flag only after success.
- Around line 73-104: The insert into tenant_invites is done directly on pool
and writeAuditLog is a separate call, so failures in writeAuditLog leave a
pending invite; fix by using a real DB transaction: acquire a client via
pool.connect(), client.query('BEGIN'), replace pool.query(...) INSERT into
tenant_invites with the same transaction client (referencing inviteId, tenantId,
email.trim().toLowerCase(), memberRole), then call writeAuditLog using that
transaction client (add an optional client/db param to writeAuditLog or perform
the audit insert via the same client using the same eventType/resourceId/meta),
commit on success, and rollback + rethrow on error; keep inviteToOrg as the
non-fatal background call and execute it after the transaction commits, and
always release the client in a finally block.

In `@apps/gateway/src/server.ts`:
- Around line 168-216: The POST /devices route is missing CSRF protection; add
the csrfProtection middleware into the route's middleware chain (i.e., include
csrfProtection() between authMiddleware and the async handler in the
app.post("/devices", ...) declaration) so the call becomes app.post("/devices",
apiRateLimiter, authMiddleware, csrfProtection(), async (req: Request, res:
Response) => { ... }); Also add the same csrfProtection() middleware to the POST
/devices route in routes/devices.ts (the route handling lines around the
existing handler) unless that route is intentionally exempt for non-browser
clients.

In `@apps/read-model-builder/internal/projection/telemetry_projection.go`:
- Around line 311-318: After iterating and calling rows.Close(), check
rows.Err() and handle any iteration error instead of proceeding with the partial
newEventIDs map; specifically, after the existing rows.Close() call invoke
rows.Err(), and if it returns a non-nil error return or propagate that error (or
log and abort) so the code that builds newEventIDs (using rows.Next and
rows.Scan) does not silently continue on an incomplete result set.

In `@apps/saga-orchestrator/internal/health/checker.go`:
- Around line 12-48: Delete the entire internal health package and its
types/functions: remove the file(s) defining Checker, postgresChecker,
redisChecker, tcpChecker and constructors NewPostgresChecker, NewRedisChecker,
NewKafkaChecker; ensure no other code imports
apps/saga-orchestrator/internal/health (update any references to use the
existing libs/health package instead), run go build/tests to verify no remaining
references, and commit the deleted directory.

In `@apps/saga-orchestrator/internal/repository/postgres_saga_repository.go`:
- Around line 102-119: IsEventProcessed and MarkEventProcessed lack the standard
withTimeout DB context wrapper used elsewhere; wrap their DB calls in
r.withTimeout(ctx) to create a cancellable context, use that context for
r.pool.QueryRow and r.pool.Exec respectively, ensure deferred cancel is called,
and return errors/results unchanged so the methods behave like other repository
functions that time out under connectivity issues.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6eb7d842-24db-4274-b9e9-b10368a639f2

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0aeb8 and 4a25f89.

⛔ Files ignored due to path filters (3)

• apps/bff/package-lock.json is excluded by !**/package-lock.json
• apps/gateway/package-lock.json is excluded by !**/package-lock.json
• apps/jobs-worker/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (53)

• .github/workflows/cd.yml
• .github/workflows/chaos.yml
• .github/workflows/e2e.yml
• .github/workflows/perf.yml
• .github/workflows/security.yml
• .github/workflows/terraform.yml
• Makefile
• apps/bff/package.json
• apps/bff/src/datasources/redis.ts
• apps/bff/src/lib/circuitBreaker.ts
• apps/gateway/package.json
• apps/gateway/src/lib/audit.ts
• apps/gateway/src/middleware/planEnforcement.ts
• apps/gateway/src/routes/teamMembers.ts
• apps/gateway/src/server.ts
• apps/gateway/src/services/device.ts
• apps/jobs-worker/src/connection.ts
• apps/read-model-builder/cmd/main.go
• apps/read-model-builder/internal/consumer/message_handler.go
• apps/read-model-builder/internal/projection/device_projection.go
• apps/read-model-builder/internal/projection/telemetry_projection.go
• apps/saga-orchestrator/internal/domain/saga.go
• apps/saga-orchestrator/internal/health/checker.go
• apps/saga-orchestrator/internal/orchestrator/provision_saga.go
• apps/saga-orchestrator/internal/repository/postgres_saga_repository.go
• apps/search-indexer/main.py
• apps/telemetry-service/migrations/000007_saas_columns.down.sql
• apps/telemetry-service/migrations/000007_saas_columns.up.sql
• apps/telemetry-service/migrations/000009_sso_alert_rules_bulk.down.sql
• apps/telemetry-service/migrations/000009_sso_alert_rules_bulk.up.sql
• infra/kafka/mirrormaker2-connector.json
• infra/terraform/backend.tf
• infra/terraform/environments/dr/main.tf
• infra/terraform/environments/dr/providers.tf
• infra/terraform/environments/prod/main.tf
• infra/terraform/environments/prod/providers.tf
• infra/terraform/environments/prod/variables.tf
• infra/terraform/modules/aurora-global/main.tf
• infra/terraform/modules/elasticache-global/main.tf
• libs/health/health.go
• scripts/load-tests/performance-budget.js
• tests/chaos/README.md
• tests/chaos/kafka-consumer-pause.sh
• tests/chaos/network-partition.yaml
• tests/chaos/pod-kill.yaml
• tests/chaos/projection-lag.sh
• tests/chaos/redis-outage.sh
• tests/chaos/run-all.sh
• tests/e2e/auth.spec.ts
• tests/e2e/billing.spec.ts
• tests/e2e/devices.spec.ts
• tests/e2e/fixtures/mockAuth.ts
• tests/e2e/playwright.config.ts

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider adding error handling for the PR comment step.

If posting the comment fails (e.g., permission issues), the workflow continues silently. Consider wrapping with error handling or using continue-on-error with explicit status checking.

Also, the truncation at 65000 characters is good for GitHub's comment size limits.

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 65 - 77, The "Post plan as PR
comment" step using actions/github-script@v7 currently calls
github.rest.issues.createComment without handling failures; wrap the script
logic that reads 'plan' and calls github.rest.issues.createComment in a
try/catch so failures are caught, log the error (including error.message) and
either fail the step via rethrow or set an output flag to handle downstream
logic; alternatively, add continue-on-error: true to the job step and explicitly
check the result and log/notify on failure. Ensure you reference the same
identifiers ('plan', 'body', and github.rest.issues.createComment) so the error
handling captures and reports any permission or API errors and preserves the
truncated plan behavior.

[coderabbitai]

Review continued from previous batch...

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find .github -name "e2e.yml" -o -name "e2e.yaml" | head -20

Repository: pahuldeepp/GrainGuard-

Length of output: 91

---

🏁 Script executed:

cat -n .github/workflows/e2e.yml

Repository: pahuldeepp/GrainGuard-

Length of output: 2840

---

Skip this job when required secrets are missing.

This job runs on every pull_request, but the build and test steps depend on repository secrets (Auth0 domain, client ID, audience, BFF URL, gateway URL). In fork PRs, these secrets are unavailable and resolve to empty strings, causing the workflow to fail noisily instead of gracefully skipping.

💡 Suggested fix

 jobs:
   e2e:
     name: Playwright E2E
     runs-on: ubuntu-latest
     timeout-minutes: 20
+    if: ${{ secrets.VITE_AUTH0_DOMAIN != '' && secrets.VITE_AUTH0_CLIENT_ID != '' && secrets.VITE_AUTH0_AUDIENCE != '' && secrets.E2E_BFF_URL != '' && secrets.E2E_GATEWAY_URL != '' }}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	e2e:
	name: Playwright E2E
	runs-on: ubuntu-latest
	timeout-minutes: 20
	e2e:
	name: Playwright E2E
	runs-on: ubuntu-latest
	timeout-minutes: 20
	if: ${{ secrets.VITE_AUTH0_DOMAIN != '' && secrets.VITE_AUTH0_CLIENT_ID != '' && secrets.VITE_AUTH0_AUDIENCE != '' && secrets.E2E_BFF_URL != '' && secrets.E2E_GATEWAY_URL != '' }}

🤖 Prompt for AI Agents

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

In @.github/workflows/e2e.yml around lines 9 - 12, The Playwright E2E job
("e2e") is running on forked pull requests where required repository secrets are
not available; update the job to skip when secrets would be missing by adding a
conditional that prevents running for forked PRs (for example, set the job-level
if to check that the PR head repo is not a fork: use
github.event.pull_request.head.repo.fork to detect forks) so the "e2e" job is
skipped for forked PRs and only runs when secrets are available in non-fork
contexts.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

These quota checks are still raceable under concurrent writes.

Both quotas read the current count before the write happens, and device counts are additionally cached for 60 seconds. Two parallel creates can pass against the same pre-insert value, so the cap needs to be enforced in the same transaction/counter update as the write.

As per coding guidelines, Verify every mutating route uses a real transaction client instead of pool-level BEGIN/COMMIT.

Also applies to: 193-195, 255-257

🤖 Prompt for AI Agents

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

In `@apps/gateway/src/middleware/planEnforcement.ts` around lines 102 - 121, The
quota check is raceable because getDeviceCount reads a cached/pooled value
outside any transaction; fix by performing the quota check and the insert/update
in the same DB transaction using a transaction client (not pool.query) — e.g.,
acquire a client via pool.connect()/client.query('BEGIN'), run a SELECT ... FOR
UPDATE on a tenant counter row (or UPDATE tenants SET device_count =
device_count + 1 WHERE id=$1 AND device_count < $limit RETURNING ...) to
atomically verify/increment the count, then perform the devices INSERT and
COMMIT; remove or bypass the 60s redis cache for these mutating paths
(getDeviceCount should not be used for enforcement), and ensure every mutating
route that creates devices uses this transaction-client pattern instead of
pool-level BEGIN/COMMIT so concurrent writes cannot exceed the cap.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Backdrop click uses fixed coordinates that may be fragile.

page.mouse.click(10, 10) assumes the backdrop is at absolute position (10, 10). This could break if the layout changes. Consider clicking outside the dialog bounds relative to the dialog's position, or using a backdrop element selector if available.

💡 Alternative approach using dialog bounds

test("modal closes on backdrop click", async ({ page }) => {
  await page.getByRole("button", { name: "+ Register Device" }).click();
  const dialog = page.getByRole("dialog");
  await expect(dialog).toBeVisible();
  // Click outside dialog bounds
  const box = await dialog.boundingBox();
  if (box) {
    await page.mouse.click(box.x - 10, box.y - 10);
  }
  await expect(dialog).not.toBeVisible();
});

🤖 Prompt for AI Agents

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

In `@tests/e2e/devices.spec.ts` around lines 30 - 35, The test "modal closes on
backdrop click" uses a fixed coordinate mouse.click(10, 10) which is brittle;
change it to compute the dialog element and click outside its bounds (or target
a backdrop element if one exists). Locate the dialog via
page.getByRole("dialog") (assign to a variable like dialog), call
dialog.boundingBox() and, if non-null, invoke page.mouse.click at coordinates
just outside box.x/box.y (e.g., box.x - 10, box.y - 10) or alternatively click
the backdrop selector if available, then assert the dialog is not visible.

[coderabbitai]

Actionable comments posted: 13

♻️ Duplicate comments (4)

apps/bff/src/datasources/redis.ts (1)

41-47: ⚠️ Potential issue | 🟠 Major

getMany() remains cluster-unsafe — outstanding fix from prior review.

The multi().exec() pipeline batches arbitrary keys that will hash to different slots in Redis Cluster, causing CROSSSLOT errors at runtime. Replace with independent GET calls:

 async getMany<T>(keys: string[]): Promise<(T | null)[]> {
   if (keys.length === 0) return [];
-  const pipeline = client.multi();
-  keys.forEach(key => pipeline.get(key));
-  const results = await pipeline.exec() as (string | null)[];
+  const results = await Promise.all(keys.map((key) => client.get(key)));
   return results.map((r) => (r ? JSON.parse(r) as T : null));
 },

🤖 Prompt for AI Agents

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

In `@apps/bff/src/datasources/redis.ts` around lines 41 - 47, getMany currently
uses client.multi() / pipeline.exec() which can produce CROSSSLOT errors in
Redis Cluster; change getMany to perform cluster-safe independent GETs (e.g.,
Promise.all over client.get(key) for each key) rather than batching with
client.multi()/pipeline.exec(), then parse each result with JSON.parse as
before; update the function getMany to remove pipeline usage and use per-key
client.get calls to ensure cluster safety.

.github/workflows/terraform.yml (1)

15-15: ⚠️ Potential issue | 🟡 Minor

Fix extra space after colon.

YAMLlint flags the extra space. This is a minor formatting issue.

🔧 Proposed fix

-  AWS_REGION:  "us-east-1"
+  AWS_REGION: "us-east-1"

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml at line 15, The AWS_REGION key in the GitHub
Actions workflow has an extra space after the colon ("AWS_REGION:  "us-east-1")
which YAML linter flags; update the workflow to remove the extra space so the
entry reads AWS_REGION: "us-east-1" (locate the AWS_REGION line in the
terraform.yml file and correct the spacing).

infra/terraform/modules/aurora-global/main.tf (2)

34-37: ⚠️ Potential issue | 🟠 Major

Add validation to enforce global_cluster_id when is_secondary=true.

The comment states global_cluster_id is required when is_secondary=true, but there's no validation. An empty string will cause a confusing error at apply time when the secondary cluster tries to join a non-existent global cluster.

🛡️ Proposed fix using a check block (Terraform 1.5+)

check "require_global_cluster_id_when_secondary" {
  assert {
    condition     = !(var.is_secondary && var.global_cluster_id == "")
    error_message = "global_cluster_id must be set when is_secondary = true"
  }
}

Alternatively, use a validation block within the variable (requires reordering variables or using locals):

 variable "global_cluster_id" {
   type        = string
   default     = ""
+  description = "Global cluster ID from primary region. Required when is_secondary=true."
 }
+
+check "require_global_cluster_id_when_secondary" {
+  assert {
+    condition     = !(var.is_secondary && var.global_cluster_id == "")
+    error_message = "global_cluster_id must be set when is_secondary = true"
+  }
+}

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/aurora-global/main.tf` around lines 34 - 37, Add
validation to prevent an empty global_cluster_id when the module is configured
as a secondary: add a check block (e.g., name it
require_global_cluster_id_when_secondary) that asserts !(var.is_secondary &&
var.global_cluster_id == "") and supplies a clear error_message like
"global_cluster_id must be set when is_secondary = true"; this ensures the
variable "global_cluster_id" and the conditional "var.is_secondary" are
validated at plan time rather than failing later at apply.

---

101-102: ⚠️ Potential issue | 🟠 Major

DR cluster lacks final snapshot and deletion protection.

The logic sets skip_final_snapshot = true and deletion_protection = false for DR ("dr" != "prod"). During failover, the DR cluster becomes the primary data source—losing it without a snapshot could be catastrophic.

🔒 Proposed fix to protect DR cluster

-  skip_final_snapshot = var.environment != "prod"   # keep snapshot in prod
-  deletion_protection = var.environment == "prod"
+  skip_final_snapshot = !contains(["prod", "dr"], var.environment)   # keep snapshot in prod and dr
+  deletion_protection = contains(["prod", "dr"], var.environment)

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/aurora-global/main.tf` around lines 101 - 102, The
current conditions use skip_final_snapshot = var.environment != "prod" and
deletion_protection = var.environment == "prod", which leaves the "dr"
environment unprotected; update the logic so the DR environment is treated like
prod for these protections (i.e., ensure skip_final_snapshot is false and
deletion_protection is true when var.environment == "dr" or "prod"). Change the
boolean expressions around skip_final_snapshot and deletion_protection to
include var.environment == "dr" alongside "prod" (e.g., treat both "prod" and
"dr" as protected environments) so the DR cluster keeps a final snapshot and has
deletion protection.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In @.github/workflows/perf.yml:
- Around line 57-59: The workflow starts the gateway and BFF with backgrounding
("Start gateway in background" and the analogous "Start bff in background") but
doesn't clean them up; update the steps to capture their PIDs (run the commands
with & and write $! to gateway.pid/bff.pid) and add a subsequent cleanup step
that reads those files and kills the processes (or use pkill with the
node/ts-node command), or alternatively switch these steps to launch via a
process manager like pm2 and stop the processes in a teardown step; ensure the
cleanup step runs even on failure by placing it in a finally/always-run step so
background processes are terminated before the job finishes.
- Around line 108-110: The perf workflow is missing a Cassandra service so the
BFF's deviceTelemetry endpoint (which calls getTelemetryHistoryFromCassandra())
crashes on startup; either add a Cassandra container to the workflow with the
CASSANDRA_HOSTS/CASSANDRA_KEYSPACE environment variables populated (so the
service is reachable at the hostname used by ELASTICSEARCH_URL/CASSANDRA_HOSTS)
or modify the tested endpoint to avoid calling
getTelemetryHistoryFromCassandra() during the k6 run (e.g., mock/skip Cassandra
reads or make the function fall back gracefully when Cassandra is unavailable).
Ensure the change references getTelemetryHistoryFromCassandra(),
deviceTelemetry, and the CASSANDRA_* env vars so the test run no longer fails.

In @.github/workflows/terraform.yml:
- Around line 83-117: The workflow's apply job named "apply" currently targets
only the dev environment (environment: dev) and lacks documentation or a
production/DR deployment path; either add an in-line comment near the apply job
explaining the intended production/DR deployment strategy and controls, or
create a separate job/workflow (e.g., apply-prod) that targets production/DR
with stricter guards (environment: prod, required reviewers/approval, protected
branch checks, and more restrictive if condition than the current if:
github.event_name == 'push' && github.ref == 'refs/heads/master'), and reference
how secrets/roles (secrets.AWS_TF_ROLE_ARN, TF_VAR_DB_PASSWORD) and manual
approvals should differ for production/DR.
- Around line 23-25: The job matrix under strategy.matrix.env currently lists
only dev and prod; add "dr" to that env array so DR runs in PR validation
(update the env: [dev, prod, dr] in the matrix), and handle DR-specific secrets
by either providing placeholder/dummy values for aurora_global_cluster_id and
redis_global_datastore_id during plan or gating the DR run behind a conditional
that skips plan when those secrets are missing (or create a separate job for DR
that requires the secrets); reference the matrix env key and the DR-specific
variables aurora_global_cluster_id and redis_global_datastore_id when
implementing the change.
- Line 22: The job-level if uses the secrets context (secrets.AWS_TF_ROLE_ARN)
which is invalid; remove the secret check from the job-level if and instead
perform the secret presence check at the step level (e.g., move
"secrets.AWS_TF_ROLE_ARN != ''" into the relevant step's if), or replace it with
a repository variable/inputs available at job-evaluation time; for both
occurrences referenced (the if around the job and the second occurrence) update
the job's if to only use allowed contexts (github/needs/vars/inputs) and add
step-level guards using if: secrets.AWS_TF_ROLE_ARN != '' (or set a prior step
that outputs a boolean and use that output in job/step logic) so the workflow no
longer references secrets in job-level expressions.

In `@apps/saga-orchestrator/internal/repository/postgres_saga_repository.go`:
- Around line 55-56: The repository query that uses "ORDER BY created_at DESC
LIMIT 1" masks duplicate sagas for a correlation_id; update the repository
method that issues this SQL to instead select all rows for the given
correlation_id (remove LIMIT), apply a deterministic order (e.g., ORDER BY
created_at ASC, id ASC) and then: if 0 rows return nil, if exactly 1 return that
saga, and if >1 return a clear error (e.g., ErrDuplicateSagaForCorrelationID) so
the caller in provision_saga.go's idempotency gate fails fast; additionally add
a DB migration to create a unique constraint on sagas(correlation_id) to prevent
future duplicates (and include a remediation strategy in the migration for
existing duplicates).

In `@infra/terraform/environments/dr/main.tf`:
- Around line 5-6: Replace the manual string variables aurora_global_cluster_id
and redis_global_datastore_id with a terraform_remote_state data source: add a
data "terraform_remote_state" "prod" block configured for the prod backend and
reference the outputs via
data.terraform_remote_state.prod.outputs.aurora_global_cluster_id and
data.terraform_remote_state.prod.outputs.redis_global_datastore_id wherever
those variables are used, and remove or deprecated the original variable
declarations (variable "aurora_global_cluster_id" and variable
"redis_global_datastore_id") so the DR environment automatically pulls the prod
values.
- Line 24: The VPC CIDR "10.2.0.0/16" is hardcoded in multiple places (vpc_cidr
assignments); create a locals block like locals { vpc_cidr = "10.2.0.0/16" } and
replace every direct "10.2.0.0/16" vpc_cidr usage with local.vpc_cidr (update
all module calls that pass vpc_cidr) so the value is centralized and no
duplicates remain.

In `@infra/terraform/environments/prod/main.tf`:
- Line 8: Replace the repeated hardcoded VPC CIDR string by defining a single
locals entry and referencing it: add a locals block with vpc_cidr =
"10.1.0.0/16" (declare as locals { vpc_cidr = "10.1.0.0/16" }) and update all
module arguments that currently use the literal "10.1.0.0/16" (e.g., the
vpc_cidr = "10.1.0.0/16" occurrences in the main.tf module calls) to use
local.vpc_cidr instead so all references (the four duplicate sites) read
vpc_cidr = local.vpc_cidr.

In `@infra/terraform/modules/aurora-global/main.tf`:
- Around line 116-129: The aws_rds_cluster_instance resource
(aws_rds_cluster_instance.this) is missing auto_minor_version_upgrade, causing
Checkov CKV_AWS_226 failures; update the resource to set
auto_minor_version_upgrade = true for each instance so minor engine patches are
applied automatically (add the attribute within the
aws_rds_cluster_instance.this block alongside
instance_class/engine_version/monitoring_interval to enable automatic minor
version upgrades).
- Around line 44-63: The aws_security_group "aurora" resource is missing
description fields required by policy CKV_AWS_23; update the resource to include
a top-level description for the security group and add description attributes on
the ingress block (for the PostgreSQL port 5432; reference
aws_security_group.aurora.ingress) and the egress block
(aws_security_group.aurora.egress) describing their intent and scope so each
rule and the SG itself have clear descriptions.

In `@infra/terraform/modules/elasticache-global/main.tf`:
- Around line 33-52: The aws_security_group resource aws_security_group.redis is
missing descriptions; add a descriptive description attribute on the security
group resource itself and add description fields to both the ingress and egress
blocks (the ingress block for port 6379 and the egress block protocol "-1") to
satisfy CKV_AWS_23 and improve auditability; ensure descriptions clearly state
purpose (e.g., "Allow Redis traffic from VPC CIDR" and "Allow all outbound
traffic") while retaining existing names/tags and CIDR/port settings.
- Around line 60-86: The replication group
aws_elasticache_replication_group.redis is missing an auth token (CKV_AWS_31);
add a new sensitive variable named auth_token (type = string, sensitive = true,
default = null) and reference it in the replication group resource by setting
auth_token = var.auth_token so clients require AUTH when provided
(transit_encryption_enabled is already true); ensure the variable is documented
and marked sensitive in variables.tf and used in any relevant module inputs or
CI secrets.

---

Duplicate comments:
In @.github/workflows/terraform.yml:
- Line 15: The AWS_REGION key in the GitHub Actions workflow has an extra space
after the colon ("AWS_REGION:  "us-east-1") which YAML linter flags; update the
workflow to remove the extra space so the entry reads AWS_REGION: "us-east-1"
(locate the AWS_REGION line in the terraform.yml file and correct the spacing).

In `@apps/bff/src/datasources/redis.ts`:
- Around line 41-47: getMany currently uses client.multi() / pipeline.exec()
which can produce CROSSSLOT errors in Redis Cluster; change getMany to perform
cluster-safe independent GETs (e.g., Promise.all over client.get(key) for each
key) rather than batching with client.multi()/pipeline.exec(), then parse each
result with JSON.parse as before; update the function getMany to remove pipeline
usage and use per-key client.get calls to ensure cluster safety.

In `@infra/terraform/modules/aurora-global/main.tf`:
- Around line 34-37: Add validation to prevent an empty global_cluster_id when
the module is configured as a secondary: add a check block (e.g., name it
require_global_cluster_id_when_secondary) that asserts !(var.is_secondary &&
var.global_cluster_id == "") and supplies a clear error_message like
"global_cluster_id must be set when is_secondary = true"; this ensures the
variable "global_cluster_id" and the conditional "var.is_secondary" are
validated at plan time rather than failing later at apply.
- Around line 101-102: The current conditions use skip_final_snapshot =
var.environment != "prod" and deletion_protection = var.environment == "prod",
which leaves the "dr" environment unprotected; update the logic so the DR
environment is treated like prod for these protections (i.e., ensure
skip_final_snapshot is false and deletion_protection is true when
var.environment == "dr" or "prod"). Change the boolean expressions around
skip_final_snapshot and deletion_protection to include var.environment == "dr"
alongside "prod" (e.g., treat both "prod" and "dr" as protected environments) so
the DR cluster keeps a final snapshot and has deletion protection.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 35ec6991-1e8c-42e7-bddf-f3c46e01140f

📥 Commits

Reviewing files that changed from the base of the PR and between 4a25f89 and cad477d.

⛔ Files ignored due to path filters (1)

• apps/gateway/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (10)

• .github/workflows/perf.yml
• .github/workflows/security.yml
• .github/workflows/terraform.yml
• apps/bff/src/datasources/redis.ts
• apps/saga-orchestrator/internal/repository/postgres_saga_repository.go
• infra/terraform/environments/dr/main.tf
• infra/terraform/environments/prod/main.tf
• infra/terraform/environments/prod/variables.tf
• infra/terraform/modules/aurora-global/main.tf
• infra/terraform/modules/elasticache-global/main.tf

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Background processes lack explicit cleanup, but acceptable for CI.

Both the gateway and BFF are started with & without explicit cleanup. GitHub Actions will terminate all processes when the job ends, so this works but could leave processes running until job timeout if tests fail early.

Consider adding a cleanup step or using a process manager like pm2 for more controlled lifecycle management in future iterations.

Also applies to: 98-100

🤖 Prompt for AI Agents

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

In @.github/workflows/perf.yml around lines 57 - 59, The workflow starts the
gateway and BFF with backgrounding ("Start gateway in background" and the
analogous "Start bff in background") but doesn't clean them up; update the steps
to capture their PIDs (run the commands with & and write $! to
gateway.pid/bff.pid) and add a subsequent cleanup step that reads those files
and kills the processes (or use pkill with the node/ts-node command), or
alternatively switch these steps to launch via a process manager like pm2 and
stop the processes in a teardown step; ensure the cleanup step runs even on
failure by placing it in a finally/always-run step so background processes are
terminated before the job finishes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if BFF code has required vs optional handling for Elasticsearch/Cassandra
rg -n "ELASTICSEARCH_URL|CASSANDRA" apps/bff/src --type ts -B2 -A5 | head -80

Repository: pahuldeepp/GrainGuard-

Length of output: 2396

---

🏁 Script executed:

# First, let's look at the perf.yml workflow around lines 108-110 and understand context
head -150 .github/workflows/perf.yml | tail -50

Repository: pahuldeepp/GrainGuard-

Length of output: 2097

---

🏁 Script executed:

# Let's see the complete structure of the workflow to identify what services are defined
cat .github/workflows/perf.yml | grep -A 5 -B 5 "services:" | head -50

Repository: pahuldeepp/GrainGuard-

Length of output: 310

---

🏁 Script executed:

# Check what endpoints are being tested in the performance test
rg -n "url|endpoint|request" .github/workflows/perf.yml | head -20

Repository: pahuldeepp/GrainGuard-

Length of output: 200

---

🏁 Script executed:

# Check if Cassandra client is actually called in the BFF startup or test endpoints
rg -n "getCassandraClient|cassandra" apps/bff/src --type ts | head -30

Repository: pahuldeepp/GrainGuard-

Length of output: 763

---

🏁 Script executed:

# Check if Elasticsearch client is actually used in test endpoints
rg -n "es\.search|es\.index|elasticsearch" apps/bff/src --type ts | head -30

Repository: pahuldeepp/GrainGuard-

Length of output: 456

---

🏁 Script executed:

# Check if Cassandra client is initialized at server startup
cat apps/bff/src/server.ts | head -100

Repository: pahuldeepp/GrainGuard-

Length of output: 3346

---

🏁 Script executed:

# Find the k6 test script
find . -name "*.js" -o -name "*.ts" | xargs grep -l "k6\|http\.get\|http\.post" | grep -i "perf\|load\|test" | head -10

Repository: pahuldeepp/GrainGuard-

Length of output: 277

---

🏁 Script executed:

# Look for k6 test files more broadly
find . -path ./node_modules -prune -o -type f -name "*perf*" -o -name "*load*" -o -name "*k6*" 2>/dev/null | grep -v node_modules

Repository: pahuldeepp/GrainGuard-

Length of output: 501

---

🏁 Script executed:

# Check what k6 script is invoked in perf.yml
grep -n "k6\|\.js" .github/workflows/perf.yml | tail -20

Repository: pahuldeepp/GrainGuard-

Length of output: 643

---

🏁 Script executed:

# Examine the main k6 test script
cat scripts/load-tests/performance-budget.js

Repository: pahuldeepp/GrainGuard-

Length of output: 5258

---

BFF requires Cassandra service but workflow defines none.

The deviceTelemetry GraphQL query tested by k6 calls getTelemetryHistoryFromCassandra(), which attempts to connect to Cassandra at startup and throws on connection failure (not gracefully degraded). With only PostgreSQL defined as a service, the BFF will crash when the k6 test invokes this endpoint, causing workflow failure.

Add a Cassandra service container to the workflow, or remove the Cassandra dependency from the tested endpoints.

🤖 Prompt for AI Agents

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

In @.github/workflows/perf.yml around lines 108 - 110, The perf workflow is
missing a Cassandra service so the BFF's deviceTelemetry endpoint (which calls
getTelemetryHistoryFromCassandra()) crashes on startup; either add a Cassandra
container to the workflow with the CASSANDRA_HOSTS/CASSANDRA_KEYSPACE
environment variables populated (so the service is reachable at the hostname
used by ELASTICSEARCH_URL/CASSANDRA_HOSTS) or modify the tested endpoint to
avoid calling getTelemetryHistoryFromCassandra() during the k6 run (e.g.,
mock/skip Cassandra reads or make the function fall back gracefully when
Cassandra is unavailable). Ensure the change references
getTelemetryHistoryFromCassandra(), deviceTelemetry, and the CASSANDRA_* env
vars so the test run no longer fails.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

GitHub Actions: which contexts are available in job-level if conditions?

💡 Result:

In GitHub Actions, the contexts available in job-level if conditions (under jobs.<job_id>.if) are: github, needs, vars, and inputs. Additionally, the status check functions always, cancelled, success, and failure can be used in these expressions. These are evaluated before the job is routed to a runner, so contexts like job, runner, env, strategy, matrix, and steps are not available at this stage.

Citations:

• 1: https://docs.github.com/en/actions/learn-github-actions/contexts
• 2: https://docs.github.com/actions/using-jobs/using-conditions-to-control-job-execution
• 3: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
• 4: https://docs.github.com/en/actions/reference/contexts-reference

---

secrets context is not available in job-level if conditions and will cause the workflow to fail.

Only github, needs, vars, and inputs contexts are available in job-level if expressions. Move the secret check into a step, use a repository variable, or remove the secret check and let AWS authentication fail naturally if credentials are missing.

Also applies to line 87.

🧰 Tools

🪛 actionlint (1.7.11)

[error] 22-22: context "secrets" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml at line 22, The job-level if uses the
secrets context (secrets.AWS_TF_ROLE_ARN) which is invalid; remove the secret
check from the job-level if and instead perform the secret presence check at the
step level (e.g., move "secrets.AWS_TF_ROLE_ARN != ''" into the relevant step's
if), or replace it with a repository variable/inputs available at job-evaluation
time; for both occurrences referenced (the if around the job and the second
occurrence) update the job's if to only use allowed contexts
(github/needs/vars/inputs) and add step-level guards using if:
secrets.AWS_TF_ROLE_ARN != '' (or set a prior step that outputs a boolean and
use that output in job/step logic) so the workflow no longer references secrets
in job-level expressions.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

DR environment is missing from the plan matrix.

The matrix includes [dev, prod] but not dr. The DR environment has its own Terraform configuration at infra/terraform/environments/dr/ but won't be validated during PRs.

🔧 Proposed fix

     strategy:
       matrix:
-        env: [dev, prod]
+        env: [dev, prod, dr]

Note: The DR environment requires additional variables (aurora_global_cluster_id, redis_global_datastore_id) that may not be available during plan. You may need to provide dummy values or skip validation for DR if the secrets aren't configured.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	strategy:
	matrix:
	env: [dev, prod]
	strategy:
	matrix:
	env: [dev, prod, dr]

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 23 - 25, The job matrix under
strategy.matrix.env currently lists only dev and prod; add "dr" to that env
array so DR runs in PR validation (update the env: [dev, prod, dr] in the
matrix), and handle DR-specific secrets by either providing placeholder/dummy
values for aurora_global_cluster_id and redis_global_datastore_id during plan or
gating the DR run behind a conditional that skips plan when those secrets are
missing (or create a separate job for DR that requires the secrets); reference
the matrix env key and the DR-specific variables aurora_global_cluster_id and
redis_global_datastore_id when implementing the change.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Apply job only targets dev — production and DR deployment strategy should be documented.

The apply job only deploys to dev. This is a valid pattern (prod/DR require more careful deployment), but the strategy for deploying to prod and DR should be documented. Consider adding a comment or creating a separate workflow for production deployments with stricter controls.

🧰 Tools

🪛 actionlint (1.7.11)

[error] 87-87: context "secrets" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 83 - 117, The workflow's apply
job named "apply" currently targets only the dev environment (environment: dev)
and lacks documentation or a production/DR deployment path; either add an
in-line comment near the apply job explaining the intended production/DR
deployment strategy and controls, or create a separate job/workflow (e.g.,
apply-prod) that targets production/DR with stricter guards (environment: prod,
required reviewers/approval, protected branch checks, and more restrictive if
condition than the current if: github.event_name == 'push' && github.ref ==
'refs/heads/master'), and reference how secrets/roles (secrets.AWS_TF_ROLE_ARN,
TF_VAR_DB_PASSWORD) and manual approvals should differ for production/DR.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Eliminate duplicated VPC CIDR values.

The VPC CIDR "10.1.0.0/16" is hardcoded in four locations. If this needs to change, it must be updated in multiple places, risking inconsistency.

♻️ Proposed fix using a local value

Add a locals block at the top of the file:

locals {
  vpc_cidr = "10.1.0.0/16"
}

Then reference it in all module calls:

 module "vpc" {
   source             = "../../modules/vpc"
   project            = var.project
   environment        = "prod"
-  vpc_cidr           = "10.1.0.0/16"
+  vpc_cidr           = local.vpc_cidr
   availability_zones = ["us-east-1a", "us-east-1b", "us-east-1c"]
 }
...
 module "aurora" {
   ...
-  vpc_cidr           = "10.1.0.0/16"
+  vpc_cidr           = local.vpc_cidr
...
 module "redis" {
   ...
-  vpc_cidr           = "10.1.0.0/16"
+  vpc_cidr           = local.vpc_cidr
...
 module "msk" {
   ...
-  vpc_cidr           = "10.1.0.0/16"
+  vpc_cidr           = local.vpc_cidr

Also applies to: 28-28, 41-41, 52-52

🤖 Prompt for AI Agents

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

In `@infra/terraform/environments/prod/main.tf` at line 8, Replace the repeated
hardcoded VPC CIDR string by defining a single locals entry and referencing it:
add a locals block with vpc_cidr = "10.1.0.0/16" (declare as locals { vpc_cidr =
"10.1.0.0/16" }) and update all module arguments that currently use the literal
"10.1.0.0/16" (e.g., the vpc_cidr = "10.1.0.0/16" occurrences in the main.tf
module calls) to use local.vpc_cidr instead so all references (the four
duplicate sites) read vpc_cidr = local.vpc_cidr.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Add descriptions to security group and rules.

Similar to the ElastiCache module, the Aurora security group lacks description attributes (CKV_AWS_23).

♻️ Proposed fix

 resource "aws_security_group" "aurora" {
-  name   = "${local.name}-aurora"
-  vpc_id = var.vpc_id
+  name        = "${local.name}-aurora"
+  description = "Security group for Aurora PostgreSQL cluster"
+  vpc_id      = var.vpc_id

   ingress {
+    description = "PostgreSQL traffic from VPC"
     from_port   = 5432

🧰 Tools

🪛 Checkov (3.2.510)

[low] 44-63: Ensure every security group and rule has a description

(CKV_AWS_23)

---

[low] 44-63: Ensure no security groups allow egress from 0.0.0.0:0 to port -1

(CKV_AWS_382)

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/aurora-global/main.tf` around lines 44 - 63, The
aws_security_group "aurora" resource is missing description fields required by
policy CKV_AWS_23; update the resource to include a top-level description for
the security group and add description attributes on the ingress block (for the
PostgreSQL port 5432; reference aws_security_group.aurora.ingress) and the
egress block (aws_security_group.aurora.egress) describing their intent and
scope so each rule and the SG itself have clear descriptions.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Enable automatic minor version upgrades for security patching.

The instances lack auto_minor_version_upgrade = true. This is flagged by Checkov (CKV_AWS_226). Enabling it ensures the cluster receives security patches automatically during maintenance windows.

♻️ Proposed fix

 resource "aws_rds_cluster_instance" "this" {
   count = var.is_secondary ? 1 : 2

   identifier         = "${local.name}-aurora-${count.index}"
   cluster_identifier = aws_rds_cluster.this.id
   instance_class     = var.instance_class
   engine             = "aurora-postgresql"
   engine_version     = var.engine_version
+  auto_minor_version_upgrade = true

   performance_insights_enabled = true
   monitoring_interval          = 60

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	resource "aws_rds_cluster_instance" "this" {
	count = var.is_secondary ? 1 : 2 # primary: 1 writer + 1 reader; DR: 1 reader
	identifier = "${local.name}-aurora-${count.index}"
	cluster_identifier = aws_rds_cluster.this.id
	instance_class = var.instance_class
	engine = "aurora-postgresql"
	engine_version = var.engine_version
	performance_insights_enabled = true
	monitoring_interval = 60 # Enhanced Monitoring: 1-min granularity
	tags = { Name = "${local.name}-aurora-${count.index}" }
	}
	resource "aws_rds_cluster_instance" "this" {
	count = var.is_secondary ? 1 : 2 # primary: 1 writer + 1 reader; DR: 1 reader
	identifier = "${local.name}-aurora-${count.index}"
	cluster_identifier = aws_rds_cluster.this.id
	instance_class = var.instance_class
	engine = "aurora-postgresql"
	engine_version = var.engine_version
	auto_minor_version_upgrade = true
	performance_insights_enabled = true
	monitoring_interval = 60 # Enhanced Monitoring: 1-min granularity
	tags = { Name = "${local.name}-aurora-${count.index}" }
	}

🧰 Tools

🪛 Checkov (3.2.510)

[low] 116-129: Ensure DB instance gets all minor upgrades automatically

(CKV_AWS_226)

---

[high] 116-129: Ensure RDS Performance Insights are encrypted using KMS CMKs

(CKV_AWS_354)

---

[low] 116-129: Ensure DB instance gets all minor upgrades automatically

(CKV_AWS_226)

---

[high] 116-129: Ensure RDS Performance Insights are encrypted using KMS CMKs

(CKV_AWS_354)

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/aurora-global/main.tf` around lines 116 - 129, The
aws_rds_cluster_instance resource (aws_rds_cluster_instance.this) is missing
auto_minor_version_upgrade, causing Checkov CKV_AWS_226 failures; update the
resource to set auto_minor_version_upgrade = true for each instance so minor
engine patches are applied automatically (add the attribute within the
aws_rds_cluster_instance.this block alongside
instance_class/engine_version/monitoring_interval to enable automatic minor
version upgrades).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Add descriptions to security group and rules for compliance and auditability.

The security group and its rules lack description attributes. This is flagged by Checkov (CKV_AWS_23) and improves auditability when reviewing security configurations in AWS Console or during audits.

♻️ Proposed fix

 resource "aws_security_group" "redis" {
-  name   = "${local.name}-redis"
-  vpc_id = var.vpc_id
+  name        = "${local.name}-redis"
+  description = "Security group for ElastiCache Redis cluster"
+  vpc_id      = var.vpc_id

   ingress {
+    description = "Redis traffic from VPC"
     from_port   = 6379
     to_port     = 6379
     protocol    = "tcp"
     cidr_blocks = [var.vpc_cidr]
   }

   egress {
+    description = "Allow all outbound traffic"
     from_port   = 0
     to_port     = 0
     protocol    = "-1"
     cidr_blocks = ["0.0.0.0/0"]
   }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	resource "aws_security_group" "redis" {
	name = "${local.name}-redis"
	vpc_id = var.vpc_id
	ingress {
	from_port = 6379
	to_port = 6379
	protocol = "tcp"
	cidr_blocks = [var.vpc_cidr]
	}
	egress {
	from_port = 0
	to_port = 0
	protocol = "-1"
	cidr_blocks = ["0.0.0.0/0"]
	}
	tags = { Name = "${local.name}-redis-sg" }
	}
	resource "aws_security_group" "redis" {
	name = "${local.name}-redis"
	description = "Security group for ElastiCache Redis cluster"
	vpc_id = var.vpc_id
	ingress {
	description = "Redis traffic from VPC"
	from_port = 6379
	to_port = 6379
	protocol = "tcp"
	cidr_blocks = [var.vpc_cidr]
	}
	egress {
	description = "Allow all outbound traffic"
	from_port = 0
	to_port = 0
	protocol = "-1"
	cidr_blocks = ["0.0.0.0/0"]
	}
	tags = { Name = "${local.name}-redis-sg" }
	}

🧰 Tools

🪛 Checkov (3.2.510)

[low] 33-52: Ensure every security group and rule has a description

(CKV_AWS_23)

---

[low] 33-52: Ensure no security groups allow egress from 0.0.0.0:0 to port -1

(CKV_AWS_382)

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/elasticache-global/main.tf` around lines 33 - 52, The
aws_security_group resource aws_security_group.redis is missing descriptions;
add a descriptive description attribute on the security group resource itself
and add description fields to both the ingress and egress blocks (the ingress
block for port 6379 and the egress block protocol "-1") to satisfy CKV_AWS_23
and improve auditability; ensure descriptions clearly state purpose (e.g.,
"Allow Redis traffic from VPC CIDR" and "Allow all outbound traffic") while
retaining existing names/tags and CIDR/port settings.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Consider adding AUTH token for Redis authentication.

The replication group has encryption enabled (transit_encryption_enabled = true, at_rest_encryption_enabled = true) but lacks an auth_token for client authentication. Checkov flags this (CKV_AWS_31). For production workloads, AUTH tokens provide an additional layer of security beyond network isolation.

🔒 Proposed enhancement

Add a variable for the auth token:

variable "auth_token" {
  type      = string
  sensitive = true
  default   = null
}

Then reference it in the replication group:

 resource "aws_elasticache_replication_group" "redis" {
   ...
   transit_encryption_enabled = true
+  auth_token                 = var.auth_token
   ...
 }

Note: auth_token requires transit_encryption_enabled = true, which is already set.

🧰 Tools

🪛 Checkov (3.2.510)

[low] 60-86: Ensure ElastiCache replication group is encrypted by KMS using a customer managed Key (CMK)

(CKV_AWS_191)

---

[low] 60-86: Ensure all data stored in the ElastiCache Replication Group is securely encrypted at transit and has auth token

(CKV_AWS_31)

🤖 Prompt for AI Agents

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

In `@infra/terraform/modules/elasticache-global/main.tf` around lines 60 - 86, The
replication group aws_elasticache_replication_group.redis is missing an auth
token (CKV_AWS_31); add a new sensitive variable named auth_token (type =
string, sensitive = true, default = null) and reference it in the replication
group resource by setting auth_token = var.auth_token so clients require AUTH
when provided (transit_encryption_enabled is already true); ensure the variable
is documented and marked sensitive in variables.tf and used in any relevant
module inputs or CI secrets.

[coderabbitai]

Actionable comments posted: 12

♻️ Duplicate comments (2)

.github/workflows/terraform.yml (1)

65-81: 🧹 Nitpick | 🔵 Trivial

Consider adding error handling for PR comment creation.

If the GitHub API call fails (e.g., rate limiting, permission issues), the error is not caught and may cause confusing failures. Consider wrapping in try/catch.

♻️ Proposed fix with error handling

       - name: Post plan as PR comment
         uses: actions/github-script@v7
         with:
           script: |
             const fs = require('fs');
-            const plan = fs.readFileSync('infra/terraform/environments/${{ matrix.env }}/plan.txt', 'utf8');
-            const body = `## Terraform Plan — \`${{ matrix.env }}\`\n\`\`\`hcl\n${plan.slice(0, 65000)}\n\`\`\``;
-            github.rest.issues.createComment({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              issue_number: context.issue.number,
-              body,
-            });
+            try {
+              const plan = fs.readFileSync('infra/terraform/environments/${{ matrix.env }}/plan.txt', 'utf8');
+              const body = `## Terraform Plan — \`${{ matrix.env }}\`\n\`\`\`hcl\n${plan.slice(0, 65000)}\n\`\`\``;
+              await github.rest.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: context.issue.number,
+                body,
+              });
+            } catch (error) {
+              core.warning(`Failed to post plan comment: ${error.message}`);
+            }

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 65 - 81, Wrap the GitHub API
call in the "Post plan as PR comment" step's script in a try/catch: around
reading the plan and the github.rest.issues.createComment(...) call, catch any
errors and log a clear message (e.g., console.error) including the caught error,
and decide an appropriate fallback (fail the step with process.exit(1) or post a
warning) so rate limits/permission errors are handled gracefully; ensure you
reference the existing variables plan and body and preserve slicing of
plan.slice(0, 65000).

.github/workflows/e2e.yml (1)

11-16: ⚠️ Potential issue | 🟡 Minor

Add condition to skip job for fork PRs where secrets are unavailable.

Fork PRs don't have access to repository secrets, causing this workflow to fail noisily. Since the build requires Auth0 secrets, add a job-level condition to skip gracefully.

🛡️ Proposed fix

 jobs:
   e2e:
     name: Playwright E2E
     runs-on: ubuntu-latest
     timeout-minutes: 20
+    if: ${{ github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false }}

🤖 Prompt for AI Agents

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

In @.github/workflows/e2e.yml around lines 11 - 16, The e2e job runs for forked
PRs where repo secrets are unavailable; update the job-level condition for the
job named "e2e" to skip when the PR comes from a fork. Add an if condition such
as: if: github.event.pull_request == null ||
github.event.pull_request.head.repo.full_name == github.repository so the "e2e"
job only runs on non-fork PRs (or non-PR events) where repository secrets are
accessible.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In @.github/workflows/e2e.yml:
- Around line 39-48: The YAML step named "Build dashboard" has extra spaces
after the colons in the env mapping (e.g., "VITE_AUTH0_DOMAIN:    ${{
secrets.VITE_AUTH0_DOMAIN }}"), which trips yamllint; edit that step to remove
extra spaces so each environment variable uses a single colon followed by a
single space before the value (update the env keys like VITE_AUTH0_DOMAIN,
VITE_AUTH0_CLIENT_ID, VITE_AUTH0_AUDIENCE, VITE_BFF_URL and VITE_GATEWAY_URL in
the "Build dashboard" job to have consistent "KEY: value" formatting).
- Around line 56-63: The "Run Playwright tests" workflow step has misformatted
environment variables under env; convert them to a proper YAML mapping by
placing each key directly under env with consistent indentation and quoted
values where needed (e.g., E2E_BASE_URL: "http://localhost:5173") and keep
secret references as ${ { secrets.VITE_AUTH0_CLIENT_ID } } style (no extra
spaces around the colon or alignment padding). Update the step named "Run
Playwright tests" so env looks like a standard key: "value" mapping for
E2E_BASE_URL, VITE_AUTH0_CLIENT_ID, and VITE_AUTH0_AUDIENCE to ensure the runner
parses them correctly.

In @.github/workflows/terraform.yml:
- Around line 83-91: The job-level if for the apply job named "apply"
incorrectly references secrets.AWS_TF_ROLE_ARN (secrets are unavailable at
job-level); change the job-level if on the apply job to only check
github.event_name and github.ref, then add a step inside the apply job (before
any AWS or terraform steps) that uses a step-level if to check
secrets.AWS_TF_ROLE_ARN != '' and fails/continues accordingly so the secret is
evaluated at step-level; update the step to run in the same working-directory
(infra/terraform/environments/dev) and keep the environment: dev setting
unchanged.
- Around line 17-28: The job-level if on the plan job uses the secrets context
which is invalid; update the plan job (named "Terraform Plan (${{ matrix.env
}})" / job id "plan") to only use a valid job-level condition (e.g.,
github.event_name == 'pull_request'), remove the secrets check from the
job-level if, and add an early step inside the job (e.g., a step named "Check
for required secrets") that uses a step-level if like ${{
secrets.AWS_TF_ROLE_ARN == '' }} to warn and exit or skip the job; keep the
matrix.env/defaults working-directory as-is and ensure the new step runs before
any terraform actions.

In `@Makefile`:
- Around line 101-106: The Makefile target lint-fix calls non-existent npm
scripts "npm run lint:fix" for gateway and jobs-worker; update the lint-fix
recipe so it calls the actual fixable lint command used elsewhere (e.g., replace
"cd apps/gateway && npm run lint:fix" and "cd apps/jobs-worker && npm run
lint:fix" with "cd apps/gateway && npm run lint -- --fix" and "cd
apps/jobs-worker && npm run lint -- --fix") or alternatively add a "lint:fix"
script to those services' package.json so the existing commands succeed; edit
the lint-fix target accordingly.
- Around line 90-94: The Makefile targets lint-ts and the related lint:fix calls
invoke npm run lint / npm run lint:fix for apps/gateway and apps/jobs-worker but
those services' package.json files lack those scripts; either add "lint" and
"lint:fix" scripts to apps/gateway/package.json and
apps/jobs-worker/package.json (matching the scripts defined in
apps/bff/package.json) or remove the npm run lint and npm run lint:fix
invocations for those services from the lint-ts (and lint-ts-fix) targets so the
Makefile only calls lint on services that actually define the scripts.

In `@tests/chaos/redis-outage.sh`:
- Around line 85-97: Wrap all Redis client calls in
apps/bff/src/datasources/redis.ts (notably the methods invoked as cache.get,
cache.acquireLock, cache.set, cache.releaseLock) in try-catch blocks so they no
longer throw when Redis is down; on error, log the error and return cache-miss
semantics (e.g., cache.get returns null, cache.acquireLock returns false,
cache.set/releaseLock no-op or false) so that
apps/bff/src/datasources/postgres.ts (which calls cached = await cache.get(...);
and locked = await cache.acquireLock(...);) will receive safe values and proceed
to query Postgres and perform fallback behavior. Ensure the signatures/returns
remain consistent so callers need no other changes.

In `@tests/e2e/billing.spec.ts`:
- Around line 24-28: The test in billing.spec.ts fails because "Contact Sales"
is rendered as a <button> in BillingPage.tsx (Enterprise action) not a link;
update the test to locate it with getByRole("button", { name: "Contact Sales" })
and assert visibility, then either remove the toHaveAttribute("href", /mailto:/)
check or replace it with a click + navigation assertion (e.g., stub/spy
window.location.href or assert the resulting navigation) since the component
sets window.location.href in its onClick handler rather than using an anchor.

In `@tests/e2e/devices.spec.ts`:
- Around line 38-43: The test expects serial input to be uppercased but
RegisterDeviceModal's onChange currently calls setSerial(e.target.value) leaving
casing unchanged; either change the onChange in RegisterDeviceModal (the input's
onChange handler that uses setSerial and setValidationError) to call
setSerial(e.target.value.toUpperCase()) so the component normalizes input, or
update/remove the test in devices.spec.ts to assert the current behavior (no
uppercase) instead of expecting "SN12345678". Ensure you modify the onChange
handler where setSerial is invoked or update the test assertion to match the
component.
- Around line 57-63: The test "invalid serial shows validation error" expects
the wrong message; update the expectation in tests/e2e/devices.spec.ts (the test
named invalid serial shows validation error) to assert the actual component
validation string from RegisterDeviceModal.tsx (the message: "Only letters,
numbers, hyphens and underscores allowed (3–64 chars)") instead of "4–30
uppercase" so the test matches the real validation text.
- Around line 45-55: The test's expectations are wrong: the form only disables
the submit button when loading or the serial is empty (disabled condition is
loading || !serial.trim()), and regex validation (SERIAL_REGEX) runs only in
handleSubmit, so update the test in devices.spec.ts to assert the "Register
Device" button is disabled initially, then enabled after filling any non-empty
serial (e.g., "SN1"), and remove the expectation that "SN12" toggles enabled
state; reference the submitBtn, the "Serial Number" field, and the
SERIAL_REGEX/handleSubmit behavior when making the change.

In `@tests/e2e/fixtures/mockAuth.ts`:
- Around line 27-33: The Auth0 localStorage cache key and mock responses use
"openid profile email" but the app's Auth0 provider includes "offline_access",
so update the AUTH0_CACHE_KEY constant and any mock scope strings to include
"offline_access" (i.e., change AUTH0_CACHE_KEY built from CLIENT_ID and AUDIENCE
to end with "::openid profile email offline_access") and update the matching
mock response scope values used elsewhere in this file (the other scope
constants/strings in the same module) so the Auth0 SPA SDK can find the cached
token; search for AUTH0_CACHE_KEY, any scope string variables, and the mock
token/response objects in this file and add "offline_access" to their scope
lists.

---

Duplicate comments:
In @.github/workflows/e2e.yml:
- Around line 11-16: The e2e job runs for forked PRs where repo secrets are
unavailable; update the job-level condition for the job named "e2e" to skip when
the PR comes from a fork. Add an if condition such as: if:
github.event.pull_request == null ||
github.event.pull_request.head.repo.full_name == github.repository so the "e2e"
job only runs on non-fork PRs (or non-PR events) where repository secrets are
accessible.

In @.github/workflows/terraform.yml:
- Around line 65-81: Wrap the GitHub API call in the "Post plan as PR comment"
step's script in a try/catch: around reading the plan and the
github.rest.issues.createComment(...) call, catch any errors and log a clear
message (e.g., console.error) including the caught error, and decide an
appropriate fallback (fail the step with process.exit(1) or post a warning) so
rate limits/permission errors are handled gracefully; ensure you reference the
existing variables plan and body and preserve slicing of plan.slice(0, 65000).

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fac12ff7-c7fd-4bf4-b2ad-1a3887f65721

📥 Commits

Reviewing files that changed from the base of the PR and between cad477d and 3c997c6.

📒 Files selected for processing (13)

• .github/workflows/chaos.yml
• .github/workflows/e2e.yml
• .github/workflows/perf.yml
• .github/workflows/terraform.yml
• Makefile
• apps/dashboard/src/features/devices/components/RegisterDeviceModal.tsx
• scripts/load-tests/performance-budget.js
• tests/chaos/README.md
• tests/chaos/redis-outage.sh
• tests/chaos/run-all.sh
• tests/e2e/billing.spec.ts
• tests/e2e/devices.spec.ts
• tests/e2e/fixtures/mockAuth.ts

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Fix YAML formatting: extra spaces after colons.

YAMLlint reports formatting errors on lines 43-47. While this doesn't affect functionality, it violates YAML style conventions.

♻️ Proposed fix

       - name: Build dashboard
         run: npm run build
         working-directory: apps/dashboard
         env:
-          VITE_AUTH0_DOMAIN:    ${{ secrets.VITE_AUTH0_DOMAIN }}
-          VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
-          VITE_AUTH0_AUDIENCE:  ${{ secrets.VITE_AUTH0_AUDIENCE }}
-          VITE_BFF_URL:         ${{ secrets.E2E_BFF_URL }}
-          VITE_GATEWAY_URL:     ${{ secrets.E2E_GATEWAY_URL }}
+          VITE_AUTH0_DOMAIN: ${{ secrets.VITE_AUTH0_DOMAIN }}
+          VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
+          VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}
+          VITE_BFF_URL: ${{ secrets.E2E_BFF_URL }}
+          VITE_GATEWAY_URL: ${{ secrets.E2E_GATEWAY_URL }}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- name: Build dashboard
	run: npm run build
	working-directory: apps/dashboard
	env:
	VITE_AUTH0_DOMAIN: ${{ secrets.VITE_AUTH0_DOMAIN }}
	VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
	VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}
	VITE_BFF_URL: ${{ secrets.E2E_BFF_URL }}
	VITE_GATEWAY_URL: ${{ secrets.E2E_GATEWAY_URL }}
	- name: Build dashboard
	run: npm run build
	working-directory: apps/dashboard
	env:
	VITE_AUTH0_DOMAIN: ${{ secrets.VITE_AUTH0_DOMAIN }}
	VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
	VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}
	VITE_BFF_URL: ${{ secrets.E2E_BFF_URL }}
	VITE_GATEWAY_URL: ${{ secrets.E2E_GATEWAY_URL }}

🧰 Tools

🪛 YAMLlint (1.38.0)

[error] 43-43: too many spaces after colon

(colons)

---

[error] 45-45: too many spaces after colon

(colons)

---

[error] 46-46: too many spaces after colon

(colons)

---

[error] 47-47: too many spaces after colon

(colons)

🤖 Prompt for AI Agents

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

In @.github/workflows/e2e.yml around lines 39 - 48, The YAML step named "Build
dashboard" has extra spaces after the colons in the env mapping (e.g.,
"VITE_AUTH0_DOMAIN:    ${{ secrets.VITE_AUTH0_DOMAIN }}"), which trips yamllint;
edit that step to remove extra spaces so each environment variable uses a single
colon followed by a single space before the value (update the env keys like
VITE_AUTH0_DOMAIN, VITE_AUTH0_CLIENT_ID, VITE_AUTH0_AUDIENCE, VITE_BFF_URL and
VITE_GATEWAY_URL in the "Build dashboard" job to have consistent "KEY: value"
formatting).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Fix YAML formatting in test step environment variables.

♻️ Proposed fix

       - name: Run Playwright tests
         run: npx playwright test --config playwright.config.ts
         working-directory: tests/e2e
         env:
-          E2E_BASE_URL:         http://localhost:5173
+          E2E_BASE_URL: http://localhost:5173
           VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
-          VITE_AUTH0_AUDIENCE:  ${{ secrets.VITE_AUTH0_AUDIENCE }}
+          VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- name: Run Playwright tests
	run: npx playwright test --config playwright.config.ts
	working-directory: tests/e2e
	env:
	E2E_BASE_URL: http://localhost:5173
	VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
	VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}
	- name: Run Playwright tests
	run: npx playwright test --config playwright.config.ts
	working-directory: tests/e2e
	env:
	E2E_BASE_URL: http://localhost:5173
	VITE_AUTH0_CLIENT_ID: ${{ secrets.VITE_AUTH0_CLIENT_ID }}
	VITE_AUTH0_AUDIENCE: ${{ secrets.VITE_AUTH0_AUDIENCE }}

🧰 Tools

🪛 YAMLlint (1.38.0)

[error] 60-60: too many spaces after colon

(colons)

---

[error] 62-62: too many spaces after colon

(colons)

🤖 Prompt for AI Agents

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

In @.github/workflows/e2e.yml around lines 56 - 63, The "Run Playwright tests"
workflow step has misformatted environment variables under env; convert them to
a proper YAML mapping by placing each key directly under env with consistent
indentation and quoted values where needed (e.g., E2E_BASE_URL:
"http://localhost:5173") and keep secret references as ${ {
secrets.VITE_AUTH0_CLIENT_ID } } style (no extra spaces around the colon or
alignment padding). Update the step named "Run Playwright tests" so env looks
like a standard key: "value" mapping for E2E_BASE_URL, VITE_AUTH0_CLIENT_ID, and
VITE_AUTH0_AUDIENCE to ensure the runner parses them correctly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

secrets context is not available in job-level if — workflow will fail to parse.

GitHub Actions only allows github, needs, vars, and inputs contexts in job-level if conditions. The secrets context is evaluated at step level, not job level. This will cause the workflow to fail.

🐛 Proposed fix: Move secret check to step level

   plan:
     name: Terraform Plan (${{ matrix.env }})
     runs-on: ubuntu-latest
-    if: ${{ github.event_name == 'pull_request' && secrets.AWS_TF_ROLE_ARN != '' }}
+    if: ${{ github.event_name == 'pull_request' }}
     strategy:
       matrix:
         env: [dev, prod]

Then add a step-level check at the beginning of the job:

    steps:
      - name: Check for required secrets
        if: ${{ secrets.AWS_TF_ROLE_ARN == '' }}
        run: |
          echo "::warning::Skipping Terraform plan - AWS_TF_ROLE_ARN secret not configured"
          exit 0

Alternatively, use a repository variable (vars.HAS_AWS_CREDENTIALS) that you set to true when secrets are configured.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	jobs:
	# ── Plan — runs on every PR that touches terraform/ ────────────────────────
	plan:
	name: Terraform Plan (${{ matrix.env }})
	runs-on: ubuntu-latest
	if: ${{ github.event_name == 'pull_request' && secrets.AWS_TF_ROLE_ARN != '' }}
	strategy:
	matrix:
	env: [dev, prod]
	defaults:
	run:
	working-directory: infra/terraform/environments/${{ matrix.env }}
	jobs:
	# ── Plan — runs on every PR that touches terraform/ ────────────────────────
	plan:
	name: Terraform Plan (${{ matrix.env }})
	runs-on: ubuntu-latest
	if: ${{ github.event_name == 'pull_request' }}
	strategy:
	matrix:
	env: [dev, prod]
	defaults:
	run:
	working-directory: infra/terraform/environments/${{ matrix.env }}

🧰 Tools

🪛 actionlint (1.7.11)

[error] 22-22: context "secrets" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 17 - 28, The job-level if on
the plan job uses the secrets context which is invalid; update the plan job
(named "Terraform Plan (${{ matrix.env }})" / job id "plan") to only use a valid
job-level condition (e.g., github.event_name == 'pull_request'), remove the
secrets check from the job-level if, and add an early step inside the job (e.g.,
a step named "Check for required secrets") that uses a step-level if like ${{
secrets.AWS_TF_ROLE_ARN == '' }} to warn and exit or skip the job; keep the
matrix.env/defaults working-directory as-is and ensure the new step runs before
any terraform actions.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Same issue: secrets context invalid in job-level if.

Line 87 has the same problem as line 22—secrets.AWS_TF_ROLE_ARN cannot be referenced in a job-level condition.

🐛 Proposed fix

   apply:
     name: Terraform Apply (dev)
     runs-on: ubuntu-latest
-    if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' && secrets.AWS_TF_ROLE_ARN != '' }}
+    if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' }}
     environment: dev

Add a step-level guard similar to the plan job suggestion.

🧰 Tools

🪛 actionlint (1.7.11)

[error] 87-87: context "secrets" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

🤖 Prompt for AI Agents

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

In @.github/workflows/terraform.yml around lines 83 - 91, The job-level if for
the apply job named "apply" incorrectly references secrets.AWS_TF_ROLE_ARN
(secrets are unavailable at job-level); change the job-level if on the apply job
to only check github.event_name and github.ref, then add a step inside the apply
job (before any AWS or terraform steps) that uses a step-level if to check
secrets.AWS_TF_ROLE_ARN != '' and fails/continues accordingly so the secret is
evaluated at step-level; update the step to run in the same working-directory
(infra/terraform/environments/dev) and keep the environment: dev setting
unchanged.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify lint scripts exist in each package.json
echo "=== gateway scripts ==="
jq '.scripts' apps/gateway/package.json

echo ""
echo "=== jobs-worker scripts ==="
jq '.scripts' apps/jobs-worker/package.json

echo ""
echo "=== bff scripts (for comparison) ==="
jq '.scripts' apps/bff/package.json

Repository: pahuldeepp/GrainGuard-

Length of output: 583

---

lint-ts will fail — gateway and jobs-worker lack lint scripts.

The Makefile at lines 91 and 93 calls npm run lint for services that don't define this script:

• apps/gateway/package.json has no lint script (only: dev, build, start, test, typecheck)
• apps/jobs-worker/package.json has no lint script (only: build, start, dev)
• apps/bff/package.json has these scripts defined, confirming they are expected

Lines 91 and 93 will fail with npm ERR! Missing script: lint. Additionally, lines 103 and 105 calling npm run lint:fix will also fail for the same services.

Fix: Either add the missing lint and lint:fix scripts to both package.json files (as defined in apps/bff/package.json), or remove those service calls from the Makefile.

🤖 Prompt for AI Agents

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

In `@Makefile` around lines 90 - 94, The Makefile targets lint-ts and the related
lint:fix calls invoke npm run lint / npm run lint:fix for apps/gateway and
apps/jobs-worker but those services' package.json files lack those scripts;
either add "lint" and "lint:fix" scripts to apps/gateway/package.json and
apps/jobs-worker/package.json (matching the scripts defined in
apps/bff/package.json) or remove the npm run lint and npm run lint:fix
invocations for those services from the lint-ts (and lint-ts-fix) targets so the
Makefile only calls lint on services that actually define the scripts.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Test will fail: "Contact Sales" is rendered as a <button>, not a link.

According to BillingPage.tsx (lines 239-257), the Enterprise action is a <button> element with an onClick handler that sets window.location.href to a mailto URL. Using getByRole("link") will not find it.

🐛 Proposed fix: Use button role instead

   test("Enterprise card shows Contact Sales link", async ({ page }) => {
-    const contactLink = page.getByRole("link", { name: "Contact Sales" });
-    await expect(contactLink).toBeVisible({ timeout: 10_000 });
-    await expect(contactLink).toHaveAttribute("href", /mailto:sales@/);
+    const contactBtn = page.getByRole("button", { name: "Contact Sales" });
+    await expect(contactBtn).toBeVisible({ timeout: 10_000 });
+    await expect(contactBtn).toBeEnabled();
   });

Note: Since the mailto is triggered via window.location.href in the click handler, you cannot assert the href attribute on a button. If you need to verify the mailto behavior, consider testing the navigation effect or refactoring the component to use an actual <a> element.

🤖 Prompt for AI Agents

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

In `@tests/e2e/billing.spec.ts` around lines 24 - 28, The test in billing.spec.ts
fails because "Contact Sales" is rendered as a <button> in BillingPage.tsx
(Enterprise action) not a link; update the test to locate it with
getByRole("button", { name: "Contact Sales" }) and assert visibility, then
either remove the toHaveAttribute("href", /mailto:/) check or replace it with a
click + navigation assertion (e.g., stub/spy window.location.href or assert the
resulting navigation) since the component sets window.location.href in its
onClick handler rather than using an anchor.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Test will fail: Component does not uppercase serial numbers.

According to RegisterDeviceModal.tsx (lines 97-105), the onChange handler sets serial directly via setSerial(e.target.value) without any uppercase transformation. This test expects "sn12345678" to become "SN12345678", but the component preserves the input as-is.

Either update the component to perform uppercasing:

onChange={(e) => {
  setSerial(e.target.value.toUpperCase());
  setValidationError(null);
}}

Or remove/update this test to match the current behavior.

🤖 Prompt for AI Agents

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

In `@tests/e2e/devices.spec.ts` around lines 38 - 43, The test expects serial
input to be uppercased but RegisterDeviceModal's onChange currently calls
setSerial(e.target.value) leaving casing unchanged; either change the onChange
in RegisterDeviceModal (the input's onChange handler that uses setSerial and
setValidationError) to call setSerial(e.target.value.toUpperCase()) so the
component normalizes input, or update/remove the test in devices.spec.ts to
assert the current behavior (no uppercase) instead of expecting "SN12345678".
Ensure you modify the onChange handler where setSerial is invoked or update the
test assertion to match the component.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for submit button disabled state logic
ast-grep --pattern 'disabled={$_}'
rg -n "disabled" --type=ts -A2 -B2 -g '*RegisterDeviceModal*'

Repository: pahuldeepp/GrainGuard-

Length of output: 2357

---

🏁 Script executed:

rg -n "SERIAL_REGEX" --type=ts

Repository: pahuldeepp/GrainGuard-

Length of output: 302

---

🏁 Script executed:

rg -n "onChange.*serial\|serial.*onChange" --type=ts -A5 -B2 -g '*RegisterDeviceModal*'

Repository: pahuldeepp/GrainGuard-

Length of output: 48

---

🏁 Script executed:

sed -n '90,140p' apps/dashboard/src/features/devices/components/RegisterDeviceModal.tsx

Repository: pahuldeepp/GrainGuard-

Length of output: 2209

---

🏁 Script executed:

rg -n "handleSubmit\|if (!SERIAL_REGEX" --type=ts -A10 -B2 -g '*RegisterDeviceModal*'

Repository: pahuldeepp/GrainGuard-

Length of output: 165

---

🏁 Script executed:

sed -n '30,50p' apps/dashboard/src/features/devices/components/RegisterDeviceModal.tsx

Repository: pahuldeepp/GrainGuard-

Length of output: 820

---

Test expectations don't match the actual button disable logic.

The submit button is disabled by loading || !serial.trim() (only when loading or input is empty), not by regex validation. Both "SN1" (3 chars) and "SN12" (4 chars) are non-empty, so the button would be enabled in both cases based on the current implementation.

The SERIAL_REGEX validation (/^[A-Za-z0-9\-_]{3,64}$/) happens only in the handleSubmit handler, not during input. The test expectations are incorrect—"SN1" is actually valid per the regex (3 characters is within the allowed range) and would not disable the button.

🤖 Prompt for AI Agents

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

In `@tests/e2e/devices.spec.ts` around lines 45 - 55, The test's expectations are
wrong: the form only disables the submit button when loading or the serial is
empty (disabled condition is loading || !serial.trim()), and regex validation
(SERIAL_REGEX) runs only in handleSubmit, so update the test in devices.spec.ts
to assert the "Register Device" button is disabled initially, then enabled after
filling any non-empty serial (e.g., "SN1"), and remove the expectation that
"SN12" toggles enabled state; reference the submitBtn, the "Serial Number"
field, and the SERIAL_REGEX/handleSubmit behavior when making the change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Test expects wrong validation message.

The component's validation message is "Only letters, numbers, hyphens and underscores allowed (3–64 chars)" (per RegisterDeviceModal.tsx line 39), not "4–30 uppercase". This test will fail.

🐛 Proposed fix: Match actual validation message

   test("invalid serial shows validation error", async ({ page }) => {
     await page.getByRole("button", { name: "+ Register Device" }).click();
     await page.getByLabel("Serial Number").fill("AB!@#");
     await page.getByRole("button", { name: "Register Device" }).click();
     await expect(page.getByRole("alert")).toBeVisible();
-    await expect(page.getByRole("alert")).toContainText("4–30 uppercase");
+    await expect(page.getByRole("alert")).toContainText("3–64 chars");
   });

🤖 Prompt for AI Agents

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

In `@tests/e2e/devices.spec.ts` around lines 57 - 63, The test "invalid serial
shows validation error" expects the wrong message; update the expectation in
tests/e2e/devices.spec.ts (the test named invalid serial shows validation error)
to assert the actual component validation string from RegisterDeviceModal.tsx
(the message: "Only letters, numbers, hyphens and underscores allowed (3–64
chars)") instead of "4–30 uppercase" so the test matches the real validation
text.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the actual scope used in Auth0Provider
rg -n "scope" --type=ts -C3 -g '*Auth0Provider*'

Repository: pahuldeepp/GrainGuard-

Length of output: 1424

---

🏁 Script executed:

#!/bin/bash
# First, locate the mockAuth.ts file
fd -t f -n "mockAuth.ts" 

# Then examine its content around lines 27-33, 38, and 98
cat -n tests/e2e/fixtures/mockAuth.ts | sed -n '20,110p'

Repository: pahuldeepp/GrainGuard-

Length of output: 4033

---

Auth0 cache key scope mismatch will cause silent auth failure in tests.

The cache key and mock responses use "openid profile email" but Auth0ProviderWithNavigate.tsx (line 12) defaults to "openid profile email offline_access". Since the Auth0 SPA SDK uses scope as part of the cache lookup key, this mismatch means the SDK won't find the cached token and will fail authentication.

Update lines 32, 38, and 98 to include offline_access:

Proposed fix

-const AUTH0_CACHE_KEY = `@@auth0spajs@@::${CLIENT_ID}::${AUDIENCE}::openid profile email`;
+const AUTH0_CACHE_KEY = `@@auth0spajs@@::${CLIENT_ID}::${AUDIENCE}::openid profile email offline_access`;

-    scope:         "openid profile email",
+    scope:         "openid profile email offline_access",

-        scope:         "openid profile email",
+        scope:         "openid profile email offline_access",

🧰 Tools

🪛 Betterleaks (1.1.1)

[high] 30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents

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

In `@tests/e2e/fixtures/mockAuth.ts` around lines 27 - 33, The Auth0 localStorage
cache key and mock responses use "openid profile email" but the app's Auth0
provider includes "offline_access", so update the AUTH0_CACHE_KEY constant and
any mock scope strings to include "offline_access" (i.e., change AUTH0_CACHE_KEY
built from CLIENT_ID and AUDIENCE to end with "::openid profile email
offline_access") and update the matching mock response scope values used
elsewhere in this file (the other scope constants/strings in the same module) so
the Auth0 SPA SDK can find the cached token; search for AUTH0_CACHE_KEY, any
scope string variables, and the mock token/response objects in this file and add
"offline_access" to their scope lists.