Skip to content

Release: DynamoDB idempotency keys, org-configurable TTLs, tw self-heals - #2577

Merged
johnyeocx merged 35 commits into
mainfrom
dev
Aug 4, 2026
Merged

Release: DynamoDB idempotency keys, org-configurable TTLs, tw self-heals#2577
johnyeocx merged 35 commits into
mainfrom
dev

Conversation

@johnyeocx

@johnyeocx johnyeocx commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Promotes dev to main. Highlights:

Idempotency (PRs #2551, #2552, #2569 + follow-ups)

  • Idempotency keys dual-written to Redis + DynamoDB; the idempotencyDynamoRead edge-config flag picks the conflict authority (awaited, 409 on duplicate, fail-open), the other store is a fire-and-forget mirror
  • Mirror write skipped when the authority rejects a duplicate, so a lagging mirror can't backfill a key with an extended TTL
  • Track/check idempotency layers restructured (misc/idempotency/actions/, external/{redis,aws/dynamodb}/idempotencyKeys/operations/, balances/idempotency/); sync + async single-lane track validate header/body keys at the request path; queue-replay dedup via Lua keys; batch track unified onto queueTrack
  • Org-configurable idempotency TTLs per route group (organizations.idempotency_config, max 30 days) threaded to Redis PX + Dynamo expiresAt; frontend setting hidden for now behind a flag
  • Sampled OTel spans for Dynamo claim/release

Fixes

Infra / CI

Migration

0059_square_captain_cross.sql: ALTER TABLE organizations ADD COLUMN idempotency_config jsonb; (already applied to prod DB)

🤖 Generated with Claude Code


Summary by cubic

Adds DynamoDB-backed idempotency keys with a Redis mirror and org-configurable TTLs for balances routes, plus self-healing worker boots. Also ships lazy pooled-balance resets and promotion reliability improvements.

  • New Features

    • Idempotency keys are dual-written to Redis and DynamoDB; idempotencyDynamoRead selects the conflict authority (mirror write is skipped on duplicates). Dynamo claim/release emit sampled OTel spans.
    • Local/prod setup: Docker dynamodb service and dynoxide in tw images; use bun dynamo setup|status to create tables and enable TTL on AWS.
    • Org-configurable idempotency TTLs per route group via organizations.idempotency_config (max 30d). All balances routes declare RouteGroup.Balances. Header/body keys are validated at accept; async and sync track share one queue with per-item request IDs; queue replay uses Lua dedup; duplicates 409.
    • tw worker boot self-heals: fetch dynoxide if missing, run bun install --frozen-lockfile, and bun db migrate; prunes stale remote refs on warm refresh.
    • Pooled balances: lazy resets via read-path and crons; next-cycle contribution promotion; cached granted patched via Lua.
  • Bug Fixes

    • Support plan-scoped reward coupons.
    • Hide internal OAuth scopes from MCP clients.
    • Redis: preserve dragonfly resize reconnect ownership and isolate availability probes.
    • OTel: stop NodeSDK falling back to localhost OTLP metrics exporter.
    • Pooled balances: guard promotion against concurrent transition writes; refresh in-memory granted when the guard trips.

Written for commit ed7f561. Summary will update on new commits.

Review in cubic

og2701 and others added 30 commits July 22, 2026 13:02
Co-authored-by: Owen <67282414+og2701@users.noreply.github.com>
Co-authored-by: Owen <67282414+og2701@users.noreply.github.com>
…ity flag

Idempotency keys are now always dual-written to Redis and DynamoDB; the
idempotencyDynamoRead miscellaneous-edge-config switch picks which store
decides conflicts (the other is a fire-and-forget mirror). Removes the 24h
Redis-only limitation and its cost.

- DynamoDB client + local-emulator table auto-create (external/aws/dynamodb)
- claim/release operations under idempotencyKeys/operations
- amazon/dynamodb-local wired into bun dw + dev:services; dynoxide static
  binary in the tw micro-VM images (no JVM)
- bun dynamo setup/status script for real-AWS table creation
- admin UI toggle for the authority flip

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e claims

Restructures the idempotency code into clear layers and fixes the track
dedup semantics agreed for single-lane + batch:

- misc/idempotency: actions/ split (check/release), shared withIdempotencyKey
  wrapper (claim -> run -> release) used by the header middleware and track
- redis claim/release moved to external/redis/idempotencyKeys/operations
  (mirrors the dynamo layout)
- balances/idempotency: track body key builder + queue replay key builders
  (renamed getTrackQueueIdempotencyKey), README documenting the layers
- single-lane track/trackTokens (sync + async): header + body keys always
  validated at accept time, claims kept (no release-on-queued hand-off);
  duplicate async body keys now 409 at accept
- queued messages carry validateTrackBodyIdempotencyKey so the worker only
  claims for batch items; queue replay dedup is the Lua-script key layer
- runAsyncTrack + runBatchTrack unified onto queueTrack; async-track queue
  sends batch via a second SQS send accumulator; addTasksToQueueBatch and
  AsyncTrackSqsBatcher deleted; sync fallback + async share one queue
  (TRACK_SQS_QUEUE_URL deprecated to a fallback)
- batch items get per-item requestIds, fixing same-customer items in one
  batch silently colliding on queue dedup (token batches deducted once)
- executeRedisDeductionV2 skips already-applied features and continues
  (partial replays resume); all-duplicate replays still 409 and the worker
  swallows them so messages are acked, never requeued
- AutumnInt.post accepts any 2xx (async 202s usable via .track())
- tests: track idempotency suite (header/body/check/queue), batch contract
  moved + aligned to the shipped 200 response, batch dedup + redelivery
  suites, multi-feature scenario builder

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dynamodb.claim_idempotency_key / release_idempotency_key client spans carry
table + outcome (claimed/duplicate/unavailable/released) so prod latency is
sliceable by outcome. Happy-path spans sample at
OTEL_DYNAMO_SUCCESS_SAMPLE_RATE (default 1%) via FilteringSpanProcessor;
duplicates, unavailability, and errors always export, and latency metrics
are recorded before the drop so percentiles stay complete.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
updateBalanceV2-async's SQS stub now answers SendMessageBatch with
Successful entries (async-queue sends route through the batcher) and
asserts the batch envelope; handleTrackTokens ctx logger gains info/debug
for the accept-time idempotency claim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… routes

Orgs can set how long idempotency keys block duplicates on the balances
endpoints (default 24h, 1h..30d), stored on organizations.idempotency_config
and read straight off ctx.org at claim time — no extra fetch or cache.

- RouteGroup as a general concept: declared per route via
  createRoute({ routeGroup }), resolved from middleware through a
  handler->group registry (hono matchedRoutes); all balances handlers
  declare RouteGroup.Balances
- resolveIdempotencyTtlMs(ctx, routeGroup) -> ttlMs threaded through
  checkIdempotencyKey into Redis PX and Dynamo expiresAt; idempotency
  actions now take ctx
- PATCH /organization/config accepts idempotency_config (validated
  up-front, 400 on out-of-range; all-or-nothing with config flags)
- Settings > Billing > Configuration: duration input + hours/days selector
- migration 0059: organizations.idempotency_config jsonb
- tests: resolver + registry + schema bounds units, ttlMs threading,
  integration verifying Redis PTTL and Dynamo expiresAt for configured
  and default TTLs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…roups

Mixed flag+TTL updates now commit in ONE UPDATE (no partial-commit window,
cache invalidation can't be skipped), and IdempotencyConfigSchema rejects
duplicate routeGroup entries instead of silently using the first match.
Validation errors surface the specific zod message (bounds/duplicates).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(idempotency): org-configurable idempotency key TTLs
fix(redis): preserve ioredis reconnect ownership
Stale base/warm snapshots predate the dynoxide binary but workers still
fast-forward onto the new start-services.sh, which hard-died and took out
every worker (all-200 boot failure). Now: fetch the ~3MB static musl
binary at boot when missing; if that fails too, degrade (skip the
emulator — the app fails open and dynamo-gated tests skip) instead of
dying. boot.ts's dynoxide port wait is likewise non-fatal with a short
timeout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: stop NodeSDK falling back to the localhost OTLP metrics exporter
…rward

Stale warm snapshots bake a DB schema behind the fast-forwarded code
(column "idempotency_config" does not exist killed all 200 workers), so
boot now applies pending migrations (AUTUMN_DB_DIRECT against the baked
localhost PG — no-op on current snapshots). The warm refresh that kept
snapshots stale was itself dying on "cannot lock ref origin/ayush/*";
the fetch --all fallback now prunes stale remote-tracking refs first.

Hook checks run manually (infisical scan clean, knip clean) — the new
pre-commit's `bun infisical` invocation is broken (no such script).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale warm fork can lag the fast-forwarded lockfile (Cannot find module
@aws-sdk/lib-dynamodb killed all 200 workers after the repo fast-forward).
Frozen-lockfile install before the schema self-heal — a no-op in seconds
on current snapshots. (--no-verify: pre-commit's `bun infisical` script is
broken; infisical scan + knip run manually, both clean.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The mirror claim is fire-and-forget (plus lazy emulator table creation on
first use), so the immediate GetItem raced it in the tw µVMs — NaN
expiresAt. (--no-verify: broken `bun infisical` hook; scan+knip manual.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ribution promotion

Pooled balances now reset via the lazy read path and crons regardless of
reset_mode (invoice.created stays as a redundant CAS-guarded fast path), so
pools whose reset interval is shorter than their sub's invoice cadence (e.g.
monthly pool on an annual sub) reset correctly. Prepaid pooled quantity
downgrades record next_cycle_contribution + effective_at, and pool resets
promote due contributions in one CTE that also recomputes granted from all
contributions (drift self-heals; contribution-less pools untouched). The lazy
path patches granted into the cached subject via updateSubjectBalances Lua.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Backfill runs manually instead; drizzle meta restored to dev.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pre-commit hook skipped: `bun infisical` script is broken on dev; ran infisical scan manually.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d out

Pre-commit hook skipped: `bun infisical` script is broken on dev; ran infisical scan manually.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
charlietlamb and others added 2 commits August 4, 2026 13:06
…rtisement

fix(auth): hide internal scopes from MCP clients
…duplicate

Prevents a lagging mirror from backfilling a duplicate's key with a fresh
(extended) TTL.

Pre-commit hook skipped: `bun infisical` script is broken on dev; ran infisical scan manually.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@johnyeocx
johnyeocx requested a review from ay-rod as a code owner August 4, 2026 12:10
@entelligence-ai-pr-reviews

Copy link
Copy Markdown
Contributor

Automatic Review Skipped

Too many files for automatic review.

If you would still like a review, you can trigger one manually by commenting:

@entelligence review

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (176 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

@capy-ai

capy-ai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@socket-security

socket-security Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​@​infisical/​cli@​0.43.116881008896100
Addednpm/​@​aws-sdk/​client-dynamodb@​3.1095.010010010098100
Addednpm/​@​aws-sdk/​lib-dynamodb@​3.1095.010010010098100

View full report

johnyeocx and others added 3 commits August 4, 2026 13:22
…writes

The pool UPDATE checks its row version (updated_at latest vs statement
snapshot) and the contribution promotion is gated on it, so a transition
committing mid-statement makes the whole promotion a no-op instead of
stomping the transition's granted. Race test orchestrates the interleave
with a held transaction against the real helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A guard-tripped promotion re-reads the pool's granted so a CAS-winning
caller refills from the concurrent writer's value, not its stale snapshot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(billing): lazy reset for all pooled balances + next-cycle contribution promotion
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
checkout Ignored Ignored Aug 4, 2026 12:28pm
landing-page Ignored Ignored Aug 4, 2026 12:28pm

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

26 issues found and verified against the latest diff

Confidence score: 2/5

  • In server/src/external/aws/dynamodb/initDynamoDb.ts, always injecting static credentials (including empty env-derived values) can break Dynamo access in IAM/task-role environments, causing immediate runtime failures for idempotency/storage paths — let the AWS SDK credential chain handle credentials when explicit keys are not valid.
  • The idempotency flow has several concrete correctness gaps across server/src/external/aws/dynamodb/ensureLocalDynamoTable.ts, server/src/internal/misc/idempotency/withIdempotencyKey.ts, server/src/external/redis/idempotencyKeys/operations/claimRedisIdempotencyKey.ts, and both release paths in .../releaseRedisIdempotencyKey.ts and .../releaseDynamoIdempotencyKey.ts; these can produce fail-open claims, stuck keys, false 409s, or deletion of newer claims under retries/races — add readiness waiting, robust status parsing, integer TTL normalization, and ownership-checked/retry-friendly release semantics.
  • server/src/queue/queueUtils.ts can lose queued messages during shutdown because shutdownSqsSendBatchers short-circuits on the first rejection, so one batcher may not finish draining — wait for both drain paths with Promise.allSettled and rethrow after both complete.
  • There are deployment/integration edge cases that can surface as production incidents: scripts/tw/image/freestyle-base.sh may install the wrong dynoxide binary on arm64 (exec-format failure), scripts/aws/dynamo/index.ts can misreport TTL readiness on the wrong attribute (unexpected row retention), and server/src/external/autumn/autumnCli.ts can throw on non-JSON 2xx responses — align binary selection to architecture and harden TTL/response validation checks.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/tw/image/freestyle-base.sh">

<violation number="1" location="scripts/tw/image/freestyle-base.sh:17">
P2: On arm64 workers this line installs an x86_64 dynoxide binary, so dynoxide startup can fail with an exec-format error. Building the URL from `uname -m` (matching `start-services.sh`) keeps base images architecture-safe.</violation>

<violation number="2" location="scripts/tw/image/freestyle-base.sh:61">
P2: This install can pick an unrelated `dynoxide*` file already present under `/tmp`, producing a nondeterministic binary in `$BIN_DIR/dynoxide`. Scoping extraction/lookup to a dedicated temp directory makes the selected artifact deterministic.</violation>
</file>

<file name="scripts/aws/dynamo/index.ts">

<violation number="1" location="scripts/aws/dynamo/index.ts:103">
P2: Setup can report TTL as ready even when DynamoDB TTL is enabled on the wrong attribute, so idempotency rows may never expire by the intended field. The early-return condition should also verify `TimeToLiveDescription.AttributeName === ttlAttribute` before skipping `UpdateTimeToLive`.</violation>
</file>

<file name="server/src/internal/balances/track/utils/queueTrack.ts">

<violation number="1" location="server/src/internal/balances/track/utils/queueTrack.ts:51">
P3: Queue fallback diagnostics can be misleading when enqueue is impossible, because this branch is reached only when no explicit `queueUrl`, no `TRACK_ASYNC_SQS_QUEUE_URL`, and no `TRACK_SQS_QUEUE_URL` are available. Updating the message to name the full condition would make operational triage clearer.</violation>
</file>

<file name="server/src/external/autumn/autumnCli.ts">

<violation number="1" location="server/src/external/autumn/autumnCli.ts:187">
P2: Successful POST calls can now fail with a raw JSON parse exception when the API returns a non-JSON 2xx body (common with async/accepted endpoints). Parsing the success payload defensively (or falling back to raw text) keeps this wrapper from turning valid upstream responses into local runtime errors.</violation>
</file>

<file name="server/src/internal/misc/miscellaneousEdgeConfig/miscellaneousEdgeConfigSchemas.ts">

<violation number="1" location="server/src/internal/misc/miscellaneousEdgeConfig/miscellaneousEdgeConfigSchemas.ts:14">
P2: The rollout note now hard-codes `24h`, but idempotency TTL is configurable per org/route, so this guidance can be wrong after this release. Consider documenting this as “after the maximum configured Redis TTL window” (or referencing the actual configured value) to avoid unsafe flips.</violation>
</file>

<file name="apps/sdk-test/sdk3.ts">

<violation number="1" location="apps/sdk-test/sdk3.ts:1">
P2: If `AUTUMN_SECRET_KEY` is unset, requests now send an invalid `Bearer undefined` header and fail later as HTTP auth errors. A startup-time guard on this assignment would make misconfiguration fail fast and easier to diagnose.</violation>
</file>

<file name="scripts/tw/image/start-services.sh">

<violation number="1" location="scripts/tw/image/start-services.sh:204">
P3: Startup logs can claim dynoxide is ready even when the script intentionally continued without it. Making the final status message conditional avoids misleading health/debug signals.</violation>
</file>

<file name="server/src/internal/orgs/handlers/handleUpdateOrgConfig.ts">

<violation number="1" location="server/src/internal/orgs/handlers/handleUpdateOrgConfig.ts:32">
P2: Clients can receive inconsistent response shapes from this endpoint: no-op updates omit `idempotency_config`, but write paths include it. Returning `idempotency_config` in the early-return branch would keep the API contract stable across request variants.</violation>
</file>

<file name="server/src/internal/misc/idempotency/actions/checkIdempotencyKey.ts">

<violation number="1" location="server/src/internal/misc/idempotency/actions/checkIdempotencyKey.ts:43">
P2: Request logs now include the raw `idempotencyKey`, which can leak client-provided identifiers into centralized logging. Since `hashedKey` is already computed, logging only the hash preserves observability without storing the raw key.</violation>
</file>

<file name="server/src/external/aws/dynamodb/ensureLocalDynamoTable.ts">

<violation number="1" location="server/src/external/aws/dynamodb/ensureLocalDynamoTable.ts:23">
P2: First idempotency claims after process start can fail open even when local Dynamo is healthy, because table creation is treated as complete before the table is writable. Adding a table-exists/ACTIVE waiter after CreateTable would make the first write path deterministic.</violation>

<violation number="2" location="server/src/external/aws/dynamodb/ensureLocalDynamoTable.ts:48">
P2: TTL setup failures are currently silent because the catch block ignores every exception type. Narrowing the catch to the known "already enabled" validation case keeps emulator tolerance while still surfacing real configuration/runtime problems.</violation>
</file>

<file name="server/src/utils/otel/FilteringSpanProcessor.ts">

<violation number="1" location="server/src/utils/otel/FilteringSpanProcessor.ts:100">
P3: Dynamo span dropping is now tied to ambient `OTEL_DYNAMO_SUCCESS_SAMPLE_RATE`, while this processor already uses an injectable rate for Redis to keep behavior testable and deterministic. Matching that pattern for Dynamo would reduce env-coupled/flaky assertions when tests exercise `onEnd` with Dynamo spans.</violation>
</file>

<file name="server/src/external/aws/dynamodb/initDynamoDb.ts">

<violation number="1" location="server/src/external/aws/dynamodb/initDynamoDb.ts:26">
P1: Dynamo calls can fail in environments that rely on IAM/task-role credentials because this client always sets static credentials, and missing env vars become empty strings. Letting the SDK fall back to its default credential provider unless both credential env vars are present avoids breaking production auth paths.</violation>
</file>

<file name="server/src/internal/balances/track/runBatchTrack.ts">

<violation number="1" location="server/src/internal/balances/track/runBatchTrack.ts:24">
P3: Queue misconfiguration now generates N warning logs for an N-item batch instead of one, which can flood logs during outages and hide higher-signal errors. A single precheck for queue availability before per-item enqueue would keep failure behavior while avoiding per-item warn spam.</violation>
</file>

<file name="vite/src/views/settings/sections/BillingSettingsSection.tsx">

<violation number="1" location="vite/src/views/settings/sections/BillingSettingsSection.tsx:247">
P2: Switching the duration unit currently changes the underlying TTL amount instead of only changing how it is displayed. `onValueChange` reuses the same numeric value across units, so values like 2 days become 2 hours and can be saved unintentionally; converting from current hours when unit changes avoids this.</violation>
</file>

<file name="server/src/queue/queueUtils.ts">

<violation number="1" location="server/src/queue/queueUtils.ts:151">
P2: Shutdown can drop queued sends from one batcher when the other batcher errors. `shutdownSqsSendBatchers` short-circuits via `Promise.all`, so waiting for both drain paths with `Promise.allSettled` (then rethrowing) is safer.</violation>
</file>

<file name="server/src/external/aws/dynamodb/idempotencyKeys/operations/claimDynamoIdempotencyKey.ts">

<violation number="1" location="server/src/external/aws/dynamodb/idempotencyKeys/operations/claimDynamoIdempotencyKey.ts:49">
P2: Requests retried exactly at TTL boundary can still be rejected as duplicates for up to one extra second. The condition uses `expiresAt < :now` even though Dynamo TTL expiration is second-based, so using `<=` avoids the boundary false-duplicate window.</violation>
</file>

<file name="server/src/internal/misc/idempotency/withIdempotencyKey.ts">

<violation number="1" location="server/src/internal/misc/idempotency/withIdempotencyKey.ts:16">
P2: Some retryable failures can be treated as non-retryable, leaving the idempotency key stuck and causing later retries to 409. `Number(error.statusCode)` can produce `NaN`; treating non-finite parses as `null` keeps unknown-error release behavior consistent.</violation>
</file>

<file name="server/src/internal/auth/oauth/internalMcpOAuthClients.ts">

<violation number="1" location="server/src/internal/auth/oauth/internalMcpOAuthClients.ts:116">
P2: Internal MCP authorize requests that contain only meta scopes can be rewritten to an explicit empty `scope`, which can later be treated as an explicit empty selection and fail consent with `invalid_scope`. It would be safer to delete the `scope` param when no non-meta scopes remain instead of setting it to an empty string.</violation>
</file>

<file name="server/src/external/redis/idempotencyKeys/operations/releaseRedisIdempotencyKey.ts">

<violation number="1" location="server/src/external/redis/idempotencyKeys/operations/releaseRedisIdempotencyKey.ts:8">
P2: Retryable failures can still leave the idempotency key stuck and block client retries when Redis is briefly unavailable, because release exits early on not-ready and ignores delete errors. This path is supposed to free retryable keys, so consider making release resilient (e.g., queued retry/backoff or another guaranteed cleanup path) instead of silently dropping the delete.</violation>
</file>

<file name="server/src/external/aws/dynamodb/idempotencyKeys/operations/releaseDynamoIdempotencyKey.ts">

<violation number="1" location="server/src/external/aws/dynamodb/idempotencyKeys/operations/releaseDynamoIdempotencyKey.ts:19">
P2: A late failure path can clear a newer idempotency claim because Dynamo release deletes by `pk` without verifying ownership/version. Consider making release conditional on claim metadata (for example a claim token or createdAt fence) so an older request cannot delete a key that was reclaimed after TTL expiry.</violation>
</file>

<file name="server/tests/unit/balances/track/run-queued-track-idempotency.test.ts">

<violation number="1" location="server/tests/unit/balances/track/run-queued-track-idempotency.test.ts:21">
P2: The concurrent tests share process-global `claimCalls`, `releaseCalls`, and `runTrackV3Calls` without per-test isolation. Any release or call added by another test (or by future test cases) can change these global counts and produce order-dependent, flaky results; scoping calls to each test key or resetting state in setup would make the assertions deterministic.</violation>
</file>

<file name="server/src/external/redis/idempotencyKeys/operations/claimRedisIdempotencyKey.ts">

<violation number="1" location="server/src/external/redis/idempotencyKeys/operations/claimRedisIdempotencyKey.ts:18">
P2: Idempotency claims can fail open for orgs with fractional TTL config because Redis PX expects an integer duration but this path forwards `ttlMs` as-is. Normalizing to an integer before `SET NX PX` keeps duplicate protection consistent instead of treating a command-argument error as store unavailability.</violation>

<violation number="2" location="server/src/external/redis/idempotencyKeys/operations/claimRedisIdempotencyKey.ts:20">
P3: Redis claim failures are swallowed silently, so idempotency fail-open events lose the signal needed to detect and debug outages. Capturing and logging the error (or routing through the standard Redis op wrapper) would preserve observability parity with the Dynamo path.</violation>
</file>

<file name="server/tests/integration/balances/track/idempotency/dynamo-idempotency-store.test.ts">

<violation number="1" location="server/tests/integration/balances/track/idempotency/dynamo-idempotency-store.test.ts:20">
P2: Test 3 ('expired-but-undeleted item is claimable') writes directly to DynamoDB via `PutCommand` without ensuring the table exists first. It depends on the implicit side effect of test 1 creating the table through `claimDynamoIdempotencyKey`. If tests are reordered, run in isolation (`--rerun-each`), or test 1 fails before the table is created, this test will throw `ResourceNotFoundException`. Consider calling `ensureLocalDynamoTable` independently — in a `beforeAll` or directly in the test before the `PutCommand`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

return {
region: process.env.AWS_REGION || DEFAULT_AWS_REGION,
...(endpoint ? { endpoint } : {}),
credentials: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Dynamo calls can fail in environments that rely on IAM/task-role credentials because this client always sets static credentials, and missing env vars become empty strings. Letting the SDK fall back to its default credential provider unless both credential env vars are present avoids breaking production auth paths.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/external/aws/dynamodb/initDynamoDb.ts, line 26:

<comment>Dynamo calls can fail in environments that rely on IAM/task-role credentials because this client always sets static credentials, and missing env vars become empty strings. Letting the SDK fall back to its default credential provider unless both credential env vars are present avoids breaking production auth paths.</comment>

<file context>
@@ -0,0 +1,51 @@
+	return {
+		region: process.env.AWS_REGION || DEFAULT_AWS_REGION,
+		...(endpoint ? { endpoint } : {}),
+		credentials: {
+			accessKeyId: process.env.AWS_ACCESS_KEY_ID || "",
+			secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY || "",
</file context>

echo "[freestyle-base] 4b/7 dynoxide (native DynamoDB emulator)"
curl -fsSL -o /tmp/dx.tar.gz "$DYNOXIDE_URL"
tar -xzf /tmp/dx.tar.gz -C /tmp
install -m0755 "$(find /tmp -type f -name 'dynoxide*' ! -name '*.tar.gz' | head -1)" "$BIN_DIR/dynoxide"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This install can pick an unrelated dynoxide* file already present under /tmp, producing a nondeterministic binary in $BIN_DIR/dynoxide. Scoping extraction/lookup to a dedicated temp directory makes the selected artifact deterministic.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/tw/image/freestyle-base.sh, line 61:

<comment>This install can pick an unrelated `dynoxide*` file already present under `/tmp`, producing a nondeterministic binary in `$BIN_DIR/dynoxide`. Scoping extraction/lookup to a dedicated temp directory makes the selected artifact deterministic.</comment>

<file context>
@@ -54,6 +55,12 @@ tar -xf /tmp/g.tar -C /tmp "$GOAWS_BIN"
+echo "[freestyle-base] 4b/7 dynoxide (native DynamoDB emulator)"
+curl -fsSL -o /tmp/dx.tar.gz "$DYNOXIDE_URL"
+tar -xzf /tmp/dx.tar.gz -C /tmp
+install -m0755 "$(find /tmp -type f -name 'dynoxide*' ! -name '*.tar.gz' | head -1)" "$BIN_DIR/dynoxide"
+rm -f /tmp/dx.tar.gz
+
</file context>

DRAGONFLY_URL="https://dragonflydb.gateway.scarf.sh/latest/dragonfly-x86_64.tar.gz"
CRANE_URL="https://github.com/google/go-containerregistry/releases/download/v0.20.2/go-containerregistry_Linux_x86_64.tar.gz"
GOAWS_IMAGE="admiralpiett/goaws:latest"
DYNOXIDE_URL="https://github.com/nubo-db/dynoxide/releases/download/v0.13.0/dynoxide-x86_64-unknown-linux-musl.tar.gz"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: On arm64 workers this line installs an x86_64 dynoxide binary, so dynoxide startup can fail with an exec-format error. Building the URL from uname -m (matching start-services.sh) keeps base images architecture-safe.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/tw/image/freestyle-base.sh, line 17:

<comment>On arm64 workers this line installs an x86_64 dynoxide binary, so dynoxide startup can fail with an exec-format error. Building the URL from `uname -m` (matching `start-services.sh`) keeps base images architecture-safe.</comment>

<file context>
@@ -14,6 +14,7 @@ PGDATA="$TW_PREFIX/pgdata"
 DRAGONFLY_URL="https://dragonflydb.gateway.scarf.sh/latest/dragonfly-x86_64.tar.gz"
 CRANE_URL="https://github.com/google/go-containerregistry/releases/download/v0.20.2/go-containerregistry_Linux_x86_64.tar.gz"
 GOAWS_IMAGE="admiralpiett/goaws:latest"
+DYNOXIDE_URL="https://github.com/nubo-db/dynoxide/releases/download/v0.13.0/dynoxide-x86_64-unknown-linux-musl.tar.gz"
 
 export DEBIAN_FRONTEND=noninteractive
</file context>
Suggested change
DYNOXIDE_URL="https://github.com/nubo-db/dynoxide/releases/download/v0.13.0/dynoxide-x86_64-unknown-linux-musl.tar.gz"
DYNOXIDE_VERSION="${DYNOXIDE_VERSION:-v0.13.0}"
ARCH="$(uname -m)"
case "$ARCH" in
x86_64) DYNOXIDE_URL="https://github.com/nubo-db/dynoxide/releases/download/${DYNOXIDE_VERSION}/dynoxide-x86_64-unknown-linux-musl.tar.gz" ;;
aarch64 | arm64) DYNOXIDE_URL="https://github.com/nubo-db/dynoxide/releases/download/${DYNOXIDE_VERSION}/dynoxide-aarch64-unknown-linux-musl.tar.gz" ;;
*) echo "[freestyle-base] unsupported arch for dynoxide: $ARCH" >&2; exit 1 ;;
esac

new DescribeTimeToLiveCommand({ TableName: tableName }),
);
const ttlStatus = ttl.TimeToLiveDescription?.TimeToLiveStatus;
if (ttlStatus === "ENABLED" || ttlStatus === "ENABLING") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Setup can report TTL as ready even when DynamoDB TTL is enabled on the wrong attribute, so idempotency rows may never expire by the intended field. The early-return condition should also verify TimeToLiveDescription.AttributeName === ttlAttribute before skipping UpdateTimeToLive.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/aws/dynamo/index.ts, line 103:

<comment>Setup can report TTL as ready even when DynamoDB TTL is enabled on the wrong attribute, so idempotency rows may never expire by the intended field. The early-return condition should also verify `TimeToLiveDescription.AttributeName === ttlAttribute` before skipping `UpdateTimeToLive`.</comment>

<file context>
@@ -0,0 +1,162 @@
+		new DescribeTimeToLiveCommand({ TableName: tableName }),
+	);
+	const ttlStatus = ttl.TimeToLiveDescription?.TimeToLiveStatus;
+	if (ttlStatus === "ENABLED" || ttlStatus === "ENABLING") {
+		log(
+			`TTL already ${ttlStatus.toLowerCase()} on ${tableName}.${ttlAttribute}`,
</file context>

} catch {
parsed = null;
}
return rawBody ? JSON.parse(rawBody) : null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Successful POST calls can now fail with a raw JSON parse exception when the API returns a non-JSON 2xx body (common with async/accepted endpoints). Parsing the success payload defensively (or falling back to raw text) keeps this wrapper from turning valid upstream responses into local runtime errors.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/external/autumn/autumnCli.ts, line 187:

<comment>Successful POST calls can now fail with a raw JSON parse exception when the API returns a non-JSON 2xx body (common with async/accepted endpoints). Parsing the success payload defensively (or falling back to raw text) keeps this wrapper from turning valid upstream responses into local runtime errors.</comment>

<file context>
@@ -181,41 +181,42 @@ export class AutumnInt {
-			} catch {
-				parsed = null;
-			}
+			return rawBody ? JSON.parse(rawBody) : null;
+		}
 
</file context>
Suggested change
return rawBody ? JSON.parse(rawBody) : null;
if (!rawBody) return null;
try {
return JSON.parse(rawBody);
} catch {
return rawBody;
}

if (!resolvedQueueUrl) {
ctx.logger.warn(
"[track] Redis unavailable and TRACK_SQS_QUEUE_URL is unset; falling back to synchronous track",
"[track] TRACK_ASYNC_SQS_QUEUE_URL is unset; falling back to synchronous track",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Queue fallback diagnostics can be misleading when enqueue is impossible, because this branch is reached only when no explicit queueUrl, no TRACK_ASYNC_SQS_QUEUE_URL, and no TRACK_SQS_QUEUE_URL are available. Updating the message to name the full condition would make operational triage clearer.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/balances/track/utils/queueTrack.ts, line 51:

<comment>Queue fallback diagnostics can be misleading when enqueue is impossible, because this branch is reached only when no explicit `queueUrl`, no `TRACK_ASYNC_SQS_QUEUE_URL`, and no `TRACK_SQS_QUEUE_URL` are available. Updating the message to name the full condition would make operational triage clearer.</comment>

<file context>
@@ -5,28 +5,50 @@ import { addTaskToQueue } from "@/queue/queueUtils.js";
 		if (!resolvedQueueUrl) {
 			ctx.logger.warn(
-				"[track] Redis unavailable and TRACK_SQS_QUEUE_URL is unset; falling back to synchronous track",
+				"[track] TRACK_ASYNC_SQS_QUEUE_URL is unset; falling back to synchronous track",
 			);
 			return null;
</file context>
Suggested change
"[track] TRACK_ASYNC_SQS_QUEUE_URL is unset; falling back to synchronous track",
"[track] TRACK_ASYNC_SQS_QUEUE_URL and TRACK_SQS_QUEUE_URL are unset; falling back to synchronous track",

fi

log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT)"
log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT dynoxide:$DYNAMODB_PORT)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Startup logs can claim dynoxide is ready even when the script intentionally continued without it. Making the final status message conditional avoids misleading health/debug signals.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/tw/image/start-services.sh, line 204:

<comment>Startup logs can claim dynoxide is ready even when the script intentionally continued without it. Making the final status message conditional avoids misleading health/debug signals.</comment>

<file context>
@@ -148,4 +201,4 @@ else
 fi
 
-log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT)"
+log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT dynoxide:$DYNAMODB_PORT)"
</file context>
Suggested change
log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT dynoxide:$DYNAMODB_PORT)"
log "All services ready (pg:$PG_PORT dragonfly:$DRAGONFLY_PORT goaws:$ELASTICMQ_PORT$( [ "${DYNOXIDE_DISABLED:-0}" != "1" ] && printf ' dynoxide:%s' "$DYNAMODB_PORT" || printf ' dynoxide:disabled' ))"

try {
recordSpanDurationMetric(span);
if (shouldDropSuccessfulRedisSpan(span, this.sampleRate)) return;
if (shouldDropSuccessfulDynamoSpan(span)) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Dynamo span dropping is now tied to ambient OTEL_DYNAMO_SUCCESS_SAMPLE_RATE, while this processor already uses an injectable rate for Redis to keep behavior testable and deterministic. Matching that pattern for Dynamo would reduce env-coupled/flaky assertions when tests exercise onEnd with Dynamo spans.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/utils/otel/FilteringSpanProcessor.ts, line 100:

<comment>Dynamo span dropping is now tied to ambient `OTEL_DYNAMO_SUCCESS_SAMPLE_RATE`, while this processor already uses an injectable rate for Redis to keep behavior testable and deterministic. Matching that pattern for Dynamo would reduce env-coupled/flaky assertions when tests exercise `onEnd` with Dynamo spans.</comment>

<file context>
@@ -67,6 +97,7 @@ export class FilteringSpanProcessor implements SpanProcessor {
 		try {
 			recordSpanDurationMetric(span);
 			if (shouldDropSuccessfulRedisSpan(span, this.sampleRate)) return;
+			if (shouldDropSuccessfulDynamoSpan(span)) return;
 			spanToExport = this.spanIngestCompactor.compact({ span });
 		} catch (error) {
</file context>

const messageDeduplicationId = `${ctx.id}-${index}`;
// One queueTrack per item — the SQS send batcher packs them into
// SendMessageBatch calls, and each item resolves/fails independently.
const results = await Promise.all(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Queue misconfiguration now generates N warning logs for an N-item batch instead of one, which can flood logs during outages and hide higher-signal errors. A single precheck for queue availability before per-item enqueue would keep failure behavior while avoiding per-item warn spam.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/balances/track/runBatchTrack.ts, line 24:

<comment>Queue misconfiguration now generates N warning logs for an N-item batch instead of one, which can flood logs during outages and hide higher-signal errors. A single precheck for queue availability before per-item enqueue would keep failure behavior while avoiding per-item warn spam.</comment>

<file context>
@@ -16,51 +15,45 @@ export const runBatchTrack = async ({
-		const messageDeduplicationId = `${ctx.id}-${index}`;
+	// One queueTrack per item — the SQS send batcher packs them into
+	// SendMessageBatch calls, and each item resolves/fails independently.
+	const results = await Promise.all(
+		body.map((item, index) => {
+			const messageDeduplicationId = `${ctx.id}-${index}`;
</file context>

// SET NX (set if not exists) for an atomic check-and-set.
const wasSet = await redis.set(storageKey, "1", "PX", ttlMs, "NX");
return wasSet ? "claimed" : "duplicate";
} catch {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Redis claim failures are swallowed silently, so idempotency fail-open events lose the signal needed to detect and debug outages. Capturing and logging the error (or routing through the standard Redis op wrapper) would preserve observability parity with the Dynamo path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/external/redis/idempotencyKeys/operations/claimRedisIdempotencyKey.ts, line 20:

<comment>Redis claim failures are swallowed silently, so idempotency fail-open events lose the signal needed to detect and debug outages. Capturing and logging the error (or routing through the standard Redis op wrapper) would preserve observability parity with the Dynamo path.</comment>

<file context>
@@ -0,0 +1,23 @@
+		// SET NX (set if not exists) for an atomic check-and-set.
+		const wasSet = await redis.set(storageKey, "1", "PX", ttlMs, "NX");
+		return wasSet ? "claimed" : "duplicate";
+	} catch {
+		return "unavailable";
+	}
</file context>

@johnyeocx
johnyeocx merged commit 80e6f7e into main Aug 4, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants