Repository navigation
chore(dev): drop localstack from the dev stack - #81257
Conversation
|
😎 Merged successfully - details. |
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 68.31 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 304.7 KiB | ../node_modules/.pnpm/posthog-js@1.417.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 246.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 243.9 KiB | ../node_modules/.pnpm/posthog-js@1.417.1/node_modules/posthog-js/dist/module.js |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.0 KiB | src/lib/api.ts |
| 95.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.23 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.23 MiB · 17 files | no change | ████░░░░░░ 38.9% of 5.72 MiB |
| Deferred (lazy) | 2.09 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 741.3 KiB | dist/toolbar/toolbar-app-7XQF5GZA.css |
| 565.7 KiB | dist/toolbar/chunk-chunk-5XQV6DM2.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-3A7RSMAG.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-RYSZ67CJ.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.2 KiB | dist/toolbar/toolbar-app-MBCFR53V.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-TVGFVNS4.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-3D4FXM63.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1450.28 MiB · no change
|
… into worktree-migrate-off-localstack
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
Prompt To Fix All With AI### Issue 1
.github/workflows/ci-nodejs.yml:389-391
**Unrebased branches lack DynamoDB**
When this workflow runs for an unrebased pull request, it requests the new `dynamodb` service from the branch's older Compose file, where that service is undefined, causing the Node.js job to fail before tests run.
### Issue 2
nodejs/tests/session-replay/recording-api/recording-api.integration.test.ts:52-53
**Loopback endpoints break sandbox tests**
When this test runs inside a PostHog sandbox, these addresses target the app container itself while DynamoDB and object storage are sibling Compose services without published ports, causing table setup and S3 operations to fail with connection errors.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "revert(dev): keep the localhost ses endp..." | Re-trigger Greptile |
…f-localstack # Conflicts: # products/workflows/frontend/generated/agentContext.ts
arnohillen
left a comment
There was a problem hiding this comment.
[arno's agent] Reviewed at head f6520ec: change verified against head code, CI classified, no blocking findings.
|
[arno's agent] The gate rename from LOCALSTACK_DISABLED to DYNAMODB_TESTS_DISABLED silently un-skips for devs who exported the old name, giving connection errors when the dynamodb profile is down. |
|
[arno's agent] The CMD-SHELL healthcheck's exec 3<>/dev/tcp/... only works because the image's /bin/sh is bash; a POSIX sh would fail every probe, worth a comment since the image tag pin is the only guard. |
| service: localstack | ||
| container_name: '${LOCALSTACK_DOCKER_NAME:-localstack-main}' | ||
| service: dynamodb | ||
| profiles: [dynamodb] |
There was a problem hiding this comment.
👍🏻
So one less container by default
webjunkie
left a comment
There was a problem hiding this comment.
Looks good from my side.
| * To skip localstack tests (e.g. when localstack isn't running): | ||
| * LOCALSTACK_DISABLED=1 pnpm jest src/session-replay/recording-api/recording-api.integration.test.ts | ||
| * The encryption contract runs against MemoryKeyStore and always runs. The DynamoDBKeyStore | ||
| * block covers what only a real DynamoDB can show: the composite key, the conditional writes, |
There was a problem hiding this comment.
This says the block covers "the conditional writes", but nothing calls generateKey twice for the same session, so attribute_not_exists(session_id) and the ConditionalCheckFailedException fallback only get hit by a mocked unit test. Already true before this PR, but the comment now claims it.
Worth adding, it's the cheapest real-DynamoDB test left. The second one especially, since if that condition expression ever breaks, re-ingesting a deleted session overwrites its tombstone and un-shreds it:
it('returns the existing key instead of overwriting on a second generate', ...)
it('does not resurrect a shredded session on regenerate', ...)Or just drop "the conditional writes" from the line.
|
|
||
| if (command instanceof GenerateDataKeyCommand) { | ||
| const { NumberOfBytes, EncryptionContext } = command.input | ||
| const plaintext = sodium.randombytes_buf(NumberOfBytes ?? sodium.crypto_secretbox_KEYBYTES) |
There was a problem hiding this comment.
The stub takes two things real KMS won't, so a regression would go green here and fail in prod. NumberOfBytes ?? crypto_secretbox_KEYBYTES means a missing or silly value quietly works, where KMS wants 1..1024. And the context serializer is happy with a number, so if someone drops the String(teamId) in dynamodb-keystore.ts:40 you'd get a ValidationException on every key generation in prod and nothing here.
Rejecting both would make the parity actually enforced rather than just claimed.
| @@ -1,3 +1,6 @@ | |||
| 2026-08-12 | |||
| localstack is gone (its license stopped being free); if you still have a localstack-main container, run hogli docker:services:down to clear it | |||
There was a problem hiding this comment.
hogli docker:services:down has no -p, so the project name comes from the directory you're in. From a worktree it hits a different compose project and clears nothing, which is where a lot of us work. docker rm -f localstack-main works anywhere.
There was a problem hiding this comment.
Putting dynamodb on nodejs_session_replay means every replay dev stack runs a JVM container that only the test uses. Fine if that's on purpose, just flagging it idles heavier than what it replaces.
…f-localstack # Conflicts: # nodejs/package.json
Problem
localstack:4.12.0).Closes #53436
Changes
amazon/dynamodb-local, the existingobjectstorageservice, and an in-process KMS stub that enforcesEncryptionContextandKeyIdthe way real KMS does.MemoryKeyStore. The DynamoDB block keeps only what a real DynamoDB decides: composite key, conditional writes, tombstones.nodejs_session_replayso replay stacks start it.LOCALSTACK_ENABLEDvariable is gone.-disableTelemetryis set (on by default), anddevenv/news.txttells devs to clear leftoverlocalstack-maincontainers.Note
The dev
SES_ENDPOINTpins tolocalhost:4566stay. The SES client passes no explicit credentials, so the dead-port pin is what keeps a local send off real AWS via ambient credentials. Cleanup belongs to the messaging owners.How did you test this code?
dynamodb-main Healthyand the file passing.DYNAMODB_TESTS_DISABLED=1skips the 7 stack-dependent cases.deleted_atfrom the tombstone write turns tests red.Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
/writing-tests,/writing-code-comments,/writing-pr-descriptions,/code-review,/simplify.objectstorage's bootstrap sentinel, the profile sat in the legacy overlay, and an SES default deletion that would have failed open to real AWS was reverted.