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

Inline comments:
In @.github/workflows/chaos.yml:
- Around line 101-107: The upload step "Upload chaos logs" can fail when
tests/chaos/results/ doesn't exist; update that step (the one using
actions/upload-artifact@v4 named "Upload chaos logs") to either add the
parameter if-no-files-found: ignore to the with: block or add a short preceding
step that ensures the directory exists (e.g., run: mkdir -p
tests/chaos/results/) so the artifact upload never errors out when no files are
present.
- Around line 45-48: The workflow uses unquoted $HOME in shell commands (mkdir
-p $HOME/.kube, redirection to > $HOME/.kube/config, chmod 600
$HOME/.kube/config) which can cause word-splitting; update those occurrences to
use quoted expansions (e.g., "$HOME/.kube" and "$HOME/.kube/config") so mkdir,
the echo/base64 redirection, and chmod operate on the intended single path even
if $HOME contains spaces or special chars.
- Around line 97-99: The "Run — network-partition" workflow step is missing the
NAMESPACE environment variable; update that step to pass NAMESPACE into the
job's environment (matching how the "pod-kill" step does it) by adding an env
entry that sets NAMESPACE from the workflow input (e.g., use the
github.event.inputs namespace value) so Chaos Toolkit can read the target
namespace.
- Around line 70-72: The "Run — pod-kill" step is missing an explicit NAMESPACE
environment variable (and the same applies to the "network-partition" step);
update those steps to add an env block that passes the job/workflow NAMESPACE
into the step (e.g., add env: NAMESPACE: ${{ env.NAMESPACE }} or the appropriate
input/var interpolation used elsewhere) so the pod-kill.yaml experiment can read
NAMESPACE at runtime.

In @.github/workflows/perf.yml:
- Around line 82-87: The workflow currently runs k6 with
BFF_URL=http://localhost:4000 but never starts the BFF service and the test file
scripts/load-tests/performance-budget.js defaults to port 8086; add steps
mirroring the gateway startup (install dependencies, build/start BFF in
background and wait for readiness) before the "Run performance budget" job,
ensure the BFF process is started on port 8086 (or update BFF_URL to match the
port used in performance-budget.js), and export the correct BFF_URL (e.g.,
http://localhost:8086) so k6 can reach the running BFF during tests.
- Around line 17-37: The postgres and redis service blocks in the workflow lack
required ports mappings, so add a ports key under the postgres service mapping
host 5432 to container 5432 and under the redis service mapping host 6379 to
container 6379 (i.e., add ports: - "5432:5432" to the postgres block and ports:
- "6379:6379" to the redis block) so the runner can reach localhost:5432 and
localhost:6379 from the job.

In @.github/workflows/terraform.yml:
- Around line 83-117: The current Terraform "apply" job (job name: apply) is
configured with environment: dev and only applies on push to master, so
production is never applied; either add an inline comment above the apply job
explaining the intended prod deployment/approval process, or add a new separate
apply job (e.g., apply_prod) that mirrors the existing apply steps but sets
environment: prod, a stricter approval gate (GitHub environment protection), and
the correct working-directory/TF vars for prod; ensure both jobs use the same
configure-aws-credentials, setup-terraform, terraform init and terraform apply
steps and that conditions (if: github.ref == 'refs/heads/master') and secrets
(TF_VAR_db_password or prod secret) are adjusted appropriately.
- Around line 13-15: The YAML env block has an extra space after the colon for
the AWS_REGION key causing formatting lint errors; update the env mapping (keys
TF_VERSION and AWS_REGION) to use a single colon-space separator (e.g.,
AWS_REGION: "us-east-1") so there are no extra spaces after the colon and the
YAML is properly formatted.
- Around line 23-25: The CI matrix under strategy.matrix.env currently lists
only [dev, prod] but the repo adds infra/terraform/environments/dr/, so update
the matrix to include "dr" (i.e., change strategy.matrix.env to [dev, dr, prod])
or, if intentional, add a brief comment/documentation in the workflow explaining
why DR is excluded and/or add conditional logic to skip DR; update the
workflow's strategy.matrix.env entry accordingly to ensure DR gets planned or
document the exclusion.
- Around line 44-47: Add Terraform plugin caching to the workflow: insert a
cache step that preserves the Terraform plugin directory between runs and set
the TF_PLUGIN_CACHE_DIR environment variable so the hashicorp/setup-terraform
step uses the cache; reference the existing step name "Set up Terraform" and the
action "hashicorp/setup-terraform@v3" when locating where to add the cache and
env var, and ensure the cache key uses TF version and operating system to avoid
conflicts.

In `@apps/gateway/src/middleware/planEnforcement.ts`:
- Around line 185-208: The middleware uses non-null assertion req.user! (e.g.,
in enforceDeviceQuota) which can throw if authMiddleware wasn't run; change to
defensively check req.user?.tenantId like requireActiveSubscription does: if
missing, return res.status(401).json({ error: "unauthenticated" }) (or call next
with an auth error) and avoid proceeding. Replace all occurrences of req.user!
in enforceDeviceQuota, enforceBulkDeviceQuota, enforceAlertRuleQuota, and
requireFeature with an optional check and early unauthorized response before
using tenantId or feature info.

In `@apps/gateway/src/routes/billing.ts`:
- Line 51: Replace the unsafe any cast on (sub as any).current_period_end with a
proper type assertion or type guard: treat sub as a Stripe.Subscription extended
with the optional current_period_end property (e.g., use a type like
Stripe.Subscription & { current_period_end?: number }) or perform a runtime
check before reading the field; alternatively update the Stripe SDK types if
they are out-of-date so current_period_end is available on the
Stripe.Subscription type—make this change where currentPeriodEnd is assigned
from sub.

In `@apps/gateway/src/routes/teamMembers.ts`:
- Around line 79-92: The handler currently queries tenants and only calls
inviteToOrg when rows[0]?.auth0_org_id exists but still returns invited: true
even when the org id is missing; change the logic so invited is only true when
an Auth0 invite was actually attempted and succeeded. Specifically, in the
pool.query.then(...) block (where inviteToOrg is called), return a boolean or
resolve a value indicating success from inviteToOrg and set the response's
invited flag from that result; if rows[0]?.auth0_org_id is falsy, explicitly set
invited: false (or return false) and avoid reporting invited: true. Ensure you
handle promise rejection from inviteToOrg (the existing .catch) so failures
produce invited: false and only a successful inviteToOrg result yields invited:
true.

In `@apps/saga-orchestrator/internal/repository/postgres_saga_repository.go`:
- Around line 17-23: PostgresSagaRepository.IsEventProcessed and
PostgresSagaRepository.MarkEventProcessed are calling pgxpool methods with the
raw context and can block indefinitely; update both methods to call
withTimeout(ctx) and use the returned context (and defer the cancel func) when
invoking pool.Query/QueryRow/Exec so the same dbQueryTimeout from withTimeout is
applied; locate the calls inside the methods (references:
PostgresSagaRepository.IsEventProcessed,
PostgresSagaRepository.MarkEventProcessed) and replace the direct use of ctx
with the ctx returned by withTimeout, ensuring you defer cancel() immediately
after obtaining it.

In `@apps/search-indexer/main.py`:
- Around line 74-90: The telemetry upsert unconditionally sets status to
"active", causing a race with index_device which sets "registered"; change the
self.es.update call to preserve a higher-priority status by using an
Elasticsearch scripted update/upsert: in the update invoked by self.es.update
(for DEVICE_INDEX) add a script that only sets ctx._source.status =
params.status when the existing ctx._source.status is not "registered" (or is
missing), and pass params.status="active" from the telemetry path; this ensures
index_device's "registered" wins if it has already been set while still
upserting telemetry fields (temperature, humidity, recorded_at).

In `@go.mod`:
- Line 24: Update the gRPC dependency to patch the authorization bypass: change
the google.golang.org/grpc module version from v1.79.2 to v1.79.3 in go.mod,
then run go get google.golang.org/grpc@v1.79.3 (or go mod tidy) to update go.sum
and vendor files; ensure any CI build uses the updated module cache so the
project imports the patched google.golang.org/grpc v1.79.3.
- Line 3: Update the Go toolchain version in go.mod by replacing the current "go
1.25.0" directive with "go 1.26.1" (edit the go.mod entry containing the literal
go 1.25.0), then run go mod tidy and rebuild/tests to ensure compatibility and
update any CI/job configurations that pin the older toolchain to use Go 1.26.1.
- Around line 21-23: Update the otlptracegrpc module version to match the other
OpenTelemetry modules: change the go.mod entry for
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" from v1.41.0
to v1.42.0 so it aligns with "go.opentelemetry.io/otel" and
"go.opentelemetry.io/otel/sdk" and avoid version mismatch issues.
- Line 9: Add a brief rationale to the PR description explaining why the go.mod
versions were downgraded: state the specific reasons for choosing
github.com/golang-migrate/migrate/v4 v4.18.3 and
github.com/testcontainers/testcontainers-go v0.37.0 (e.g., compatibility with
our Go toolchain or other transitive deps, observed runtime/test failures,
performance/regression issues, or CI constraints), reference the exact modules
named (github.com/golang-migrate/migrate/v4 and
github.com/testcontainers/testcontainers-go) and mention any investigation
performed (logs, failing tests, or dependency conflicts) so reviewers can
understand and reproduce the decision.

In `@infra/kafka/mirrormaker2-connector.json`:
- Around line 22-28: The connector currently re-uses
${KAFKA_USERNAME}/${KAFKA_PASSWORD} for both source and target in the
"source.cluster.sasl.jaas.config" and "target.cluster.sasl.jaas.config" fields;
change these to use distinct environment variables (e.g.,
${SOURCE_KAFKA_USERNAME}, ${SOURCE_KAFKA_PASSWORD} and ${TARGET_KAFKA_USERNAME},
${TARGET_KAFKA_PASSWORD}) and update any deployment/secret manifests to
provision two separate secrets/credentials and map them to those new env names
so each cluster has its own credentials.
- Around line 30-37: Add explicit producer and consumer tuning entries to the
connector JSON to improve cross-region reliability: include producer settings
such as "producer.retries" (e.g., high value), "producer.acks" ("all"),
"producer.max.in.flight.requests.per.connection" (1 or 5 depending on
idempotence), "producer.request.timeout.ms", "producer.retry.backoff.ms", and
"producer.delivery.timeout.ms", and consumer settings like
"consumer.request.timeout.ms" and "consumer.max.poll.interval.ms"; place them
alongside the existing keys (e.g., near "checkpoints.topic.replication.factor"
and "emit.heartbeats.interval.seconds") so the mirror-maker connector will use
these tuned producer/consumer properties during cross-region replication.

In `@infra/terraform/backend.tf`:
- Around line 4-6: Update the bootstrap comments around the existing "aws s3api
create-bucket" example (bucket name "grainguard-terraform-state") to include a
follow-up "aws s3api put-public-access-block" command that sets BlockPublicAcls,
IgnorePublicAcls, BlockPublicPolicy and RestrictPublicBuckets to true for that
bucket, so the state bucket is blocked from public access; include the bucket
identifier "grainguard-terraform-state" in the command example and a short note
that this is required for Terraform state security.

In `@infra/terraform/environments/dr/main.tf`:
- Line 15: Extract the hardcoded CIDR into a Terraform local by adding a locals
block (e.g., locals { vpc_cidr = "10.2.0.0/16" }) and replace the four
occurrences of the literal "10.2.0.0/16" with local.vpc_cidr in this file; also
update module calls or variables for aurora_dr, redis_dr, and msk_dr to consume
the local (pass local.vpc_cidr to any module input like vpc_cidr or change
module variable defaults) so all references use the single local.vpc_cidr value.
- Around line 5-9: Move the inline variable declarations out of main.tf into a
new variables.tf to match prod structure: create a variables.tf containing the
variable blocks for aurora_global_cluster_id, redis_global_datastore_id,
db_password (preserve sensitive = true), project (preserve default =
"grainguard"), and aws_region (preserve default = "us-west-2"), then remove
those variable blocks from main.tf so main.tf only contains resources and module
calls; ensure variable names and attributes remain identical to avoid breaking
references.
- Around line 67-69: The output name dr_redis_primary_endpoint is misleading for
a DR (secondary) cluster; update the Terraform output to either rename it to
dr_redis_endpoint (or dr_redis_writer_endpoint) and/or add a descriptive text
field that clarifies it is the cluster's write (primary) endpoint for the DR
cluster; locate the output definition referencing
module.redis_dr.primary_endpoint and change the output name and/or add
description = "DR Redis cluster write/primary endpoint (used for failover)" so
the intent is explicit.

In `@infra/terraform/environments/prod/main.tf`:
- Line 8: Extract the hardcoded CIDR into a Terraform local and replace the
literal occurrences: create a local named vpc_cidr (e.g., local.vpc_cidr) and
use that value instead of the string "10.1.0.0/16" in the VPC definition and in
the module blocks referencing aurora, redis, and msk; update each module input
(the existing vpc_cidr parameter) to reference local.vpc_cidr so future changes
are made in one place.
- Around line 57-60: Add descriptive "description" attributes to the Terraform
outputs so external consumers understand their purpose: update the outputs
aurora_global_cluster_id, redis_global_datastore_id, and msk_bootstrap_brokers
to include a brief description string (e.g., describing that
aurora_global_cluster_id is the Global Cluster ID for cross-region Aurora,
redis_global_datastore_id is the Global Datastore ID for Redis replication, and
msk_bootstrap_brokers is the bootstrap broker list for the MSK cluster) using
the Terraform output "description" field.

In `@infra/terraform/environments/prod/variables.tf`:
- Around line 1-3: Add descriptive text and validation to the Terraform
variables: add a description for variable "project" explaining its purpose, a
description for "aws_region" plus a validation block that restricts values to
the allowed AWS regions (e.g., a list of accepted region strings) and provides a
helpful error message, and a description for the sensitive variable
"db_password"; reference the variables by name ("project", "aws_region",
"db_password") and implement the validation under "aws_region" using Terraform's
validation/condition/message fields.

In `@infra/terraform/modules/aurora-global/main.tf`:
- Around line 107-108: The module enables Enhanced Monitoring by setting
monitoring_interval = 60 but does not provide a monitoring role ARN; add a new
variable monitoring_role_arn (type string, default ""), and update the RDS
instance resource to pass monitoring_role_arn = var.monitoring_role_arn != "" ?
var.monitoring_role_arn : null so AWS receives the required IAM role for
Enhanced Monitoring; alternatively set monitoring_interval = 0 to disable
Enhanced Monitoring if you don't want to supply a role.

In `@infra/terraform/modules/elasticache-global/main.tf`:
- Line 14: The variable block for node_type uses invalid HCL semicolon
separators; update the variable "node_type" declaration so its attributes use
newline-separated assignments (remove the semicolon between type and default)
within the variable "node_type" block to conform to HCL syntax, e.g. place type
= "string" and default = "cache.r6g.large" on separate lines inside the variable
"node_type" block.
- Line 90: The output "global_datastore_id" uses a redundant nested ternary:
replace the conditional expression so it returns var.global_datastore_id when
var.is_secondary is true, otherwise return the id from
aws_elasticache_global_replication_group.this[0].id directly; update the
expression referencing output "global_datastore_id", var.is_secondary,
var.global_datastore_id and aws_elasticache_global_replication_group.this to
remove the length(... ) > 0 check.

In `@Makefile`:
- Around line 72-73: The Makefile target test-e2e is invoking Playwright in the
wrong directory (it runs `cd apps/dashboard && npx playwright test`) so it
misses the new comprehensive tests; update the target to run Playwright from the
tests/e2e directory instead by changing the command to `cd tests/e2e && npx
playwright test` (keep the target name test-e2e and the use of `npx playwright
test` so CI/local behavior matches the GitHub Actions e2e workflow).
- Around line 149-157: The health Makefile target masks curl failures because
the pipeline exit status is from jq; wrap each piped probe in a shell that
enables pipefail so curl errors propagate — replace lines like `@curl -sf
http://localhost:3000/health | jq . || echo "Gateway: DOWN"` with a command that
runs under bash with pipefail, e.g. `@bash -c 'set -o pipefail; curl -sf
http://localhost:3000/health | jq .' || echo "Gateway: DOWN"`, and do the same
for the readiness, BFF (http://localhost:4000/health) and Ingest
(http://localhost:3001/health) checks in the health target so the || echo
fallback runs on transport failures.

In `@scripts/load-tests/performance-budget.js`:
- Around line 65-68: COMMON_HEADERS currently always includes Authorization:
`Bearer ${JWT}`, which yields "Bearer " when JWT is empty; change the header
construction so Authorization is only set when JWT is non-empty (e.g.,
conditionally add Authorization to COMMON_HEADERS when JWT truthy) to avoid
sending a malformed "Bearer " value; update the code that references
COMMON_HEADERS to work with the conditional header (symbol: COMMON_HEADERS,
variable: JWT).

In `@tests/chaos/network-partition.yaml`:
- Around line 22-24: The test currently only checks rollout health via the
existing probe named "telemetry-service-healthy" and sleeps during post-actions,
but does not assert Kafka consumer lag; add explicit lag measurements and
assertions both before the partition and after recovery by introducing probes
(e.g., "kafka-consumer-lag-before" and "kafka-consumer-lag-after") that query
the consumer lag metric for the telemetry topic/consumer group and fail if lag
exceeds the acceptable threshold, and wire these probes into the probes list and
post-actions so the workflow records and enforces low lag pre-fault and
successful recovery post-fault (reuse the existing "telemetry-service-healthy"
probe for rollout checks but ensure the new lag probes run at the appropriate
stages).
- Around line 60-64: Remove the unsupported "--stdin" argument from the kubectl
apply arguments: in the args sequence that currently contains "apply", "-f",
"-", and "--stdin", delete the "--stdin" entry and keep "apply", "-f", "-" so
kubectl reads the manifest from stdin; leave the existing stdin: | block intact
so the manifest is still piped into kubectl.

In `@tests/chaos/projection-lag.sh`:
- Around line 78-81: The test currently only warns when the alert_fired flag is
false (alert never fired) which permits false-positive CI; update the logic
around alert_fired to fail the test (exit non-zero) when alerts do not fire
unless an explicit toggle is set: add a STRICT_ALERT_CHECK (or similar)
environment variable check and if STRICT_ALERT_CHECK is true (or set to "1")
then call a hard-fail (exit 1) when alert_fired is false, otherwise keep the
existing warn "ProjectionLagHigh alert did NOT fire..." behavior; reference the
alert_fired variable and the warn function to locate the code and implement the
conditional exit.
- Around line 57-87: The script unconditionally scales read-model-builder to 0
then back to 1, risking leaving the deployment down if the script exits early;
capture the current replica count before scaling (e.g., query kubectl get
deployment/read-model-builder -n "$NAMESPACE" -o jsonpath='{.spec.replicas}')
into a variable like original_replicas, set a trap on EXIT (or ERR) that always
restores that count using kubectl scale and waits with kubectl rollout status,
and update the final restore logic to use original_replicas instead of a
hard-coded 1; reference the existing kubectl scale/deployment/read-model-builder
commands, the NAMESPACE variable, and the later kubectl rollout status call when
adding the trap and restore logic.

In `@tests/chaos/redis-outage.sh`:
- Around line 77-79: The panic_count log window currently uses
--since="${OUTAGE_SECONDS}s" in the kubectl logs call (captured into the
panic_count variable) but that flag is evaluated at the time the command runs
(mid-outage), so it actually looks further back than the outage start; update
the nearby comment to explicitly state this behavior and the intent (i.e., we
intentionally allow a larger lookback to capture early panic messages) or, if
you prefer strict outage-only logs, change the approach to record a timestamp at
outage start and use --since-time instead; reference the panic_count assignment,
the kubectl logs invocation, and the OUTAGE_SECONDS/NAMESPACE variables when
making the comment or code change.
- Around line 84-85: The sleep call can receive a negative value when
OUTAGE_SECONDS < 15; change the block around the log/sleep to compute a
non-negative wait value (e.g., remaining=$(( OUTAGE_SECONDS > 15 ?
OUTAGE_SECONDS - 15 : 0 ))), then call sleep "$remaining" (or skip sleeping if
remaining is 0) and update the log message to reflect the actual waited seconds;
modify the lines referencing OUTAGE_SECONDS and sleep to use this safe remaining
variable.
- Around line 51-55: The script scales Redis to 0 but has no cleanup on
interruption; capture the current replica count into a variable (e.g.,
PREV_REPLICAS) using kubectl get for "$REDIS_DEPLOY" in "$NAMESPACE" before
scaling, add a restore function (e.g., restore_redis) that scales the deployment
back to PREV_REPLICAS (or 1 if capture fails) and logs the action, and register
a trap on EXIT (trap 'restore_redis' EXIT) so Redis is restored whether the
script completes or is interrupted; reference the REDIS_DEPLOY and NAMESPACE
variables and the kubectl scale calls to locate where to add the capture,
restore function, and trap.

In `@tests/e2e/devices.spec.ts`:
- Around line 69-73: The test "Refresh button triggers refetch" only checks
visibility and clickability of refreshBtn and doesn't verify a refetch occurred;
update the test to await a network request/response triggered by the click
(e.g., using page.waitForResponse or page.waitForRequest matching the devices
API endpoint) or assert a post-refresh UI change (for example a changed
timestamp, updated device list item, or a refreshed request counter) after
clicking refreshBtn so the test actually validates the refetch side-effect.
- Around line 30-34: The test "modal closes on backdrop click" uses a hard-coded
page.mouse.click(10, 10); replace this brittle coordinate click with a
bounds-based or semantic backdrop click: obtain the dialog locator from
page.getByRole("dialog") and compute its bounding box to click a point outside
those bounds (or, if available, target a backdrop overlay locator such as
page.locator('.backdrop') or page.getByTestId('backdrop') and click it); update
the test body in the "modal closes on backdrop click" test to perform the
outside-click via the dialog's bounding box or the backdrop locator instead of
fixed coordinates so the dismissal is robust to layout changes.

---

Outside diff comments:
In `@apps/bff/src/datasources/redis.ts`:
- Line 47: The set method currently declares the value parameter as any — change
it to a safe type by using either unknown or a generic type parameter: replace
async set(key: string, value: any, ttlSeconds: number): Promise<void> with
either async set(key: string, value: unknown, ttlSeconds: number): Promise<void>
or async set<T>(key: string, value: T, ttlSeconds: number): Promise<void>; then
update the function body to perform explicit serialization/type-guards or casts
where necessary and adjust call sites to provide the generic type or to handle
unknown (e.g., JSON.stringify for storage or validate before use) so the method
no longer accepts implicit any.
- Line 44: The map callback currently uses the any type; replace (r: any) with a
specific Redis response type such as (r: string | null | undefined) and, if the
surrounding function can return typed objects, make the function generic (e.g.,
<T>) and cast JSON.parse(r) to T (or T | null) so the line becomes
results.map((r: string | null | undefined) => r ? JSON.parse(r) as T : null),
updating the function signature/return type accordingly to remove any usage of
any.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Line 98: The catch block in circuitBreaker.ts currently types the error as
`any` (catch (err: any)); change it to `unknown` and add a narrow type guard
before using error properties: update the catch parameter in the
CircuitBreaker-related function (the catch in the circuitBreaker.ts file) to
`err: unknown`, then check e.g. `if (err instanceof Error)` or test for `typeof
err === 'object' && err !== null && 'message' in err` before accessing
message/stack; adjust any logging or rethrowing to use the guarded value to
satisfy ESLint.

In `@apps/gateway/src/lib/audit.ts`:
- Around line 26-35: The union type AuditEventType contains a duplicated member
"device.registered"; remove the redundant entry so that "device.registered"
appears only once in the AuditEventType union (locate the union declaration in
apps/gateway/src/lib/audit.ts and remove the earlier/repeated
"device.registered" line), ensuring the list still includes all unique event
strings like "device.created", "device.creation_failed",
"webhook_endpoint.created", "api_key.created", "api_key.revoked", and
"device.deleted".

In `@apps/gateway/src/routes/devicesImport.ts`:
- Around line 117-134: The quota check in the devices import route must account
for the incoming CSV device count: update the call to checkDeviceQuota in
devicesImport.ts to pass the parsed total number of devices (the variable named
total from the CSV parsing) and update the planEnforcement.checkDeviceQuota
signature/logic to accept an optional incomingCount/total parameter and validate
currentCount + incomingCount <= maxDevices (instead of just currentCount >=
maxDevices); keep the same return shape (allowed, message, currentCount,
maxDevices, plan) and ensure the devicesImport.ts code treats allowed=false as
before and aborts before starting the import.

In `@apps/gateway/src/server.ts`:
- Around line 288-294: The audit call at logAuditEvent for eventType
"device.creation_failed" currently persists the raw exception via meta: { error:
String(err) } — replace this with a stable error classification/code instead of
the full error text to avoid leaking internal details; e.g., derive a short
errorCode from the thrown error (use err.name or map errors in a new helper like
classifyError(err) or set errorCode = "validation_error" | "db_error" |
"unknown_error") and store meta: { serialNumber: req.body?.serialNumber,
errorCode } (keep any sensitive details out of meta and do not persist
String(err)).

In `@apps/gateway/src/services/device.ts`:
- Around line 39-45: The current credentials selection silently falls back to
grpc.credentials.createInsecure() when fs.existsSync("/certs/ca.crt") is false;
change this to be environment-aware by checking process.env.NODE_ENV (or another
env var you use for production). If in production and the certs are missing,
throw a descriptive error (e.g., "Missing TLS certs: /certs/ca.crt") instead of
returning insecure credentials; otherwise (non-production) call
grpc.credentials.createInsecure() but emit a clear warning via your logger.
Update the credentials assignment logic around fs.existsSync,
grpc.credentials.createInsecure, and grpc.credentials.createSsl so it either
throws in production or logs a warning then falls back in non-production.

In `@apps/read-model-builder/internal/projection/telemetry_projection.go`:
- Around line 183-188: There is a duplicated nil-check for redisClient that
immediately returns nil twice; remove the redundant second "if redisClient ==
nil { return nil }" so only one early return remains, keeping the surrounding
logic intact (look for the redisClient variable and the function/method
containing those consecutive checks in telemetry_projection.go and delete the
duplicate check).
- Around line 300-307: The loop that builds newEventIDs iterates over rows but
never checks rows.Err() after calling rows.Close(); add a rowsErr := rows.Err()
check immediately after rows.Close() and handle/return/log the error the same
way as the existing check at line 380 (i.e., propagate or log the error
consistent with the function's error handling), referencing the same variables
(rows, newEventIDs) so any iteration/scan errors are caught.

---

Duplicate comments:
In `@apps/bff/src/datasources/redis.ts`:
- Around line 39-45: getMany currently uses client.multi() and pipeline.exec()
(client.multi(), pipeline.get(), pipeline.exec()) which will trigger CROSSSLOT
errors in Redis Cluster when keys span multiple slots; change getMany to perform
parallel GETs instead (e.g., map keys to client.get(key) and await Promise.all)
then JSON.parse each non-null result, preserving the early return for empty keys
and the return type Promise<(T|null)[]>.
- Around line 8-14: The TypeScript error comes from calling createCluster({
nodes }) — node-redis v4 expects rootNodes, not nodes; update the call to
createCluster({ rootNodes: nodes }) where nodes is the array derived from
REDIS_CLUSTER_NODES (keep the host and numeric port parsing logic), so
createCluster receives the correct property and resolves TS2353 for the
createCluster call.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Line 158: Calling this.syncToRedis() creates a floating promise; update the
call site to either await this.syncToRedis() (make the caller async if
necessary) or attach a .catch(...) to handle errors explicitly so failures are
not swallowed — locate the call to syncToRedis() and replace the bare invocation
with an awaited call or add a .catch that logs/handles errors using the existing
logger in the surrounding class/method.
- Line 143: The call to syncToRedis() is a floating promise and must be handled
to avoid unhandled rejections; update the code where this.syncToRedis() is
invoked (in the circuit breaker class/method) to either await the call if the
enclosing function is async or append a .catch(...) to handle errors (log with
the existing logger or swallow as appropriate), ensuring any rejection from
syncToRedis() is caught.
- Line 122: The call to syncToRedis() is a floating/unawaited promise; change
the call site (where syncToRedis() is invoked) to explicitly handle the promise
by appending .catch(...) to capture and log/handle errors (or await it if the
caller is async and should wait); reference the syncToRedis() invocation and
ensure the handler uses the module logger or processLogger to record the error
so the promise rejection is not unobserved.
- Around line 40-67: The cache keys in syncFromRedis and syncToRedis (used as
`cb:${this.name}`) omit tenantId; either make these keys tenant-scoped or
explicitly document the tenant-agnostic intent. Fix: if circuit breaker state
must be isolated per tenant, include tenantId in the key (e.g.,
`cb:${tenantId}:${this.name}`) where tenantId is obtained from the
class/context, updating both syncFromRedis and syncToRedis to use the same
composite key; otherwise add a clear code comment and/or README entry near the
CircuitBreaker class constructor (or the methods syncFromRedis/syncToRedis)
stating that the breaker is intentionally global/shared across tenants and why.
Ensure the unique symbols mentioned (syncFromRedis, syncToRedis, this.name /
constructor) are updated so tests and cache usage remain consistent.

In `@apps/jobs-worker/src/connection.ts`:
- Around line 23-28: The connection error/close handlers currently only log and
leave stale globals, so update the conn.on("error", ...) and conn.on("close",
...) callbacks to clear the module-level connection and channel references
(e.g., set conn = null and ch = null or the equivalents used in this file) and
then invoke the existing reconnect/health-failure path (the same flow used when
a reconnect is needed or when health check fails) so getChannel() cannot return
a zombie channel; specifically modify the handlers around conn and ch in
connection.ts and call the module's reconnection trigger or health-failure
function immediately after clearing the references.

In `@apps/read-model-builder/cmd/main.go`:
- Around line 87-95: clusterNodes is split and appended into addrs without
filtering empty entries; update the loop that processes clusterNodes (the
strings.Split(clusterNodes, ",") iteration that appends to addrs) to trim each
entry and skip/apply continue for any resulting empty string so only non-empty
addresses are added, then log the count using len(addrs) to reflect the filtered
list; ensure the single-node branch (using getenv("REDIS_ADDR", "redis:6379"))
remains unchanged.

In `@apps/saga-orchestrator/internal/orchestrator/provision_saga.go`:
- Around line 132-135: The error message for the JSON marshal branch incorrectly
references "tenant.detach_device" while the code is marshaling the
quota.allocate_device command; update the fmt.Errorf call (the one returning
fmt.Errorf("marshal tenant.detach_device command: %w", detachErr)) to accurately
name the command being marshaled (e.g., "marshal quota.allocate_device command:
%w") or derive the command name from the cmd object; locate the marshal call
using the variables cmdBytes and detachErr in provision_saga.go to apply the
fix.

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

Inline comments:
In @.github/workflows/perf.yml:
- Around line 82-102: The BFF fails at module load because the env block in the
"Start BFF in background" step is missing required JWT configuration; add
JWKS_URL, JWT_ISSUER, and JWT_AUDIENCE environment variables to that step so
apps/bff's startup code (server.ts) can initialize; set JWKS_URL to a well-known
JWKS endpoint derived from AUTH0_DOMAIN (e.g.
https://placeholder.auth0.com/.well-known/jwks.json), set JWT_ISSUER to the
auth0 issuer URL (e.g. https://placeholder.auth0.com/), and set JWT_AUDIENCE to
match AUTH0_AUDIENCE (placeholder) or appropriate audience.
- Around line 114-119: The perf step never exercises secured endpoints because
scripts/load-tests/performance-budget.js skips /devices/:id/latest and /graphql
when JWT is empty; update the "Run performance budget" job so it provides a
non-empty JWT env var (e.g., set JWT via an action that obtains a test/service
token or inject a static test token) when invoking k6, ensuring the script sees
process.env.JWT and will exercise the protected routes; reference the Run
performance budget step and scripts/load-tests/performance-budget.js when making
the change.

In @.github/workflows/terraform.yml:
- Around line 1-11: Add a concurrency stanza to the Terraform GitHub Actions
jobs so plans for the same environment are queued instead of running
concurrently; under the Terraform job(s) in the workflow (the jobs block that
runs your plan/apply) add a concurrency key with a group that scopes to the
target environment/branch (for example use an expression like terraform-${{
github.ref }} or terraform-${{ github.head_ref }} to uniquely identify the
environment) and set cancel-in-progress: false to ensure jobs are queued rather
than cancelled.
- Around line 114-117: The "Terraform Apply" step should re-run a plan to detect
drift before applying; change the step to first run `terraform plan -input=false
-out=tfplan` (or `terraform plan -detailed-exitcode` if you want to detect and
fail on changes) and then run `terraform apply -input=false tfplan` instead of
applying directly, so the workflow uses a freshly generated plan and can detect
unexpected state drift before making changes; update the step named "Terraform
Apply" to perform these two commands and handle non-zero detailed-exitcode if
you want the job to fail when drift is detected.

In `@apps/bff/src/datasources/redis.ts`:
- Around line 46-47: getMany() currently fires client.get for every key via
Promise.all (see getMany and its use from resolvers.ts with deviceIds), which
can overwhelm Redis; change getMany to limit concurrency by batching or using a
concurrency limiter (e.g., process keys in chunks of a defined BATCH_SIZE or use
a p-limit-like pattern) so only a fixed number of client.get calls run
concurrently, preserve input ordering when assembling results, and continue to
JSON.parse non-null values and return null for misses; add a configurable
constant (e.g., BATCH_SIZE or MAX_CONCURRENCY) and ensure tests still expect the
same return shape.
- Around line 9-17: When building rootNodes from REDIS_CLUSTER_NODES before
calling createCluster(), reject blank or malformed entries: split on commas,
filter out entries that are empty after trim, then for each segment parse host
and port and validate that host is non-empty and port parses to a finite integer
within 1–65535 (or use default 6379 only if port is absent). If validation fails
for any node, surface a clear error (throw or log + exit) instead of producing a
node with empty host or NaN port; then map the validated entries into the same
socket shape used by rootNodes. Ensure the check is applied where
REDIS_CLUSTER_NODES and rootNodes are defined so createCluster() only receives
valid nodes.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Around line 46-50: syncFromRedis currently only adopts remote state when
shared.failureCount > this.failureCount which prevents pods from converging
(e.g. a CLOSED write with failureCount 0 is ignored by OPEN pods). Change the
comparison to use a monotonic version/timestamp field (add and persist a
shared.lastUpdated/version) and in syncFromRedis compare shared.lastUpdated >
this.lastUpdated; when true, copy shared.state, shared.failureCount,
shared.lastFailureTime and set this.lastUpdated to shared.lastUpdated so all
pods converge deterministically.

In `@apps/gateway/src/lib/audit.ts`:
- Around line 26-29: The AuditEventType union in apps/gateway/src/lib/audit.ts
contains duplicate string literals ("device.created", "device.creation_failed",
"device.registered", "webhook_endpoint.created") that are already declared again
under the // Devices and // Webhooks sections; remove these top-level duplicates
so each audit event literal appears only once (leave the canonical declarations
in the // Devices and // Webhooks blocks), and verify the symbol AuditEventType
still compiles after the cleanup.

In `@apps/gateway/src/server.ts`:
- Around line 191-199: The audit write is currently fire-and-forget via
logAuditEvent, risking lost entries if the process exits; change the callers
(the device creation path where logAuditEvent is invoked and the other
occurrences around the device route) to await the promise returned by
logAuditEvent (ensure the enclosing handler function is async) so the request
handler waits for writeAuditLog to complete; do the same for the other instance
referenced (the block at 203-210) to ensure all audit writes are awaited and any
internal errors remain swallowed by writeAuditLog.

In `@apps/telemetry-service/migrations/000009_sso_alert_rules_bulk.down.sql`:
- Around line 6-9: The tenants SSO columns (auth0_org_id, sso_connection_id,
sso_connection_type) are declared in both migrations 000007_saas_columns and
000009_sso_alert_rules_bulk, causing ambiguous ownership—remove the column ADDs
and their DROPs from the 000007 migration files (e.g.,
000007_saas_columns.up.sql and its down.sql) so these columns are only
created/dropped in 000009_sso_alert_rules_bulk.up.sql/down.sql (ensure the
auth0_org_id index and its removal remain in 000009 and update any
comments/commit messages to reflect single ownership).

In `@apps/telemetry-service/migrations/000009_sso_alert_rules_bulk.up.sql`:
- Around line 4-7: This migration duplicates SSO column additions already
created in migration 000007_saas_columns.up.sql; remove the three ALTER TABLE
ADD COLUMN IF NOT EXISTS statements for auth0_org_id, sso_connection_id, and
sso_connection_type from 000009_sso_alert_rules_bulk.up.sql so the columns
remain owned by 000007; after removing, ensure ALTER TABLE tenants in 000009
only contains the non-SSO changes (or any other intended column additions) and
verify no subsequent migration logic or tests rely on 000009 to create those SSO
columns.
- Around line 43-45: Add non-negative CHECK constraints to the count columns to
enforce data integrity: update the table definition (or an ALTER TABLE in this
migration) to add CHECK constraints that ensure total_rows >= 0, success_rows >=
0 and error_rows >= 0 (you can add three separate constraints or a single
combined CHECK). Reference the column names total_rows, success_rows and
error_rows (and the surrounding CREATE TABLE/ALTER TABLE statement in this
migration) and ensure constraint names are unique and descriptive so the DB will
reject any negative counts at insert/update time.

In `@scripts/load-tests/performance-budget.js`:
- Around line 73-75: The /health probe failures aren't counted because
error_rate and total_errors are only updated inside the JWT-protected branches;
update the same counters when handling healthRes so failures are included. In
the block around healthRes (variable healthRes, check call that asserts r.status
=== 200, and gatewayP95.add) add logic to increment total_errors and update
error_rate when healthRes.status !== 200 (mirror the JWT-branch error handling),
and ensure any tags/metrics used for error aggregation are the same as in the
JWT branches so CI runs without JWT still contribute to the error budget.

In `@tests/chaos/kafka-consumer-pause.sh`:
- Around line 18-19: The script only lists CONSUMERS=("read-model-builder"
"cdc-transformer") so it misses the sibling consumer group
"read-model-builder-devices" started by the read-model-builder binary; update
the CONSUMERS array to include "read-model-builder-devices" and ensure any
checks/loops that use CONSUMERS and ORIGINAL_REPLICAS operate over both entries
so the pause/unpause verification waits for both read-model-builder and
read-model-builder-devices to catch up.
- Around line 29-35: The current_lag() function is summing the wrong awk field
(using $NF which is CLIENT-ID) and thus returns incorrect lag; update the awk
invocation inside current_lag() to sum column 5 (the LAG field) instead of $NF
so the function computes total lag correctly (replace the awk expression that
uses $NF with one that uses $5).

In `@tests/chaos/projection-lag.sh`:
- Around line 19-21: Increase the alert timing windows to avoid false failures:
update ALERT_WINDOW and RECOVERY_WINDOW (and any hardcoded checks in the same
file range ~lines 64-85) to values that account for pod shutdown, rollout
completion, and lag accumulation before the `ProjectionLagHigh` alert's `for:
2m` expires; specifically enlarge ALERT_WINDOW from 120s to a value >= (shutdown
+ rollout + 2m + one evaluation interval) and extend RECOVERY_WINDOW similarly,
and ensure STRICT_ALERT_CHECK logic still gates strict verification but uses
these larger windows so CI doesn't fail prematurely.
- Around line 39-42: The alert_firing() helper currently returns true for any
ProjectionLagHigh alert; update it to scope to the read-model-builder instance
by verifying the alert also contains the label for this experiment (e.g., the
job/instance/service label used in the Prometheus rule). Modify alert_firing to
fetch ${PROMETHEUS_URL}/api/v1/alerts and check both
'"alertname":"ProjectionLagHigh"' and the experiment-specific label (for example
'"job":"read-model-builder"' or the actual label name used in slo-rules.yaml) —
use a combined check (jq or two greps with &&) so only ProjectionLagHigh alerts
for read-model-builder make the function return success.
- Around line 31-36: The current_lag() function is summing the wrong awk field
(using $NF which is CLIENT-ID); update the awk expression in current_lag() to
sum the LAG column (column 6) instead: replace the awk script with one that uses
$6 (e.g., 'NR>1 { sum += $6 } END { print sum+0 }') so the function returns the
actual consumer lag used by the pre-check and recovery assertions.

In `@tests/chaos/redis-outage.sh`:
- Around line 117-122: The warm-up probe measures elapsed_ms but never asserts
it meets the 30s SLO; after calling http_check "Cache warm-up probe" and
computing elapsed_ms, add an assertion that elapsed_ms is <= 30000 (30,000 ms)
and fail the script if it exceeds the SLO (e.g., call an existing fail function
or exit 1). Update the block around t_start, http_check, t_end, elapsed_ms, and
log to perform the check and produce a clear error message including elapsed_ms
when the SLO is violated.
- Around line 27-41: The http_check function currently treats any HTTP 200 as
success but discards the response body; update http_check (the function using
GATEWAY_URL, GRAPHQL_QUERY and TEST_JWT) to capture the response body and HTTP
status, then parse the JSON to assert GraphQL-level success (e.g., ensure no
"errors" field and that "data" is present/non-null) before reporting success; if
JSON parsing fails or errors exist, log the body and return failure so the
script won't falsely report a successful GraphQL query.

---

Duplicate comments:
In @.github/workflows/terraform.yml:
- Around line 23-25: The CI matrix currently defines strategy.matrix.env with
only [dev, prod], which omits the DR environment required by prod's dependent
global resources; update the matrix definition (strategy → matrix → env) to
include "dr" so CI validates DR alongside prod, or alternatively add a clear
comment and documentation explaining why "dr" is intentionally excluded and
where/how DR infra is validated; ensure the change references the
strategy.matrix.env entry so reviewers can find it easily.

In `@apps/bff/src/lib/circuitBreaker.ts`:
- Around line 43-45: The Redis key `cb:${this.name}` used in cache.get (and the
corresponding cache.set usage around lines 59-63) is currently global; update
CircuitBreaker code to include the tenant identifier in the key (e.g.,
`cb:${tenantId}:${this.name}`) or, if the breaker must be truly global, move it
off the shared `cache` and use the dedicated global cache instance instead.
Locate the usages of cache.get and cache.set that reference `cb:${this.name}`
and modify them to read the tenantId from the request/context available to the
class (or switch to the app-level cache), ensuring every cache key includes
tenantId when using the shared cache.

In `@apps/gateway/src/routes/teamMembers.ts`:
- Around line 80-93: The current pool.query promise chain (in the teamMembers.ts
block that calls inviteToOrg with tenantId, email, memberRole) swallows failure
and missing auth0_org_id cases and always allows the route to return 201
invited:true; change this to return the actual invite outcome: when
rows[0]?.auth0_org_id is missing return an explicit failure outcome (e.g.,
invited: false), and when calling inviteToOrg await/propagate its result instead
of just calling it in a then; also catch inviteToOrg rejections and return
invited: false (or an object indicating error) so the route handler can use that
returned value to set the proper 201/4xx response and invited flag. Ensure the
change is made in the promise chain that currently invokes inviteToOrg
(referencing pool.query, rows[0].auth0_org_id, and inviteToOrg) so the caller
receives and uses the real outcome.

In `@apps/read-model-builder/cmd/main.go`:
- Around line 85-103: The code currently builds addrs from
getenv("REDIS_CLUSTER_NODES") but doesn't filter out empty/whitespace entries,
so a trailing comma can leave a single valid address and erroneously log "Redis
cluster mode" while NewUniversalClient will create a standalone client; update
the logic that builds addrs (used by clusterNodes, addrs, and
redis.NewUniversalClient) to trim and skip empty strings when splitting, then:
if you truly want cluster mode only when multiple valid addresses exist, require
len(addrs) > 1 to log cluster mode and call redis.NewClusterClient (or keep
NewUniversalClient but only in multi-node case), otherwise fall back to a
single-node path (use the single addr or redis.NewClient) and log single-node
mode; ensure you reference clusterNodes, addrs, redis.NewUniversalClient and
consider switching to redis.NewClusterClient when multiple valid addresses are
present.

In `@go.mod`:
- Around line 21-23: Update the OTel exporter module to match the other
OpenTelemetry modules: change the dependency version for
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc from v1.41.0 to
v1.42.0 in go.mod so all three modules (go.opentelemetry.io/otel,
go.opentelemetry.io/otel/sdk, and
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc) are aligned;
run go mod tidy afterwards and verify tracing initialization code in
libs/observability/tracing.go still builds and imports the otlptracegrpc package
without version conflicts.
- Line 24: The go.mod currently pins a vulnerable google.golang.org/grpc version
(google.golang.org/grpc v1.79.2); update the dependency to a patched release
immediately—the first patched release is v1.79.3, but you should upgrade to a
non-vulnerable release series (recommended: >= v1.80.0) and ensure your
go.mod/go.sum are updated and tests re-run to verify compatibility.

In `@tests/chaos/projection-lag.sh`:
- Around line 58-60: The script currently hard-codes scaling read-model-builder
to 0 and back to 1 without an EXIT trap; capture the current replica count first
(e.g., query deployment/read-model-builder replicas via kubectl to a variable),
install a trap on EXIT (and on INT/TERM) to restore that captured replica count,
then use that captured value when scaling back; apply the same pattern to the
second scaling block around lines 89-91 so both pauses restore the original
replica count even if the script is interrupted.

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

Inline comments:
In @.github/workflows/security.yml:
- Around line 77-86: The workflow step "Scan filesystem for secrets and
misconfigs" uses aquasecurity/trivy-action@0.28.0 with exit-code: "0", which
silences failures; update that step to fail the job on detected issues by
removing the forced "0" or setting exit-code to a non-zero value (e.g., "1") or
enabling the action's built-in fail-on-severity behavior so that the specified
severity list (CRITICAL,HIGH) will cause the job to fail; locate the step by its
name or the uses line and modify the exit-code/config to ensure secrets or
critical misconfigs block the workflow.
- Around line 52-57: The current "Ensure Trivy SARIF exists" step
unconditionally writes an empty SARIF file which masks real scan failures when
combined with exit-code: "0"; update this step to check the previous Trivy step
result and, if that step failed, generate a SARIF containing a
toolExecutionNotification/error entry (instead of an empty runs array) or fail
the job so failures are visible; specifically modify the logic that creates
"trivy-${{ matrix.service }}.sarif" to inspect the prior step outcome (or the
Trivy exit code) and write a minimal SARIF with an error notification
referencing Trivy, or exit non-zero to avoid silently uploading an empty SARIF.
- Around line 41-50: Update the Trivy action and tighten failure behavior:
change the action reference aquasecurity/trivy-action@0.28.0 to
aquasecurity/trivy-action@0.35.0 and set the workflow input exit-code from "0"
to "1" so CRITICAL/HIGH findings fail the job; after updating, rotate any CI/CD
secrets that may have been exposed by runs using the compromised versions
(0.0.1–0.34.2) to fully mitigate the supply-chain risk.