e609320666bb63cfef37b3f3d2bd487cdeee0df9
5243 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Félix Malfait
|
9be2b21e51 |
fix(ai): gate chat thread totals on stream ownership to prevent usage under-counting (#22534)
Fixes a usage under-counting bug introduced by #22524 (progressive assistant-message persistence), flagged by @cubic-dev-ai and confirmed by @FelixMalfait. ## Root cause #22524 gated the thread-totals update (tokens, credits, `conversationSize`) on `assistantMessageExistedAtStart` — an existence check on the deterministic `uuidv5(streamId)` message id captured at job start. That was a sound idempotency signal *before* #22524, when the message only ever existed if a prior run had completed and applied totals. Progressive checkpoints broke that assumption: a checkpoint creates the message row ~2s into the stream, without applying totals. So if the worker is SIGKILLed after a checkpoint but before `handleStreamFinish`, and the job is re-delivered (BullMQ's stalled re-run — `aiStreamQueue` has no `maxStalledCount: 0` yet, that's #22518 — or an admin `retryJobs`), the re-run sees `assistantMessageExistedAtStart === true` and returns before the totals update. The turn's usage is lost permanently. cubic's P2 (the non-transactional `delete`+`insert` in `upsertAssistantMessage`) is the same root cause: its partless window only mattered because it tripped the same existence-based gate. ## Fix Stop inferring "totals already applied" from message existence. Gate the totals update on **still owning the stream** — a conditional `UPDATE ... WHERE id = :threadId AND activeStreamId = :streamId`, and only `notifyThreadUsageUpdated` when it affects a row. This is the same claim pattern the stream already uses (#22481), and it's idempotent by construction: - The run that completes while holding the claim → `affected = 1` → totals applied exactly once. This holds **even when a checkpoint already created the message**, which is precisely the bug. - A duplicate/zombie run after another run completed (and its `finally` cleared `activeStreamId`) → `affected = 0` → skipped, no double-count. - A superseded run whose thread has moved to a newer stream → `affected = 0` → skipped (defense-in-depth, aligns with #22518's ownership pre-check). The `assistantMessageExistedAtStart` flag and its start-of-stream `hasMessageById` query are removed entirely — the message write is already idempotent via the deterministic id + `upsert`, so it needs no gate. This subsumes cubic's P2: the totals are no longer lost regardless of the `delete`+`insert` window, so no transaction is required for correctness (the residual window is a benign sub-millisecond transient for an actively-streaming message; happy to add a workspace-datasource transaction as separate hardening if you'd prefer). ## Validation `stream-agent-chat.job.spec.ts` (9 green): - New: totals update returns `affected: 0` → `notifyThreadUsageUpdated` **not** called (prior completion not double-counted), message still upserted. - New: message already exists from a checkpoint but claim still held (`affected: 1`) → totals **are** applied — the exact regression #22524 caused. - Existing success/error/cancel/abort flows updated for the conditional criteria and still green. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22534?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
e5fc1b3702 |
feat(server): drain queue workers gracefully on SIGTERM (#22514)
## Why Deploys kill workers mid-job: neither the queue worker nor the API server ever calls `enableShutdownHooks()`, so NestJS never listens for SIGTERM and the graceful close in `BullMQDriver.onModuleDestroy` is dead code. On every rollout a worker dies instantly — an in-flight 10-minute AI stream freezes for the watching user, and BullMQ silently re-runs the half-executed job on another worker ~10 minutes later. This is the root cause, not a symptom: the correct drain semantics already exist in the driver (`worker.close()` waits for active jobs and stops picking new ones, per the BullMQ graceful-shutdown docs) — the process just never received the signal. ## What - `queue-worker.ts` + `main.ts`: enable shutdown hooks. SIGTERM now runs `onModuleDestroy` across providers: the BullMQ driver drains active jobs, `RedisClientService` and the AI cancel subscriber quit their Redis connections, TypeORM closes its pools, then the process exits on its own. - BullMQ close order: workers drain before queues close, so a job finishing during the drain can still enqueue follow-ups (e.g. the AI queue flushing the next queued message). - API server: `forceCloseConnections` so long-lived subscription sockets don't hold `close()` open until the pod is force-killed. They were dropped abruptly on every deploy before this PR too — clients already recover. - Drain start/completion logs so pod terminations are debuggable. ## User impact Deploys stop corrupting in-flight background work. Follow-ups build on this: bounded drain-then-abort for AI stream jobs, and eliminating the stalled-job zombie re-run. ## Validation - Local: SIGTERM'd a running worker mid-job — drain log appears, the active job completes, "Message queue shutdown complete" is logged, process exits by itself. (Also verified with `LOGGER_IS_BUFFER_ENABLED=true` that final logs are not swallowed.) - The k8s side (termination grace period ≥ drain budget, exec'ing `node` directly so PID 1 receives SIGTERM) lands separately in twenty-infra. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22514?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> |
||
|
Félix Malfait
|
9496a98aa3 |
feat(ai): persist assistant chat messages progressively during streaming (#22524)
## Why Today the assistant chat message is written to the DB exactly **once**, at `onFinish` (`handleStreamFinish`). The per-step hook (`onStepFinish` in `chat-execution.service.ts`) only does billing/metrics — no persistence. Content streams to the client live over Redis, but the durable record only lands at the end. The graceful paths already cover partials: normal completion, user cancel, and the shutdown-abort from #22517 all fire `onFinish` with `isAborted` and persist whatever parts exist. The gap is a **true SIGKILL** — OOM, node loss, or a grace-period overrun — where `onFinish` never runs. When that happens mid-turn, the assistant message vanishes from the thread even though its tool calls already executed real CRM mutations. That's the worst failure shape: side effects persisted, the record of them didn't. This closes that gap by materializing the assistant message progressively, so a hard kill leaves the tools-already-run on the thread. It's the app-level piece behind the earlier discussion on #22518 — with this, a retried/interrupted turn also resumes from its own partial (the model reloads history and continues) instead of re-doing completed steps. Scope is **chat only** — the workflow agent path (`AgentAsyncExecutorService`, blocking `generateText`) is a different model with its own step-log persistence and workflow-engine resumption, and is deliberately out of scope here. ## What - `AgentChatService.upsertAssistantMessage`: idempotent message+parts write keyed on the deterministic `uuidv5(streamId)` id (upsert the row, replace its parts), reusing the existing `mapUIMessagePartsToDBParts` / `finalizeDanglingToolParts`. - `stream-agent-chat.job.ts`: tee the assembled UI stream — one branch keeps publishing chunks unchanged; the other drives the SDK's own `readUIMessageStream` and, throttled to `AGENT_CHAT_CHECKPOINT_INTERVAL_MS` (2s), fires a serialized fire-and-forget `upsertAssistantMessage`. No chunk re-assembly — the parts come straight from the SDK assembler, identical to what `onFinish` produces. - `handleStreamFinish` now upserts (authoritative) instead of insert-then-skip. Two ordering/idempotency guards: - Checkpoints are serialized through one promise chain and gated off (`isFinalizingPersist`) before the final write, which drains the chain first — so the authoritative write always lands last and never races a checkpoint on the parts table. - The old `hasMessageById`→skip protected the thread-totals accumulation from double-counting on a re-executed job. Since checkpoints now make the row exist mid-stream, that signal is captured **once at stream start** (`assistantMessageExistedAtStart`) and used to gate the totals update — preserving the exact prior idempotency while allowing progressive writes. `readUIMessageStream` runs with `terminateOnError: false` and the checkpoint consumer swallows errors: checkpoints are best-effort and must never affect the stream or the authoritative persist. ## User impact A worker that dies hard mid-turn no longer erases the assistant message. Combined with #22434's Retry, the user sees the partial turn (including executed tools) and can continue, rather than a turn that silently disappeared while its side effects stuck. Note: this is insurance against true SIGKILL specifically — graceful shutdown (#22514/#22517) already persists partials — so it's most valuable for OOM/node-loss/grace-overrun. Framed that way deliberately; happy to drop it if you'd rather not touch this path for that scope. ## Validation - Unit (`stream-agent-chat.job.spec.ts`, 59 green): the success path persists via `upsertAssistantMessage` with the assembled parts + turnId; a re-executed job whose message existed at start still upserts but does **not** re-apply thread totals; all existing flows (mid-stream error, user cancel, shutdown-abort, missing workspace) unchanged. - Local runtime (isolated instance, real OpenAI stream, checkpoint interval shortened for the test): - **SIGKILL mid-stream** (no graceful onFinish) → the assistant message row (deterministic id) is present afterward with a partial text part (~381 chars) that would otherwise have been lost. - **Normal completion** → the final upsert converges to the full message ("Hello, Tim."), `activeStreamId` cleared, `totalOutputTokens` applied exactly once. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22524?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Paul Rastoin
|
43730d7748 |
Centralized side effects devxp basis (#22295)
# Introduction
This PR introduces a centralized, strictly-typed **metadata side-effect
engine** that unifies how system metadata side effects are derived and
applied across both metadata entry points — the **metadata GraphQL API**
and the **application sync / manifest** flow — and migrates the first
side effect end-to-end: **a unique scalar field owns its backing
single-field `UNIQUE` index** (full create / update / delete lifecycle).
## New conventions
- **Engine-owned companions**: metadata flagged `isSystemSideEffect:
true` is owned by the engine. Its deletion is never inferred from
absence in a manifest — it results from PG-level cascade or from a
delete side effect (a side effect always has a cause, its parent
metadata).
- **Reserved deterministic identifiers**: apps cannot declare metadata
reusing an engine-owned deterministic `universalIdentifier`. Doing so
fails validation with `RESERVED_SYSTEM_UNIVERSAL_IDENTIFIER` (until an
explicit override API exists).
- **Record-native operation matrix**: the operation matrix is keyed by
`universalIdentifier` (`AllFlatEntityOperationRecordByMetadataName`)
instead of arrays, making parent resolution and deduplication O(1).
Array-based API callers are transpiled to records at the
validate-build-and-run boundary.
- Twenty-sdk user-facing experience with system fields will only be
related to overrides.
# What this PR does
## 1. Side-effect engine (foundation)
- `MetadataSideEffectEngineService.expandWithSideEffects(...)` takes the
intention-carrying record matrix and returns it expanded with derived
side effects, or a structured failure.
- Handlers are registered via a typed **decorator + registry** pattern
(`MetadataSideEffectHandler({ operation, metadataName, name, description
})`), with runtime duplicate-name detection. Multiple handlers per
(operation, metadataName) are supported.
- Handler contract mirrors the validator pattern:
- receives the trigger flat entity, the live record matrix, and
**strictly-typed related flat entity maps**
(`MetadataFlatEntityAndRelatedFlatEntityMapsForSideEffect<P>`, derived
from declared companion metadata names — no loose
`Partial<AllFlatEntityMaps>` context)
- returns `MetadataSideEffectResult`: `success` (operations record) |
`noop` | `fail` (structured failure)
- **Non-recursion is structural**: triggers are read from the original
caller input, never from the expanded matrix, so a side effect can never
trigger another side effect.
- **Deduplication + collision detection**: side effects are deduped by
`universalIdentifier` per operation; a caller-declared entity colliding
with an engine-owned deterministic identifier is recorded as a
collision.
- **Unified failure channel**: handler failures and reserved-identifier
collisions are merged into the same `OrchestratorFailureReport` contract
as builder validation errors, and the run short-circuits (fail-closed,
nothing is applied).
## 2. First migrated side effect — unique field → backing unique index
Three handlers own the complete lifecycle of the deterministic
single-field `UNIQUE` index backing a unique scalar field:
- **create**: unique scalar field → generate the deterministic backing
index (`fieldUniqueBackingIndexOnCreate`)
- **update**: `isUnique` flips and renames of still-unique fields (the
index name — and therefore its deterministic identifier — derives from
the field name, so a rename drops the stale index and recreates the
deterministic one) (`fieldUniqueBackingIndexOnUpdate`)
- **delete**: cascade-delete the backing index
(`fieldUniqueBackingIndexOnDelete`)
Supporting rules:
- The primary key `id` field never spawns a backing index (uniqueness
comes from the PK constraint) — explicit `isPrimaryKeyFlatFieldMetadata`
guard.
- Parent object resolution is **optimistic-first**: an object created or
updated in the same batch wins over the workspace cache (so e.g.
renaming an object while flipping a field to unique builds the index
from the post-rename object), resolved in O(1) via the record matrix.
- A missing parent object is reported as a structured side-effect
failure, never silently skipped.
## 3. Path convergence — manifest and API share one flow
- The manifest sync now derives a from→to **record matrix** from the
cache and feeds `validateBuildAndRunWorkspaceMigrationFromRecord`, the
same flow the API uses — both paths converge on the engine.
- Manifest-side unique-index generation and API transpiler
system-unique-index handling were removed (declared/composite/relation
indexes stay untouched).
- New `WorkspaceMigrationFlatEntityMapsService` mutualizes
flat-entity-maps computation between the side-effect engine and the
builder: cache keys are derived from the caller metadata names (+
validation- and side-effect-related closures) instead of hardcoded
loads.
- App-scoping and pruning are folded into one shared primitive
(`getSubAllFlatEntityMapsByApplicationIdsOrThrow`): slicing dependency
maps to the involved applications always prunes dangling one-to-many
aggregators — callers can no longer forget it.
- **Behavior change**: an app extending another app's view with a view
field now syncs successfully (cross-app view-field extension), covered
by a dedicated integration test.
## 4. Backfill upgrade command (2.19)
`upgrade:2-19:backfill-system-unique-index-universal-identifier`
rewrites legacy system unique-index `universalIdentifier`s to their
deterministic value so the engine can own pre-existing indexes. The
backfill is **driven from `isUnique: true` fields** (mirroring the
engine ownership predicate — excludes PK / morph / relation fields) and
resolves each field's backing index in O(1).
# Bugs fixed along the way
- `database:reset` seeding failed with
`INDEX_FIELD_INVALID_DEFAULT_VALUE`: the engine derived a backing
`UNIQUE` index for the default `id` primary key. Fixed with the explicit
primary-key guard.
- `isUnique` updates on system-flagged standard fields (e.g.
auto-created `name`) did not trigger the backing-index side effect.
- Manifest sync crashed with "Could not find flat entity with universal
identifier ..." when app-scoped slices left dangling aggregator
references — fixed by centralizing pruning in the shared slice primitive
|
||
|
Félix Malfait
|
9f4efa57ff |
Expose sent message identifiers in workflow send-email step output (#22520)
## Context
First step toward thread-continuity / follow-up email steps in workflows
(email sequences). The outbound send pipeline already knows the sent
email's RFC-822 Message-ID, the provider thread id, and the persisted
message/thread records — but none of it was surfaced in the send-email
step output, so a later step had no way to reference the email that was
sent.
## What changed
- `saveMessagesWithinTransaction` also returns a `messageExternalId →
messageThreadId` map, and `saveMessagesAndEnqueueContactCreation`
returns the message/thread id maps (both other call sites ignore the
return value)
- `SentMessagePersistenceService.persistSentMessage` and
`SendEmailService.persistSentMessage` return the persisted `{ messageId,
messageThreadId }` (`undefined` when persistence is skipped or fails —
sending still succeeds)
- `SendEmailTool` result now includes `headerMessageId`,
`threadExternalId`, `messageId` and `messageThreadId`
- SEND_EMAIL step output schema (server + frontend) declares
`headerMessageId`/`messageId`/`messageThreadId` so they show up in the
variable picker; DRAFT_EMAIL keeps its success-only schema since draft
creation returns no identifiers yet
This already enables manual thread continuity today: wire
`{{sendEmailStep.headerMessageId}}` into a later email step's
In-Reply-To advanced field — the composer resolves the References chain
and provider thread from it.
## Tests
- New `send-email-tool.spec.ts` covering identifiers in the result,
persistence disabled, and persistence failure
- Extended save-messages spec with the new map, updated frontend
`computeStepOutputSchema` tests
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22520?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
|
||
|
Félix Malfait
|
3cf04bea28 |
Support sender variable on workflow send-email action (#22512)
## Context Draft-email workflow steps already accept a workspace member variable as the sender: the step input's `connectedAccountId` can hold a workflow variable that resolves to a workspace member id, which the action then maps to that member's first connected account. Send-email steps were gated out of this and only accepted a static connected account pick. This enables the same dynamic sender resolution on send-email, e.g. sending from the assignee/owner of the record that triggered the workflow. ## What changed - Removed the draft-only gate in `EmailWorkflowActionBase.postprocessInput` so send-email resolves a workspace member id to a connected account the same way draft-email does - Exposed the variable picker and hint on the Account field for both email actions in the workflow step editor - Updated the send-email action spec to cover sender resolution (mirrors the draft-email spec) and replaced the `SendEmailHasNoVariablePicker` story with a variable-sender story for `SEND_EMAIL` <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22512?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
13f380b80d |
perf(front-component): fingerprint built-JS URLs by path for CDN caching (#22530)
**Stacked on #22523** — base is that branch, so the diff shows only this commit. GitHub will retarget it to `main` automatically once #22523 merges. Follows up on @FelixMalfait's question on #22523: move the BuiltFrontComponent cache key from a query string into the path so it plays well with Cloudflare cache rules. ## What - URL: `/rest/front-components/:id?checksum=<c>` → `/rest/front-components/:id/<c>.js` (`getFrontComponentUrl`). - Route: the controller now accepts `[':frontComponentId', ':frontComponentId/:cacheKey']`. `:cacheKey` is a pure cache-buster the server **ignores** — it still resolves by `:frontComponentId`, exactly as the query param did. ## Why a path segment (not `:id-<checksum>.js`) A path-based, extension-bearing URL is matched by Cloudflare's **default** static-asset caching and by trivial `*.js` path cache rules, and it's immune to any "ignore query string" cache setting that would otherwise collapse `?checksum=` to one entry and serve stale JS. I used a path **segment** (`/:id/:checksum.js`) rather than the literal `:id-<checksum>.js` you sketched because the id is a **UUID — which itself contains hyphens** — so a `-` separator is ambiguous to parse. A segment is unambiguous and equally CDN-friendly (still ends in `.js`). ## Backward compatibility The bare `:frontComponentId` route is kept, so URLs minted before this deploys (query-string form, or in-flight pages) still resolve. It can be dropped in a later release once no client mints the old form. No data migration — the URL is computed at render time from `frontComponentId` + `builtComponentChecksum`. ## ⚠️ Decision for you: this alone does not edge-cache — `private` vs `public` BFC is served behind `WorkspaceAuthGuard` and #22523 set its header to **`private`**, max-age, immutable. `private` means shared caches (Cloudflare) **won't** store it — so today this is browser-cache only, and the path change just makes it *ready* for edge caching + clean cache rules. To actually get **edge** caching you'd additionally either flip BFC to `public` or add a Cloudflare rule that overrides cache-control — which means **accepting that the `id`+`checksum` URL becomes the access capability** (a cache hit is served without re-checking origin auth). The cache key is unique per component+build so there's no cross-workspace mixup, but the built JS effectively becomes public-by-URL (same posture PublicAsset already has). I've **left it `private`** here; flipping to `public` is your call and can be a one-line follow-up. ## Tests - `getFrontComponentUrl` unit test: fingerprinted path when a checksum is present, bare fallback otherwise. - Integration test: the `/front-components/:id/:checksum.js` path serves the built JS with `Content-Type: application/javascript` and `Cache-Control: private, max-age=86400, immutable`. Existing bare-route tests remain and still pass. https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W --- _Generated by [Claude Code](https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22530?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
41ef601a7a |
perf(twenty-server): cache BuiltFrontComponent and PublicAsset responses (#22523)
Follow-up to #22510. Closes #22515 — extends `Cache-Control` to the two remaining app-asset folders that #22510 left `immutable: false` because they're path-addressed. Each now gets the directive that matches **how it is addressed**. ## BuiltFrontComponent → immutable I was wrong in the #22515 write-up to call this "stable URL, mutable bytes." The browser **already content-addresses it**: `FrontComponentRenderer` fetches `/rest/front-components/:id?checksum=${builtComponentChecksum}` (`getFrontComponentUrl`), so a rebuild changes the checksum → changes the URL → busts the cache. That makes `immutable` safe — no stale-code window — and needs no new versioning machinery. Wired the header into `FrontComponentController.getBuiltJs` (which passed no folder) and the front-component presign path. ## PublicAsset → bounded public cache Genuinely path-addressed and overwritten in place on every app (re)install/redeploy (upsert on `['path','workspaceId','applicationId']`), so it **cannot** be `immutable`. Instead: - **`public`** — the `/public-assets/...` endpoint is unauthenticated (`PublicEndpointGuard`), so the bytes are already world-readable; marking the response `public` lets a CDN (e.g. Cloudflare in front of the server) serve app/marketplace logos from the edge instead of hitting the origin on every render. Today these responses carry no `Cache-Control` at all. - **`max-age=3600`, not `immutable`** — a bounded window so an asset overwrite recovers within an hour. This one hour is the single judgement call here; tune it (or add `stale-while-revalidate`) to taste. ## Mechanism Generalized `FileFolderConfig.immutable` (boolean) into `cacheControl` (`string | null`) so a folder can carry its own directive instead of only opting into one hardcoded string. `setFileResponseHeaders` and the presign paths now read `cacheControl` directly. The immutable-folder set is unchanged; only BuiltFrontComponent (→ immutable) and PublicAsset (→ bounded public) move. ## Tests `setFileResponseHeaders` spec updated: BuiltFrontComponent now asserts immutable, PublicAsset asserts `public, max-age=3600`, and the remaining path-addressed folders (`AppTarball`, `Source`, `BuiltLogicFunction`, `Dependencies`) assert no `Cache-Control`. _Note: I bundled both folders into one PR since they share the config generalization — happy to split BuiltFrontComponent (safe/immutable) from PublicAsset (the `max-age` judgement call) if you'd rather review them separately._ https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W --- _Generated by [Claude Code](https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22523?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Paul Rastoin
|
cdd667b106 |
fix(server): restrict /webhooks/server dispatch to server-route-exposed functions (#22469)
## What `ServerRouteTriggerService.findResolver` resolved a logic function purely by `universalIdentifier` and app-registration ownership, then executed it before its resolver result shape was validated. As a result the public `/webhooks/server/:universalIdentifier` route could dispatch any owner-workspace app function — including ones exposed only as authenticated HTTP routes, tools, or workflow actions — instead of only functions declared as server-route resolvers. ## Change `findResolver` now requires `serverRouteTriggerSettings`: - DB predicate `serverRouteTriggerSettings: Not(IsNull())`, so non-exposed functions are never fetched - in-memory `isDefined(...)` guard alongside the existing owner-workspace check A function that did not opt into server-route exposure is now rejected at `findResolver`, before any execution. A legitimately exposed resolver is unaffected. ## Tests - Unit (`server-route-trigger.service.spec.ts`): asserts the resolver query carries the exposure predicate, and that an owner-workspace function without `serverRouteTriggerSettings` is rejected and never handed to the executor. - Integration (`server-route-trigger-authorization.integration-spec.ts`): exercises the public endpoint end to end — a non-exposed owner-workspace function is rejected before execution, while a server-route-exposed resolver still passes the boundary. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22469?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Etienne
|
1270054d35 |
feat(ai): dashboard & view building (#22411)
## Why
Building a dashboard through AI chat used to cost ~9 sequential LLM
round-trips
(~160K input tokens for a single request): the agent had to resolve
object/field UUIDs and assemble views through many granular tool calls,
each
step replaying the full cached context.
## What changed
### 1. Reference objects & fields by name (fewer round-trips)
The agent no longer needs to resolve UUIDs before acting.
- `get_object_metadata`: filter by `objectName` (singular/plural) and a
new
`includeFields` flag returning each object's fields (`{id, name, type,
label}`)
inline — object + field IDs in one call.
- `get_field_metadata`: accepts `objectName` as an alternative to
`objectMetadataId`.
- All three dashboard write tools (`create_complete_dashboard`,
`add_dashboard_widget`, `update_dashboard_widget`): accept `objectName`
and
`*FieldName` variants (`aggregateFieldName`,
`primaryAxisGroupByFieldName`,
`secondaryAxisGroupByFieldName`, `groupByFieldName`, ratio `fieldName`),
resolved to UUIDs server-side by `resolveWidgetFieldNamesToIds`. UUID
variants
still win when both are given.
### 2. `upsert_complete_view` — one atomic call to build/reconfigure a
view
- New `upsert_complete_view` tool + `ViewService.upsertCompleteView`:
create or
update a view together with its fields, filters, and sorts.
- Children are **declarative**: a provided array replaces all existing
entries of
that kind, `[]` clears them, omitting leaves them untouched. Fields are
referenced by name or UUID; no child-row IDs needed.
- Runs as a **single workspace migration** (`view` + `viewField` +
`viewFilter` +
`viewSort` in one `validateBuildAndRunWorkspaceMigration` matrice)
instead of
chained per-entity service calls. New
`buildCompleteViewChildrenFlatOperations`
util assembles the child create/delete operations.
- Granular tools (`create_view_filter`, `update_view_sort`, …) are
retained for
surgical single-entry edits.
### 3. Chart filters on dashboard widgets (end-to-end)
- Added `chartFilterSchema` (`recordFilters` + optional
`recordFilterGroups` for
AND/OR logic) to the four chart configs, with field-by-name or -UUID
references
and documented operands/value formats.
- **Relative dates supported** — e.g. `PAST_7_DAY`, `THIS_1_MONTH`,
`NEXT_3_WEEK`,
plus open-ended `IS_IN_PAST` / `IS_IN_FUTURE` / `IS_TODAY`. Filters
route
through the same read pipeline (`computeRecordGqlOperationFilter`) as
view
filters, so they resolve and apply correctly.
- `resolveChartFilterFieldNamesToIds` resolves filter `fieldName` → id
against the
widget object.
### 4. Re-enable AI-assisted dashboards
- Removed the "coming soon" gating (`isActive: false` on the dashboard
skill and
the "not available yet" copy in the MCP server + chat prompts) and
registered
`DashboardToolProvider`.
- Rewrote the dashboard skill prompt: confirmation gate (present a plan,
wait for
confirmation), completion guard (once confirmed, emit the create tool
in-turn —
no "now let me…" preambles), default-and-proceed (pick sensible defaults
for
missing fields instead of stalling), and an intent gate so informational
dashboard questions are answered directly without loading skills.
### 5. Frontend: clearer advanced-filter labels
- `useRecordFilterField` now derives the filter label from field
metadata and
appends the relation target field (e.g. `Company → Name`), so
relation/target
filters — including those set by the AI — display correctly instead of
showing
a stale/blank stored label.
## Fixes
- **`get_object_metadata({ objectName })` crash.**
`ObjectMetadataService.findManyWithinWorkspace`
spread an array-form (`OR`) `where` into a plain object, producing
`{ "0": {...}, "1": {...}, workspaceId }` → `Property "0" was not found
in
"ObjectMetadataEntity"`. Now injects `workspaceId` into each OR clause,
so name
lookups work.
- **Invalid SELECT/MULTI_SELECT filter options silently produced broken
charts/views.**
Chart-configuration validation and the migration-layer
`FlatViewFilterValidator`
now reject filters that reference options that don't exist, with a clear
`Allowed values: …` message at creation time (shared
`getInvalidSelectFilterOptionValues` util + tests).
- **Non-atomic view assembly.** The previous multi-call view build could
leave a
half-built view on failure; `upsert_complete_view` now runs as a single
transaction (one validation pass, one cache recompute, rollback on
error).
- **Blank RECORD_TABLE widgets from UNLISTED views.** Guidance + the
upsert
ownership check steer widget-backing views to `WORKSPACE` visibility; an
UNLISTED view created without an owner renders a blank widget.
- **Extra discovery round-trip removed.** Deleted the skill→tool bundle
mechanism
(`SKILL_TOOL_BUNDLES`, `getBundledToolNamesForSkills`, and the
`load_skills`
schema-loading path) that forced a second `learn_tools` call.
- **Type-safety of widget resolution.** Reworked the widget resolver to
build a
properly typed `WidgetWithMetadataIds` (dedicated input/output types)
instead of
returning an untyped, cast-heavy object.
## Notes
- Backend changes are in `twenty-server`; one small `twenty-front`
change to the
advanced-filter label hook. No entity/schema changes, so no migration.
- Tests added: `getInvalidSelectFilterOptionValues`,
`resolveWidgetFieldNamesToIds`
(incl. filter/relative-date resolution), `update_dashboard_widget`, and
expanded
view-tools factory specs.
- Design decisions: dedicated composite tool over code-interpreter
orchestration
(atomicity + validation + consistency with `create_complete_dashboard` /
`create_complete_workflow`); name-or-UUID but no child-row IDs on
`upsert_complete_view`; name→id resolution kept as stateless utils, not
services.
## Test plan
- [ ] `npx nx run twenty-server:typecheck`
- [ ] `npx nx lint:diff-with-main twenty-server` and `twenty-front`
- [ ] `npx nx test twenty-server` (view tools factory,
`getInvalidSelectFilterOptionValues`,
`resolveWidgetFieldNamesToIds`, `update_dashboard_widget`)
- [ ] AI chat: "Create a dashboard with a chart of deal value by
pipeline stage
and a table of the top 10 open opportunities" → plans, waits for
confirmation, then builds with fewer round-trips
- [ ] AI chat: add a chart widget filtered by a relative date (e.g.
deals created
in `PAST_7_DAY`) and confirm the chart is actually filtered
- [ ] Filter on a non-existent SELECT option is rejected with a clear
error
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22411?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
|
||
|
martmull
|
1a85b88d38 |
feat(files): direct-to-storage upload endpoints with pending file lifecycle (#22449)
<img width="1484" height="404" alt="image" src="https://github.com/user-attachments/assets/b2d363bf-d9e1-49fb-9811-8cc98041aa79" /> ## Context Uploading large files currently OOMs the server: every upload resolver buffers the whole file in memory (`streamToBuffer`) before writing it to storage. This PR is the first of a series introducing direct client-to-storage uploads. It adds the server-side endpoints and driver support only — it is non-breaking and nothing consumes the new flow yet. Follow-up PRs will migrate the frontend upload paths, add a stale-pending-file cleanup cron, and cap the legacy buffered resolvers. ## What it does **New upload flow (initiate → PUT → confirm):** - `createFileUpload(filename, size, fileFolder, fieldMetadataId?)` validates the request (folder allowlist: `FilesField`/`Workflow`, max size, extension-derived mime type), creates the file record in a new `PENDING` status, and returns an upload target: - **S3 with presign enabled** → a presigned PUT URL with `Content-Type`/`Content-Length` pinned in the signature, so the client uploads straight to the bucket; - **local storage, or S3 without presign** → a token-authenticated streaming endpoint on the server (`PUT /file-upload/:id?token=…`, new `FILE_UPLOAD` JWT type) that pipes the request body to the storage driver with constant memory usage and a declared-size cap. - `completeFileUpload(fileId)` verifies the bytes actually landed in storage (HEAD + size match against the declared size) and flips the record to `UPLOADED`. Idempotent. **Pending lifecycle safety:** - New `status` column on `core.file` (`PENDING`/`UPLOADED`, default `UPLOADED` so all existing rows and the legacy upload path are unaffected) + fast instance command. - Files are refused by the serving endpoints and by FILES-field sync while `PENDING`. **Driver support (both drivers):** - `getPresignedUploadUrl` (S3: presigned PUT; local: `null` → server-endpoint fallback) - `writeFileStream` (local: `fs` pipeline with the existing symlink/containment hardening, partial-file cleanup on error; S3: `@aws-sdk/lib-storage` `Upload` for bounded-memory streaming) - `getFileMetadata` (HEAD/stat for confirm-time verification) ## Tests - `file-upload.service.spec.ts`: initiate validation (folder allowlist, size), presigned vs fallback target, confirm verification (missing object, size mismatch, happy path, idempotency) - `local.driver.spec.ts`: `writeFileStream` (content, symlink rejection, partial-file cleanup on stream error), `getFileMetadata` - `s3.driver.spec.ts`: `getPresignedUploadUrl` (disabled → null, PUT command with signed content-type/content-length) - `direct-file-upload.integration-spec.ts`: full end-to-end flow against the local driver (initiate → PUT → complete → download), plus error paths (complete without upload, oversized PUT → 413, invalid token → 403, unsupported folder, size above max) ## Notes for reviewers - The upload-size ceiling for direct uploads is `settings.storage.maxDirectUploadFileSize` (1GB), separate from the 10MB `maxFileSize` used for pictures. - Since content can't be sniffed before it reaches storage, the mime type is derived from the file extension (with the existing `TWENTY_MIME_POLICY` override) and unknown extensions fall back to `application/octet-stream`; the serving path already forces `Content-Disposition: attachment` for anything not on the inline-safe allowlist. - Self-hosters using S3 presign will need a bucket CORS policy allowing `PUT` from the frontend origin (config variable description updated). https://claude.ai/code/session_015UH8KWmsB9zdYaog8MFG1d --- _Generated by [Claude Code](https://claude.ai/code/session_015UH8KWmsB9zdYaog8MFG1d)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22449?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Paul Rastoin
|
0db4ddd46e |
fix(server): tolerate metadata-physical index drift in legacy index name normalization (#22472)
## Context The 2.18 `NormalizeLegacyIndexNames` workspace command (`1799200000000`, introduced in #22053) fails several production workspaces with `42P01 relation … does not exist`, rolling back the whole per-workspace upgrade transaction and marking the workspace `Failed`. ## Root cause The command assumes the physical index in the workspace schema is named exactly as recorded in `core."indexMetadata"."name"`. For workspaces where the physical index was already rebuilt/renamed under the v2 deterministic name (only targeted phone/relation rebuilds got new names after #14567) while metadata kept the legacy hash, the rename source no longer exists, so `ALTER INDEX … RENAME` aborts the entire workspace upgrade. (The duplicate-drop path uses `DROP INDEX IF EXISTS` and is unaffected.) ## Fix - **`WorkspaceSchemaIndexManagerService`**: new `doesIndexExist` and `getIndexDefinition` helpers querying `pg_indexes` for a `(schema, index)` pair. `renameIndexWithoutRebuild` keeps its strict semantics (no `IF EXISTS`) — drift tolerance lives in the command, which is the only caller that expects it. - **`NormalizeLegacyIndexNamesCommand`** — the rename operation now reconciles drift instead of blindly renaming: - Target name already exists physically, source gone → skip the rename, just point `indexMetadata.name` at it (the common "physical already v2, metadata still legacy" case). - Both source and target exist physically → compare their `pg_indexes.indexdef` ignoring the name: if identical, drop the legacy duplicate (it would otherwise be orphaned forever since metadata stops referencing it, adding permanent write/maintenance cost); if the definitions differ, keep it in place and log a warning. - Source exists, target free → rename as before, then update metadata. - Neither exists → log a warning and update metadata so a future rebuild recreates the index under the expected v2 name. In every branch the metadata name ends up on the recomputed v2 name, and no missing physical index can abort the workspace transaction anymore. ## Tests - Regression tests on the command spec for the four drift cases (target-already-renamed, both-missing, both-present-identical → drop, both-present-different → keep); existing rename/duplicate/dry-run/rollback tests updated to declare the physical indexes present. - New spec for `WorkspaceSchemaIndexManagerService` covering the rename SQL, the `pg_indexes` existence check, and the definition lookup. - New spec for `areIndexDefinitionsEquivalent` (name-only diff, uniqueness, columns, where clause, malformed input). `npx jest` on all three specs (20 passed), `lint:diff-with-main` and `typecheck` green. |
||
|
neo773
|
3c3a8078fe |
fix email alias guard with message channel availibility (#22521)
<!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22521?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
martmull
|
0992d0b748 |
feat(server): promote registration display fields to first-class columns (#22513)
Part of the application settings architecture work: https://github.com/twentyhq/core-team-issues/issues/2456 — follow-up to #22453, delivering the promised removal of the temporary manifest load. Display data (description, author, category, websiteUrl, aboutDescription, termsUrl, emailSupport, issueReportUrl, screenshots) only existed inside the `manifest` jsonb, forcing hot paths to load it. This PR: - Promotes those 9 fields to first-class columns on `applicationRegistration`, populated at every ingestion point (`updateFromManifest`, both `upsertFromCatalog` branches) — fast command creates the columns at deploy, slow command backfills them from the manifest. - `findManyListedCatalogCards()` (marketplace list) now selects only scalar columns — the manifest jsonb is no longer loaded there. - `findPublicByClientId()` (OAuth consent page) now selects `id, name, logo, websiteUrl, oAuthScopes` — no manifest. - The narrow select used by `findMany`/`findAll`/`findOneById`/`findOneByIdGlobal` includes the new columns. - GraphQL surface unchanged (no new fields); the marketplace detail endpoint still reads the manifest and is slimmed in the next PR. Verified: migration applied via the real runner (both commands recorded completed), backfill SQL exercised against live rows (full + minimal manifests), migration generator reports no pending schema changes, typecheck, lint, unit suites (application-registration + marketplace 15/15). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011sST4rPLU1Koi2oVGi84ei --- _Generated by [Claude Code](https://claude.ai/code/session_011sST4rPLU1Koi2oVGi84ei)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22513?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
5d3b6d05b3 |
feat(ai): add a stream heartbeat and reap dead claims so a worker crash cannot brick a thread (#22482)
## Rationale If the worker process dies mid-stream (OOM, deploy, crash), nothing ever clears `activeStreamId`: `aiStreamQueue` runs with `attempts: 1`, the job's `finally` never executes, and the SSE keepalive comes from the API server — so it actively masks worker death. The thread is bricked: every send queues behind a dead claim until someone intervenes manually. This is a CONFIRMED-high from the chat-stack audit, and worker death is not hypothetical: Sentry shows an unhandled promise rejection inside the AI SDK in the worker ([TWENTY-SERVER-H7Y](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-H7Y)) — unhandled rejections terminate Node by default. ## Design - **Claim-time mark**: every enqueue site marks `agent-chat-stream-alive:<streamId>` with a TTL matching the job lock horizon (600s) — covering the enqueue→pickup window where a waiting job holds no lock. - **Running refresh**: the job tightens it to **30s, refreshed every 5s**; if the process dies, the interval dies with it and the key expires. The expiry *is* the death signal. (Was 60s/15s — tightened after review: detection latency is bounded by the TTL, robustness by TTL−interval and the missed-beat tolerance; 30s/5s halves detection while tolerating *more* missed beats, 5 vs 3.) - **Read-path reap**: the send gate and the catchup query convert a heartbeat-less claim into a normal retryable `STREAM_INTERRUPTED` failed-turn state (conditional UPDATE guarded on the observed streamId, so a newer stream's claim is never touched), reset the Redis chunk state, and publish the terminal error. `isAlive` fails open on Redis errors — a liveness probe must not turn a Redis blip into a broken send path. ## Why this is the root cause, not a symptom patch The strongest alternative — BullMQ's own stalled-job detection — fails on four concrete grounds: detection latency is bounded by the deliberate 10-minute `AI_STREAM_LOCK_DURATION_MS` (long silent tool runs must not spuriously stall); the stalled checker needs a *surviving* worker in the pool; the signal fires in the worker process while the thing needing repair is a DB claim read by API-server resolvers; and a `waiting` job holds no lock at all. Reaping at the read path means recovery happens exactly when a user is looking — the moment it matters — with zero background machinery. **Relationship to the graceful-shutdown work (planned follow-ups)**: shutdown hooks + drain-then-abort will make *deploys* (cooperative SIGTERM) end streams cleanly, and disabling stalled re-runs will stop hard-killed jobs from zombie re-executing tools. This PR remains the only recovery layer for non-cooperative deaths — OOMKill is a straight SIGKILL, crashes and unhandled rejections never run shutdown hooks — and the backstop when the drain path itself fails. The two are complements, not alternatives. ## User impact Today a worker crash mid-answer bricks the thread until manual intervention; users see sends silently queue forever. With this, the next interaction (send, reload) converts it into a visible "response was interrupted" error with a working Retry, within ~30s of actual death. ## Test plan - [x] Claim spec: live stream untouched; heartbeat-less claim reaped into retryable `STREAM_INTERRUPTED` + chunk-state reset + published terminal event; no-op when the claim moved to a newer stream mid-check - [x] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 |
||
|
Charles Bochet
|
203da78b05 |
feat(logic-function): namespace shared Lambda resources per instance (#22509)
## Problem When several Twenty instances run in the **same AWS account + region** (our customer's setup), their logic-function Lambda resources collide. Four shared resources are named purely from a content hash: | Resource | Name | Derived from | |---|---|---| | Builder fn | `twenty-builder-<sha256(handler)>` | handler code (⇒ version) | | Yarn-install fn | `twenty-yarn-install-<sha256(handler)>` | handler code | | Common layer | `twenty-common-layer-<sha256(pkg+lock)>` | dependencies | | Deps layer | `deps-<yarnLockChecksum>` | app yarn.lock | Because the hash is identical across instances of the same version, every instance computes the **same name**. `ensureBuilderLambdaExists` / `ensureYarnInstallLambdaExists` do `GetFunction → if it exists, return` with **no role check**, so the first instance to create the function binds it to *its* `LOGIC_FUNCTION_LAMBDA_ROLE` and every other instance silently reuses it. When that role is later deleted or belongs to a different account, invokes fail with: > The role defined for the function cannot be assumed by Lambda. Nothing tears these shared resources down, so a poisoned function persists indefinitely. (Executors are UUID-named and SDK layers are workspace-scoped, so they don't collide.) ## Fix Namespace the four shared resources by a per-instance segment. - New optional config var **`LOGIC_FUNCTION_LAMBDA_RESOURCE_NAMESPACE`** (LOGIC_FUNCTION_CONFIG group). - When unset it defaults to `sha256(LOGIC_FUNCTION_LAMBDA_ROLE).slice(0, 10)`. Keying the namespace on the execution role makes the sharing boundary correct: - **same role → same names →** instances still dedupe (original intent preserved), - **different role → different names →** full isolation, invoke can never hit a role it can't assume, - **role change →** resources are recreated fresh under a new name (self-healing). Names become `twenty-builder-<namespace>-<checksum>`, `deps-<namespace>-<checksum>`, etc. — the owning instance stays legible in the AWS console. ### Defensive role-heal As a safety net (and to heal already-poisoned functions), after `GetFunction` succeeds we compare `Configuration.Role` to the configured role; on mismatch we delete and recreate the tool function. (`waitFunctionDeleted` polls `GetFunction` until `ResourceNotFoundException` — this SDK version has no `waitUntilFunctionNotExists` waiter.) ## Scope / compatibility - Executor functions (`<logicFunctionId>`) and SDK layers (`sdk-<workspaceId>-<appUUID>`) are unchanged — already unique. - On upgrade, shared-resource names change once (role-hash namespace), so each instance recreates its builder/yarn-install/common-layer/deps on first use; old ones are orphaned (harmless, unreferenced). - Operators who want explicit control can set `LOGIC_FUNCTION_LAMBDA_RESOURCE_NAMESPACE`. ## Ops note (immediate unblock, independent of this PR) Delete the poisoned `twenty-builder-<hash>` (and sibling `twenty-yarn-install-*`) in the affected region; it is recreated with the correct role on next use. ## Tests - `compute-hashed-lambda-resource-name.util.spec.ts` — namespace segment behavior - `get-lambda-deps-layer-name.util.spec.ts` — namespaced deps layer name - `get-lambda-resource-namespace.util.spec.ts` — stable, role-distinct namespace All pass; `lint:diff-with-main` clean; typecheck clean for changed files. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22509?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
8b191d6fcc |
chore(server): remove the five dead FileFolder values and their legacy serving pipeline (#22516)
Follow-up cleanup after #22510: shrink `FileFolder` and `fileFolderConfigs` to only folders that actually exist, so per-folder policy entries are real decisions. ## What **Remove the five dead enum values** — `ProfilePicture`, `WorkspaceLogo`, `Attachment`, `PersonPicture`, `File`. They were already marked replaced/removed in the enum, have no production write path, and `FileByIdGuard`'s `SUPPORTED_FILE_FOLDERS` allowlist already rejects them at the serving endpoint. **Delete the legacy path-based serving pipeline that existed only for them** — verified wired to no route: - `FilePathGuard` — registered as a provider in `FileModule` but applied to no controller - `extractFileInfoFromRequest` (parsed the old `/files/profile-picture/original/TOKEN/file.jpg` format) — only consumer was `FilePathGuard` - `checkFileFolder` — only consumer was `extractFileInfoFromRequest` - `settings.storage.imageCropSizes` — keyed exclusively by the three dead picture folders, zero consumers - the crop-size helpers in `utils/image.ts` (`getCropSize`, `ShortCropSize`, `CropSize`) — zero consumers outside the file; `getImageBufferFromUrl` is kept - `AllowedFolders` type — last consumer was `checkFileFolder` **Test fixtures** referencing dead folders were moved to living ones; the specs of deleted utils are deleted with them. **Generated files** (`twenty-front/src/generated-metadata/graphql.ts`, `twenty-client-sdk` schema) hand-updated to match the shrunk GraphQL enum. ## Legacy data safety Workspaces may still hold `File` rows whose `path` starts with a dead prefix (e.g. `attachment/…`). These stay inert, exactly as today: - Serving: `FileByIdGuard` rejects non-supported folders before any config lookup, and file lookups filter by `path LIKE '<current-folder>/%'`, so dead-prefix rows are unreachable. - Every consumer that feeds stored paths into `removeFileFolderFromFileEntityPath` (which throws on unknown prefixes) is upstream-guarded by a current-folder filter or allowlist — audited all seven call sites. - Stored legacy member `avatarUrl` strings are parsed with `extractFileIdFromUrl(url, FileFolder.CorePicture)` and already fall back to `''` for old formats; unchanged. ## GraphQL note `FileFolder` is exposed as a GraphQL enum (input of the dev-only `uploadApplicationFile` mutation, which only accepts application-code folders). Clients sending a removed value were already rejected at the resolver allowlist; they now fail GraphQL enum validation instead. No supported client sends them — the frontend only uses `CorePicture`. Net: **+10 / −301** across 17 files. https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W --- _Generated by [Claude Code](https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22516?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
8156bf2b89 |
perf(twenty-server): drive immutable file caching from fileFolderConfigs on both serving paths (#22510)
Follow-up to #22166 (now merged). Single commit, rebased onto main. ## Why #22166 introduced a `Cache-Control` header for avatar responses, gated on a hardcoded `CACHEABLE_PICTURE_FILE_FOLDERS = [CorePicture]` list. Two limitations: - The list is an ad-hoc second classification of `FileFolder`, maintained separately from the central `fileFolderConfigs`. - The header is only set on the stream branch of `getFileById`. On S3 deployments with presigned URLs enabled, the controller 302-redirects before `setFileResponseHeaders` runs and the presigned S3 response carries no `Cache-Control` at all — so the header never fires where it matters most. Whether a folder's bytes are cacheable-forever is a property of how the folder is written, and the codebase already has a per-folder source of truth: `fileFolderConfigs`. ## What - Add `immutable: boolean` to `FileFolderConfig`. `true` for folders whose write paths mint a fresh `v4()` file id embedded in the resource path on every upload — so the bytes behind a given URL can never change: `CorePicture`, `FilesField`, `Workflow`, `AgentChat`, `EmailAttachment`, `Dpa`. `false` everywhere else, notably: - `PublicAsset` — path-addressed, overwritten in place on app (re)install (including the new manifest logo import) - `AppTarball` — reuses `tarballFileId` and a stable `${registrationId}/app.tar.gz` path across version bumps - `setFileResponseHeaders` reads the flag instead of the ad-hoc list (list deleted). - Thread `responseCacheControl` through `FileStorageService.getPresignedUrl` → `StorageDriver` → `S3Driver`, which passes it as `ResponseCacheControl` on the `GetObjectCommand`, so presigned S3 responses return the same `Cache-Control: private, max-age=86400, immutable` on the redirect path. `private` is kept because responses are gated by a per-workspace file token; `immutable` is safe because a changed file always gets a new id and URL. ## Tests - `setFileResponseHeaders` spec: header set for each immutable folder, not set for mutable folders (`PublicAsset`, `AppTarball`, deprecated picture folders) or when no folder is provided. - `S3Driver.getPresignedUrl` spec: asserts `ResponseCacheControl` is forwarded onto the `GetObjectCommand`. https://claude.ai/code/session_01AKwhTxYFDhWhCZ4b7sf35W |
||
|
Abdul Rahman
|
9a7c0f25a5 |
fix(server): resolve foreign key violation blocking application uninstall (#22502)
Fixes [sonarly issue #54192](https://sonarly.com/issue/54192) ## Problem Uninstalling an application fails with a DB error when its `packageJsonFileId` / `yarnLockFileId` columns are populated: update or delete on table "file" violates foreign key constraint "FK_3818380258798f9ffa9963b6dc4" on table "application" Storage was also wiped before the failing DB delete, leaving the app half-uninstalled. ## Root cause `application` and `file` reference each other through `ON DELETE RESTRICT` FKs (`application.packageJsonFileId/yarnLockFileId → file.id` and `file.applicationId → application.id`), so no deletion order works on its own. The deferrable-FK migration doesn't help: in Postgres, `RESTRICT` fires immediately even on `DEFERRABLE INITIALLY DEFERRED` constraints (only `NO ACTION` honors deferral). Uninstall deleted file rows first, in autocommit statements. ## Fix `ApplicationService.delete()` now runs in a single transaction: 1. Clear `packageJsonFileId` / `yarnLockFileId` (breaks the FK cycle) 2. Delete the app's `file` rows 3. Delete the `application` row Storage cleanup moved after commit and made non-fatal, so a failure can no longer leave partial state. `deleteApplicationFiles` is split into `deleteApplicationFileRows` (DB, transactional) and `deleteApplicationFilesFromStorage` (blobs). The test cleanup util had the same file-first ordering bug and is fixed the same way. ## Questions / Follow-ups - **Should the FK cycle be resolved at the schema level?** Both legs could be switched to `ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED`, which appears to be what the deferrable-FK migration intended — deferral would then actually apply to deletes, making transactional deletion order-independent. Happy to open a separate PR if there's interest. - **Should the marketplace install path set the package file FKs?** It stores `package.json` in the `file` table but never populates `application.packageJsonFileId` / `yarnLockFileId` — today only workspace creation and `application:rebuild-default-deps` set them. Marketplace packages also don't ship a `yarn.lock`, so this needs a product decision. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22502?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
martmull
|
af1b89c788 |
feat(server): import application logo into file storage at install (#22437)
Installed apps stored the logo as the manifest's relative path but never imported the file, so the public-assets URL 404'd and logos went missing in the UI for npm/tarball sources. Import the logo (best-effort — a declared but unshipped logo is skipped, not fatal) and record it as a first-class logoFileId on the application so it can be served reliably. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22437?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
martmull
|
20f2e33702 |
feat(server): first-class logo on application registration + narrowed settings queries (#22453)
Part of the application settings architecture work: https://github.com/twentyhq/core-team-issues/issues/2456 Application-registration list queries loaded the entire `manifest` jsonb (potentially 100KB+/row) on every settings/marketplace list request because display data (logo, description, author, category) only exists inside it. This PR: - Adds a first-class nullable `logo` column on `applicationRegistration`, populated at every ingestion point (`updateFromManifest`, `upsertFromCatalog`) and backfilled from `manifest->application->>logoUrl` via a slow instance command (self-sufficient backfill since `runDataMigration` runs before `up`). - Backs the `logoUrl` GraphQL getter with the column (manifest fallback for un-backfilled rows) — **GraphQL surface unchanged**. - Narrows `findMany` / `findAll` / `findOneById` / `findOneByIdGlobal` to an explicit scalar select that excludes `manifest` and `oAuthClientSecretHash` (every caller audited — none needs them; OAuth verification paths are untouched). - Replaces `findManyListed()` with `findManyListedCatalogCards()`: a projection query that extracts the four display strings from the manifest in SQL (with explicit soft-delete filtering) instead of hydrating full entities, feeding `findManyMarketplaceApps`. Verified: typecheck, lint:diff-with-main, unit suites (application-registration 5/5, marketplace 10/10, instance-command 31/31), migration applied via the real runner, and the migration generator reports no pending schema changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011sST4rPLU1Koi2oVGi84ei --- _Generated by [Claude Code](https://claude.ai/code/session_011sST4rPLU1Koi2oVGi84ei)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22453?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Clive F
|
f2e3eb5fb7 |
perf(twenty-server): browser-cache picture file responses (avatars/logos) (#22166)
Fixes #22163. ## Why Picture file responses (`GET /file/:fileFolder/:id`, `FileController.getFileById`) set no `Cache-Control` header, so the browser re-fetches the same avatar/picture on every render. When one member's avatar appears many times on a page (e.g. a record table or Kanban where that member owns many rows), this fires dozens of parallel GETs for the identical image; the browser cancels the redundant in-flight ones, and the server logs each client-aborted stream as `Error streaming file from storage`. Picture files are content-addressed by an immutable file id — changing an avatar or logo mints a new file id (and therefore a new URL) — so the bytes at any given URL never change and can be cached aggressively. ## What - `setFileResponseHeaders` now adds `Cache-Control: private, max-age=86400, immutable` for the picture folders (`CorePicture`, `ProfilePicture`, `WorkspaceLogo`, `PersonPicture`); `getFileById` passes the `fileFolder` through. - Scoped to picture folders so non-image files (attachments, tarballs, source, …) are not cached past a permission/visibility change. - `private` because files are served behind a per-workspace file token; `immutable` + the content-addressed id gives automatic cache-busting when the picture changes. ## Tests - Unit tests for `setFileResponseHeaders`: header is set for each picture folder, and not set for non-picture folders or when no folder is provided. - Controller test asserts the header on a `CorePicture` stream response. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22166?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Charles Bochet <charles@twenty.com> Co-authored-by: Félix Malfait <felix.malfait@gmail.com> |
||
|
martmull
|
25fe66565c |
feat(applications): add type and options to application variables (#22157)
## Before <img width="1452" height="709" alt="image" src="https://github.com/user-attachments/assets/cd384ffa-cbe6-49d5-a807-ca8d580f55a9" /> <img width="1074" height="452" alt="image" src="https://github.com/user-attachments/assets/720d38db-3495-4032-8831-17d24ec6a7e7" /> ## After <img width="1421" height="865" alt="image" src="https://github.com/user-attachments/assets/2275c996-c895-4800-8324-2aa2ddfddd43" /> <img width="1348" height="870" alt="image" src="https://github.com/user-attachments/assets/3e1a891d-6db0-4cbd-870a-2a5bbde4929d" /> ## Summary Adds typed application variables with optional select **options**. This is the other half of #22059, split out from the custom-settings-tab removal. ## Changes - **Shared types**: `ApplicationVariable` / `ServerVariables` gain an optional `type` (a `FieldMetadataType` subset — `TEXT`, `BOOLEAN`, `NUMBER`, `DATE`, `SELECT`, `MULTI_SELECT`, `RAW_JSON`, `RICH_TEXT`, `ARRAY`, …) and select `options`. New `serializeApplicationVariableValue` / `deserializeApplicationVariableValue` helpers convert typed values to/from the encrypted string storage. - **Server**: `type`/`options` columns on `applicationVariable` and `applicationRegistrationVariable` (entities + DTOs), a fast `2-17` instance command, manifest processing via the serialization helpers, and a `QueryDeepPartialEntity` cast where the manifest JSON column is persisted. - **Frontend**: a polymorphic `SettingsApplicationVariableInput` that renders the native `Form*` field component for each type (boolean, number, date/date-time, select, multi-select, array, raw JSON, rich text, text); fragment/query updates to fetch `type`/`options`. - **SDK**: `defineApplication` validates that `SELECT`/`MULTI_SELECT` variables declare non-empty `options` at build time (since `options` is kept structurally optional for TypeORM/SDK compatibility). Variables default to `TEXT` when no type is given, so existing manifests are unaffected. ## Notes The generated GraphQL artifacts (`type`/`options` on the variable types) are regenerated by codegen; that change accompanies this PR. https://claude.ai/code/session_013Z7UB35V2mvUozh55QHG23 --- _Generated by [Claude Code](https://claude.ai/code/session_013Z7UB35V2mvUozh55QHG23)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22157?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Etienne
|
90f35658c5 |
fix(ai) - sync AI agent step output schema when agent response format changes (#22466)
## Summary
When an AI agent workflow step is built via the AI chat tools, its
persisted
`settings.outputSchema` was left empty (or stale as a text `{ response
}` schema)
even after the agent was given a structured JSON `responseFormat`. The
workflow
still executed correctly (runtime uses actual step results), but the
builder UI
resolves downstream variables (`{{stepId.fieldName}}`) exclusively from
the
persisted `outputSchema`, so those variables showed as **"Not Found"**.
Root cause: the `update_agent` tool only mutated the agent entity and
never
re-derived the linked step's `outputSchema`, and `enrichOutputSchema`
did not
handle `AI_AGENT` steps at all.
## What changed
- **Enrich AI_AGENT output schema on the backend**: added `AI_AGENT` to
`BACKEND_ENRICHED_TYPES` in
`WorkflowSchemaWorkspaceService.enrichOutputSchema`,
so a step's `outputSchema` is computed from the agent's `responseFormat`
on
every create/update (text → `{ response }`, JSON → one field per
property).
- **Re-sync the step when the agent's response format changes**: after
`update_agent` sets a `responseFormat`, the tool now finds the draft
workflow
version(s) whose `AI_AGENT` step references that agent and re-runs the
step
update so the persisted `outputSchema` is regenerated.
- **Fix stale-cache read**: `updateOneAgent` reads `flatAgentMaps`
before its
migration, which can leave a memoized/local stale copy for a few
seconds. The
resync now invalidates `flatAgentMaps` before re-enriching, so the fresh
`responseFormat` is used.
- **Surface failures**: resync errors are logged (`UpdateAgentTool`)
instead of
failing silently; the agent update itself still succeeds.
- Added unit tests for the `update_agent` resync behavior (fires on
`responseFormat` change, invalidates the cache, skips unrelated agents,
and
reports success when the resync fails).
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22466?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
|
||
|
Félix Malfait
|
9ef1af9799 |
fix(ai): make stream claims atomic via conditional UPDATEs with claim-or-queue send (#22481)
## Rationale
`activeStreamId` is the mutex that guarantees one live stream per thread
— but claiming it is a plain read-then-update. The resolver checks it,
then `streamAgentChat` enqueues the job **before** writing the claim.
Two racing sends both pass the check, both start jobs, and each job's
`resetStreamState` wipes the other's Redis chunk list — tokens from two
answers interleave into the visible message. The same window exists for
retry vs. send, the queue drain vs. send, and `stopAgentChatStream`,
which cleared the claim **unguarded** (`{ id, userWorkspaceId }`) and
could wipe a newer stream's claim entirely.
## Why this is the root cause, not a symptom patch
Ownership must live in the `activeStreamId` column regardless of any
locking mechanism — the queue-behind gate, the thread DTO, and stop all
read it. So the correct primitive is a single-row compare-and-set on
that column: `UPDATE … WHERE "activeStreamId" IS NULL` checked via
affected rows, claim **before** enqueue, release on enqueue failure.
Every mutation of the claim is now guarded on the observed value.
Alternatives evaluated and rejected:
- **BullMQ jobId dedup by threadId**: the driver appends a `-${v4()}`
suffix to custom ids and dedups via a non-atomic `getJobs(['waiting'])`
scan that ignores active jobs — two racing sends still run concurrently,
and it does nothing for stop/retry races.
- **`SELECT FOR UPDATE` / Redis SETNX / advisory locks**: all add a
second mechanism (transaction plumbing or a second source of truth) to
protect a single-row write that Postgres can already do atomically.
Path-specific claim predicates fall out naturally: send/drain claim with
`pendingQuestionMessageId IS NULL`, retry claims with `lastStreamError
IS NOT NULL` (and restores the error if its enqueue fails) — closing the
double-retry race for free.
## User impact
Double-send (impatient double-click, two tabs, retry racing a queued
drain) can currently garble the assistant's answer with interleaved
tokens from two model runs and strand one stream's claim. All of these
become deterministic: exactly one winner streams; the loser queues
politely.
## Test plan
- [x] New claim spec: conditional claim before enqueue, race-loser
queues, halted-backlog send queues at the back and kicks the drain
front-first, enqueue-failure releases the claim
- [x] Retry spec updated: rollback restores the prior `lastStreamError`;
guarded shapes asserted
- [ ] CI green
https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22481?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
|
||
|
martmull
|
13be2188cc |
Fix non-idempotent application sync for viewSorts (subFieldName undefined vs null) (#22505)
## Summary Successive application syncs (`yarn twenty dev --once`) kept reporting the same viewSorts as updated, even with no manifest changes. The manifest converter never set `subFieldName`, so the manifest-derived flat viewSort carried `undefined` where the flat viewSort computed from the database carried `null`. The comparator (microdiff) treats `null` vs `undefined` as a change, producing a phantom update action on every sync that never converges — the resulting update is a no-op on the database. Fixes twentyhq/core-team-issues#2629 ## Changes - **Converter**: `fromViewSortManifestToUniversalFlatViewSort` now sets `subFieldName: viewSortManifest.subFieldName ?? null`, matching how the sibling converters (e.g. view filters) handle optional compared properties. - **Type definition**: added optional `subFieldName?: string` to `ViewSortManifest` in `twenty-shared`, mirroring `ViewFilterManifest` — this also makes sorts on composite sub-fields (e.g. `amountMicros`) expressible in app manifests, which the entity already supports. - **Tests**: - Asserts `subFieldName` is `null` (not `undefined`) when omitted — the idempotency regression. - Asserts `subFieldName` is passed through when provided. ## Verification - All 12 application-manifest converter suites pass (47 tests). - Flat-entity comparison/constants suites pass (36 tests, 21 snapshots). - `subFieldName` was already part of the viewSort compare properties, so no comparator/constants changes needed. https://claude.ai/code/session_018FrD42MMQtu1UvDyiEZbSq <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22505?utm_source=github" rel="nofollow noreferrer noopener" target="_blank">``<img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">``</a> |
||
|
Abdul Rahman
|
429e8c4b84 |
fix: align email validation between front and server and roll back optimistic value on failed save (#22490)
## Summary
Inline edits of EMAILS fields could leave the UI in a misleading state:
the frontend validated with Zod's default `z.email()` while the server
used the stricter `z.regexes.unicodeEmail` pattern (which caps the local
part at 64 characters). A very long email passed client validation and
was optimistically written to the UI; the server then rejected the
mutation. An error snackbar was shown, but the field kept displaying the
unsaved value until a page reload.
## Changes
- **Single source of truth for email validation**: added a shared
`emailSchema` (`z.email({ pattern: z.regexes.unicodeEmail })`) in
`twenty-shared/utils`, now used by:
- the server-side EMAILS field validator
(`validate-emails-primary-email-subfield-or-throw.util.ts`)
- the `EmailsFieldInput` inline editor
- spreadsheet import validation
- **Rollback on failed save**: `useUpdateOneRecord` now restores the
optimistically updated fields in the record store when the mutation
fails, mirroring the store upsert already done in the success path.
Previously the catch block only rolled back the Apollo cache — which
stopped reverting the UI after table virtualization, since the record
store (the render source of truth) is no longer synced reactively from
the cache. The error is still rethrown, so the existing global
promise-rejection handler keeps showing the error snackbar. This fixes
the stale-value-until-reload behavior for all field types and all
callers, not just EMAILS fields.
- **Regression tests**: added unit tests for the shared schema,
including the >64-character local part case.
Fixes [sonarly issue
#54034](https://sonarly.com/issue/54034?share=eyJ0aWQiOjMzMCwidHlwIjoiYnVnIiwicmlkIjo1NDAzNCwiZXhwIjoxNzgzNTI1OTQzfQ.9e7639034a677301512fceeafab764b1)
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22490?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
|
||
|
Félix Malfait
|
024a9b4d94 |
fix(upgrade): re-slot all 2-19 upgrade commands to their real merge epochs and guard timestamps in CI (#22498)
# Fix broken 2-19 upgrade command sequence (dev incident: `lastStreamError` / `workspaceDiscoverability`) ## Incident The dev environment (tracking main) throws: - `Property "lastStreamError" was not found in "AgentChatThreadEntity"` - `Cannot return null for non-nullable field Workspace.workspaceDiscoverability` ## Root cause The 2-19 upgrade commands were committed with **fabricated future timestamps** (year-2027 epochs like `1820000000000`). The upgrade cursor (`upgrade-aware-entity-metadata.adapter.ts`) tracks a **single** most-recent applied step: it looks up the latest `core."upgradeMigration"` row's name in the sequence (sorted by timestamp within kind) and hides every `@WasIntroducedInUpgrade` column at or past that index. Two ways this breaks, and both were live on main: 1. **Cursor regression**: a command merged *later* with a *smaller* timestamp (e.g. `pendingQuestion` at `1811…` after `metadata-overrides` at `1820…` had run) sorts *before* already-applied steps. When migrate runs, the "latest" row now points earlier in the sequence, re-hiding columns that were already applied. 2. **Migrate not running at all** (Felix's hypothesis): if the deploy pipeline skipped `database:migrate:prod`, none of the 2-19 rows exist and every 2-19-gated column is hidden. Both hypotheses have the same fix path; the discriminating query is in the verification section below. ## Fix **Real timestamps** (per maintainer direction — no more fabricated epochs): | Command | Old (fabricated) | New (real merge epoch) | Introduced by | |---|---|---|---| | workspace: backfill-workspace-custom-application-registration | `1820000000000` | `1782853718000` | #22378 | | fast: add-metadata-overrides-column | `1820000100000` | `1782986475000` | #22417 | | slow: backfill-metadata-overrides | `1820000110000` | `1782986476000` (+1s to order after its fast pair) | #22417 | | fast: add-last-stream-error-to-agent-chat-thread | `1821000000000` | `1782996657000` | #22434 | | fast: add-pending-question-to-agent-chat-thread | `1811000000000` | `1782999138000` | #22346 | | fast: add-workspace-discoverability-to-workspace | `1820000001000` | `1783004140000` | #22423 | Each value is the committer epoch of the squash-merge commit that introduced the command on main (verified via `git log --diff-filter=A`). Sorted by real time, the fast sequence is strictly increasing, so the cursor can no longer regress. **Idempotency**: renaming a command changes its step name, so every one of these re-runs on any instance that already applied it under the old name (dev cluster, edge self-hosters — 2.19 is unreleased, so tagged releases are unaffected). All six are now safe to re-run: - `lastStreamError`, `pendingQuestion`, `metadata-overrides` fast: `ADD COLUMN IF NOT EXISTS` (already were) - `metadata-overrides` slow backfill: `WHERE … IS NULL` guard (already was) - workspace command: skips when `applicationRegistrationId` is already set (already did) - `workspaceDiscoverability`: **made idempotent in this PR** — `CREATE TYPE` wrapped in a `duplicate_object` handler, `ADD COLUMN IF NOT EXISTS` **CI guard** (replaces the append-only check added earlier on this branch): - Timestamps must be **real**: within `[now − 60 days, now + 2 days]`. This is the check that would have prevented the original sin — 2027 epochs can never pass. - Still **append-only** within the version directory, but computed from `git diff --name-status --find-renames` so renamed/copied files are checked too (cubic's P2), and files the PR deletes/renames away no longer count toward the existing max (otherwise a re-slotting PR like this one could never pass its own guard). - Covers `workspace-command-<ts>-` filenames, not just `instance-command-fast|slow-<ts>-`; skips `.spec.ts` files. - Failure message documents the escape path (re-slot the fabricated blocker to its real epoch + make it idempotent) and a bypass label `ci:allow-upgrade-command-timestamp-exception` for deliberate exceptions. ## Deploy sequencing (important) After this merges and deploys, `database:migrate:prod` **must run** before the API pods are relied on: the old step names no longer exist in the sequence, so until the renamed commands run once, the cursor resolves to 0 and *every* gated column is hidden. The commands are idempotent, so the re-run is harmless. Running API/worker pods only compute the cursor at boot — restart them after migrate. ## Verification / diagnosis on dev ```sql SELECT name, status, "createdAt" FROM core."upgradeMigration" WHERE "workspaceId" IS NULL ORDER BY "createdAt" DESC LIMIT 15; ``` - Latest rows named `…_182xxxxxxxxxx` (fabricated) and completed → migrate ran, cursor regressed (hypothesis 1). - No 2-19 rows at all → migrate never ran for 2-19 (hypothesis 2). - After the fix: latest row should be `2.19.0_AddWorkspaceDiscoverabilityToWorkspaceFastInstanceCommand_1783004140000`, status `completed`. https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 |
||
|
Félix Malfait
|
5057f14df9 |
test(ai): pin the queue-updated event that now follows stream-error on the failure path (#22500)
## Rationale main's `server-test` is red: 4 error-path tests in `stream-agent-chat.job.spec.ts` fail. #22494 made the stream failure path publish `queue-updated` **after** the terminal `stream-error` (so background tabs refetch the persisted partial transcript), but these specs still asserted `stream-error` is the *last* published event. Classic squash-merge semantic conflict — #22494's branch predated the spec assertions, both were green in isolation. ## Why this is the right fix (not patching a symptom) The job behavior is the intended one from #22494; the specs encode the old contract. Rather than loosening the assertions to "a stream-error was published somewhere", this pins the full intended terminal sequence — `stream-error` followed by `queue-updated` — so the convergence event itself is now regression-tested on all four failure paths (mid-stream provider error, setup rejection, persistence failure, missing workspace). ## Impact Unblocks `server-test` / `ci-server-status-check` for every open PR (including #22498, which is needed to fix the dev-cluster migration incident). Test-only change. https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22500?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Abdul Rahman
|
f00b6ae185 |
use Temporal for date filter evaluation and fix IS operand (#22408)
## Summary - Refactored `evaluateDateFilter` in the workflow filter action from native `Date` to the Temporal API, using the shared `parseToInstantOrThrow` and `isSamePlainDate` utilities from `twenty-shared` (resolving the long-standing `// TODO: refactor this with Temporal`). - Fixed a bug in the `IS` operand: it previously compared only `getDate()` (day-of-month 1–31), so e.g. `2023-01-15` incorrectly matched `2023-02-15`. It now compares the full calendar day in UTC. - Removed server-local-timezone leakage: `IS_TODAY` and day comparisons now run in UTC (consistent with the neighbouring relative-date filter util), instead of relying on `toDateString()`/`getDate()`. ## Behavior changes (intended) - `IS` now matches on the full UTC calendar day, not day-of-month. - `DATE_TIME` `IS` matches on the same UTC day (not exact-instant equality). - Date comparisons are UTC-based, so evaluations near midnight in a non-UTC server locale may differ from the old local-timezone behavior. - Parsing is stricter (ISO + known formats via `parseToInstantOrThrow`); malformed operands resolve to "no match" instead of being loosely guessed by `new Date()`. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22408?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
027a098099 |
fix(ai): fully roll back a question answer when the resume enqueue fails (#22492)
## Rationale `answerAgentChatQuestion` is a three-step transaction without the transaction: resolve the question (flip tool part to `answered`, clear `pendingQuestionMessageId`, claim the stream), then enqueue the resume job. If the **enqueue fails**, the catch restores only `activeStreamId`. What's left behind: tool part says `answered`, `pendingQuestionMessageId` is `null`, no job will ever run. The client's own error handler rolls its card back to *pending* — so the user sees an answerable question whose re-submission deterministically throws `QUESTION_NOT_PENDING`. The turn is stuck and state is divergent on three surfaces (DB part, DB thread, client). ## Why this is the root cause, not a symptom patch The failure path was rolling back one of three writes. This makes the rollback total and **exact**: `resolvePendingQuestion` now returns the part's precise previous `toolOutput` (no reconstruction guesswork — question tools can carry arbitrary output fields), and the failure path restores the part verbatim plus the thread's pending-question state, guarded on the observed streamId so a competing claim is never clobbered. After rollback, server and client agree again: the question is pending, answering retries cleanly. The audit's alternative — forward recovery (keep the answers, mark the turn interrupted, resume via Retry) — has nicer UX in isolation but contradicts the client's existing rollback-to-pending behavior; matching the established contract wins until the client changes. ## User impact A transient Redis/queue hiccup at answer time currently bricks the question turn permanently. With this, the user sees the question again and can just re-answer. ## Test plan - [ ] CI green - [ ] Manual: fail the enqueue (kill Redis briefly) at answer time → question card returns to pending, re-answer succeeds https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22492?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
087bee0036 |
fix(ai): notify all tabs when a pending question is answered (#22491)
## Rationale `resolvePendingQuestion` updates the question tool-part to `answered` and re-claims the thread — but publishes **nothing**. The answering tab converges via a local browser event; every other tab keeps rendering the question card as interactive until the resumed stream's first chunk happens to arrive. A second tab (or teammate view on shared context) can attempt to answer an already-answered question and hit a confusing `QUESTION_NOT_PENDING` error. ## Why this is the root cause, not a symptom patch Answering a question is a state transition every subscriber cares about — exactly like queue promotion, message persistence, and stream errors, all of which publish. This transition just never did. The fix publishes the existing refetch-trigger event (`queue-updated`, which every tab already handles by refetching messages + thread state) right after resolution — no new event type, no new client code path, consistent by construction with how every other transition converges tabs. A dedicated `question-answered` event carrying the answers would save one refetch round-trip; the audit's verdict was that's over-engineering for a rare interaction. Publishing *before* the resume-enqueue is deliberate: even if the enqueue fails, the question **is** answered server-side, and tabs should reflect server truth. ## User impact Second tabs stop offering an interactive question that will error when submitted; everyone sees the answered state within a refetch instead of whenever the stream resumes. ## Test plan - [ ] CI green - [ ] Manual: two tabs on one thread, answer the question in tab A → tab B's card flips to answered without interaction https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22491?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
d1a4e250cf |
fix(ai): converge every tab's transcript when a stream fails with partial output (#22494)
## Rationale When a turn fails after emitting output, the partial assistant message **is persisted** — but only the success path publishes the event (`message-persisted`) that makes other tabs refetch. On failure, tabs that weren't watching the live stream keep a stale transcript until manual reload. (Error *visibility* itself already works — `stream-error` reaches every subscribed tab — the gap is purely the persisted-transcript sync. The original gap-analysis framing of this as an error-visibility problem was wrong; this is the corrected scope.) ## Why this is the root cause, not a symptom patch Turn settlement should converge subscribers regardless of *how* the turn settled — success and failure both persist state that tabs need. The failure path now publishes the same refetch-trigger event the rest of the lifecycle uses, right after the terminal `stream-error`. No new event type, no client changes: the existing guarded replay (`firstLiveSeq === null`) already ensures tabs with a live view keep their in-place error rendering while background tabs pick up the persisted partial message and error state. ## User impact Open the same thread in two tabs, have the turn die mid-answer in one: the other tab currently shows the conversation frozen pre-turn until reload. Now both converge to the persisted partial output plus the failed-turn state within one refetch. ## Test plan - [ ] CI green - [ ] Manual: two tabs, kill the provider mid-stream in tab A → tab B shows the partial message + error without reload https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22494?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
a505ed3245 |
feat(ai): typed CONTEXT_WINDOW_EXCEEDED error that hides the pointless Retry (#22488)
## Rationale When message pruning can't fit the conversation into the model's context window, `chat-execution.service.ts` throws a **raw `Error`**. `mapErrorToStreamError` classifies it as generic `STREAM_EXECUTION_FAILED`, so the client renders a standard failure with a **Retry button that deterministically fails again** — the conversation doesn't get shorter by retrying. Users loop on Retry against a permanently-failing thread. ## Why this is the root cause, not a symptom patch The failure is *terminal for the thread by construction*, and the error channel already distinguishes terminal-vs-retryable via typed `AiExceptionCode`s — this failure just never got one. Adding `CONTEXT_WINDOW_EXCEEDED` (typed exception → `UserInputError` mapping instead of a 500 → both error surfaces render the start-a-new-thread message without `onRetry`) puts it on the same rails as `API_KEY_NOT_CONFIGURED` and the other special-cased codes. Both frontend error surfaces route through `AiChatErrorRenderer`, so one case covers the in-message and under-list renderings. The deeper endgame (auto-summarize/compact older turns so threads never brick) is a multi-week feature — and this typed error remains necessary even then, as its terminal fallback. ## User impact Instead of an opaque error and a Retry that never works, users hitting the context limit get told exactly what happened and what to do (start a new thread), and monitoring stops counting a user-condition as a server error. ## Test plan - [ ] CI green - [ ] Manual: fill a thread past the model limit → typed message, no Retry on either error surface https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22488?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
56a6f9419b |
chore(server): fix stream-agent-chat job spec formatting breaking main's lint (#22493)
The stacked merges of #22479 and #22480 left `stream-agent-chat.job.spec.ts` on main failing the `oxfmt --check` gate (2 stray blank lines), which currently fails `server-lint-typecheck` on **every** open PR's merge ref. Two-line whitespace fix, no behavior change. Merging this first unblocks the rest of the queue. https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22493?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
1feb41eb64 |
perf(sse): drop the per-workspace lock around atomic activeStreams set operations (#22476)
## Rationale Every event-stream create/destroy in a workspace serializes on a single cache lock (`workspace:<id>:activeStreams`) just to run `setAdd`/`setRemove`. On Redis those are native `SADD`/`SREM` — already atomic (`cache-storage.service.ts:79-109`) — so the lock adds zero correctness. What it does add: 100ms lock-retry polling under concurrency, and a hard 5s ceiling (50 retries × 100ms, `cache-lock.service.ts:36`) after which stream creation **fails** with `Failed to acquire lock`. **Production evidence (Sentry):** [TWENTY-SERVER-H3W](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-H3W) (125 events, 16 users) server-side and [TWENTY-FRONT-6MM](https://twenty-v7.sentry.io/issues/TWENTY-FRONT-6MM) (65 users) client-side — spiky, consistent with deploy-driven reconnect storms in large workspaces where every tab reconnects at once and queues on one lock. ## Why this is the root cause, not a symptom patch The failure isn't "the lock timeout is too short" (raising it would just trade errors for latency) — it's that the critical section doesn't exist. A set-membership add/remove of a single element has no read-modify-write window on Redis. The lock that **is** legitimate stays untouched: `@WithLock` on `addQuery`/`removeQuery`, which genuinely read-modify-write the JSON queries map. The non-Redis fallback of `setAdd` is read-modify-write, but that path only serves single-node dev/test setups where the worst case is a transiently miscounted metrics gauge (`twenty_event_streams_live_total`), not a correctness issue — the authoritative per-stream state lives in its own key. ## User impact During reconnect storms (deploys, network blips) in busy workspaces, tabs no longer randomly fail to establish their event stream — which previously meant no live updates for that tab and, through the strict client error path, a crash of the listener sync loop (fixed separately in #22475). Also removes up-to-5s of serialized queueing latency per workspace on every connect wave. ## Test plan - [x] Verified `setAdd`/`setRemove` are native `SADD`/`SREM` + `EXPIRE` on the Redis path - [x] No callers depend on the lock's ordering (checked `engine/subscriptions` call sites) - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22476?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
3b76ec528f |
fix(ai): route missing-workspace stream failures through the standard error path (#22480)
## Rationale When `StreamAgentChatJob` can't find the workspace, it publishes a transient `stream-error` event and **returns before the try/finally exists** (`stream-agent-chat.job.ts`). Consequences on main: - `activeStreamId` is never cleared → every subsequent send in that thread queues behind a dead claim, forever; - no `lastStreamError` is persisted → nothing renders after a reload, and Retry has nothing to retry; - nothing throws → **zero telemetry**. Sentry confirms: the "Workspace not found" issues that exist are all auth/Stripe paths — this path fails in complete silence. ## Why this is the root cause, not a symptom patch The job's catch/finally already implement the correct failure contract for *every other* error: persist a typed `lastStreamError`, publish the typed event, release the claim guarded on the observed streamId. The bug is that one code path bypasses that contract via an early return. The fix removes the bypass — the lookup moves inside the `try` and throws a typed `AiException(WORKSPACE_NOT_FOUND)` — rather than duplicating cleanup in the early-return branch (which would be the symptom patch, and would drift the next time the contract changes). The alternative "prevent the job from existing when the workspace is gone" isn't achievable: workspace deletion between enqueue and pickup is an inherent race, so the job must handle it regardless. ## User impact A workspace deleted/deactivated mid-flight currently bricks the thread silently (the user just sees sends vanish into a queue). With this, the failure is visible (typed error message), recoverable (standard failed-turn state), and observable (real exception in monitoring). ## Stack Based on #22479 (spec harness) — it extends the same spec file with the regression test. `WORKSPACE_NOT_FOUND` is a TypeScript enum member, not a GraphQL schema change: no client-sdk regeneration needed. ## Test plan - [x] Regression test: missing workspace → typed rejection, `lastStreamError` persisted, terminal event published, claim released - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22480?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
ca8ab32253 |
fix(ai): delete the Redis chunk list on credits-exhausted terminal events (#22477)
## Rationale The AI chat stream keeps every published chunk in a Redis list (`agent-chat-stream-chunks:<threadId>`, 1h TTL) so late subscribers can catch up. On `message-persisted` the list is deleted. On `credits-exhausted` — the *other* successful terminal event — it wasn't. Any reload/refetch within the TTL replayed the orphaned chunks, flipping the thread into a "streaming" state that no terminal event ever closes: an endless spinner until the user sends another message. **Production evidence (Sentry):** `Billing Credits Exhausted` fired for **290 users / 937 events in 90 days**, ongoing ([TWENTY-SERVER-G42](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-G42)) — every one of those users who reloads the chat within an hour hits this. ## Why this is the root cause, not a symptom patch The chunk list's lifecycle contract is "cleared when the turn settles". `credits-exhausted` resolves the job successfully **without** persisting a `lastStreamError`, so unlike `stream-error` there is no persisted terminator for catchup to replay after the chunks — the replay is unconditionally un-closeable. Deleting on both settle events restores the contract exactly where it's already enforced for `message-persisted`. `stream-error` deliberately keeps the list: the persisted error acts as the replay terminator, letting a reloading client still see the failed turn's partial output. ## User impact Users who hit their billing cap mid-answer (~100/month) no longer come back to a permanently spinning thread after a reload — they see the settled conversation and the billing state. ## Test plan - [x] Unit spec: chunk accumulation with 1-based seq, deletion on both terminal events (`it.each`), retention on `stream-error` - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22477?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
6ac8ebcd18 |
test(ai): pin StreamAgentChatJob stream lifecycle with a reusable spec harness (#22479)
## Rationale `StreamAgentChatJob` is the most failure-sensitive path in the AI chat stack — it coordinates the model stream, Redis event publishing, message persistence, the thread's stream claim, and the queued-message flush — and it had **zero unit coverage**. Both historical hangs lived here: - a throw before the model stream merges bypassed `onFinish` entirely and hung the job until the **10-minute BullMQ lock** expired (thread stuck the whole time); - a throw inside `onFinish` (persistence failure) left trailing chunks published with **no terminal event** — the client spinner ran forever. Production still shows this class is live: `Query read timeout` thrown from inside `handleStreamFinish` ([TWENTY-SERVER-GV7](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-GV7)). ## Why this shape, not something else Tests-only PR, zero production risk. The fake chat stream mirrors the one AI SDK contract the job's coordination depends on — verified against the installed `ai@6.0.97` dist: `toUIMessageStream` converts mid-stream errors into error parts and **always** fires `onFinish` when the stream ends (`handleUIMessageStreamFinish` invokes it from both `flush()` and `cancel()`). Pinning that contract in the fake means a future SDK upgrade that breaks it fails these tests instead of production. Six tests pin current behavior: chunk ordering with `message-persisted` last, opaque error-chunk suppression, mid-stream failure persisting `lastStreamError` + releasing the claim, the two hang regressions above, and cancel skipping the queued flush. Three sibling PRs extend this exact spec file (missing-workspace routing, halted queue, and — later — auto-retry), which is why the harness lands first. ## User impact None directly; it makes the two worst historical user-facing hangs (10-minute dead thread, infinite spinner) regression-proof before the stuck-state fix series touches this code. ## Test plan - [x] 6 unit tests, no production code changed - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22479?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
a75414a05e |
fix(sse): treat expected event-stream coordination errors as 403s and never crash the sync loop (#22475)
## Rationale `NOT_AUTHORIZED` and `EVENT_STREAM_ALREADY_EXISTS` on the event-stream mutations are **expected coordination outcomes** — the frontend explicitly recognizes both (`isGracefullyHandledEventStreamError`) and recovers by recreating its stream. But `EventStreamExceptionFilter` rethrows them as `InternalServerError`, which (a) Sentry captures on every single occurrence, and (b) counts as a 500 in operation metrics. **Production evidence (Sentry):** this turned a March client-regression into a 119,510-event / 2,607-user flood ([TWENTY-SERVER-FP3](https://twenty-v7.sentry.io/issues/TWENTY-SERVER-FP3), plus FP0 at ~29k) that buried real errors. The trigger was fixed back then, but the amplifier — error-level capture of an expected signal — is still in place, and a residual trickle still fires today. Second defect, client side: for any *non-graceful* server error (e.g. a lock-acquisition timeout), `SSEQuerySubscribeEffect.handleError` **threw** from inside a debounced callback — an unhandled rejection ([TWENTY-FRONT-62M](https://twenty-v7.sentry.io/issues/TWENTY-FRONT-62M), 233 users; [6MM](https://twenty-v7.sentry.io/issues/TWENTY-FRONT-6MM), 65 users) that left the tab's query listeners permanently out of sync with the server (no more live updates until reload). ## Why this is the root cause, not a symptom patch The protocol design already says these are recoverable client-coordination signals — the bug is purely that the server encodes them with 500 semantics and the client punishes unexpected errors by giving up instead of resetting. This PR aligns both ends with the existing design rather than adding new machinery: - Server: `ForbiddenError` (403) with the same `subCode` — the client's graceful check already accepts `code === 'FORBIDDEN'`, so this is compatible by construction; `FORBIDDEN` is already in `graphQLErrorCodesToFilter`, so monitoring capture stops with no new filtering logic. - Client: the non-graceful path now does exactly what the graceful path does (reset listeners + recreate stream) and *additionally* reports the unexpected error — visibility without a crash. ## User impact Tabs that hit any event-stream error now always self-heal back to live updates instead of silently going stale until reload (~300 users hit the crash path over 90d). On the ops side: expected coordination noise leaves error monitoring, and 403/500 metrics become truthful. ## Test plan - [x] Behavior preserved for graceful codes (same reset path, client check already includes FORBIDDEN) - [ ] CI green https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38 --- _Generated by [Claude Code](https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22475?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
neo773
|
6f64be5751 |
Gate and meter email group: enterprise license (self-host) + credits (cloud) (#22390)
Email group (marketing email) was gated only by the `IS_EMAIL_GROUP_ENABLED` feature flag with no server-side enforcement. This adds real gating, split by deployment: - **Self-hosted** (`IS_BILLING_ENABLED=false`): requires a valid Enterprise plan. - **Cloud** (billing enabled): metered by credits, mirroring the existing AI credit system. Priced on AWS SES cost ($0.10/1,000 outbound) × 3 margin = $0.30/1,000 (300 micro-credits/email). Pre-flight blocks sends when out of credits; each email is charged after SES accepts it, in the async send job — matching how AWS bills us (no refund on bounce). Enforcement is applied at every email group resolver, and denials surface as proper client errors through a dedicated GraphQL exception filter. |
||
|
Félix Malfait
|
a28887bba6 |
feat(server): workspace opt-out of root-domain directory listing (#22423)
## What Lets a workspace opt out of being surfaced in the multi-workspace root-domain (app.twenty.com) picker via **email-domain discovery**. Adds `isDirectoryListingEnabled` (default `true`) on the workspace. When `false`, the workspace is filtered out of the approved-access-domain branch of `findAvailableWorkspacesByEmail`, so a user whose email domain matches an approved access domain no longer sees the workspace in the sign-up picker. ## Scope of the opt-out (deliberately narrow) The filter is applied **only** to the approved-access-domain discovery source: - **Members** (`availableWorkspacesForSignIn`) — never filtered; they keep access. - **Explicit invitations** — never filtered; the intent is one-to-one. - **Approved-access-domain discovery** — the only "listing" source, gated by the flag. A hidden workspace stays fully reachable by members and invited users via the direct workspace subdomain; it just isn't advertised in the global picker. > Open question for review: do we also want a stronger mode that hides the workspace from the root-domain picker even for existing members (forcing them to use the subdomain directly)? That would additionally filter the member/invitation sources and is a larger behavior change — not included here. ## Changes **Backend** - `workspace.entity.ts` — new `isDirectoryListingEnabled` column (`@Field`, default `true`). - `user-workspace.service.ts` — filter the approved-access-domain branch on the flag. - `update-workspace-input.ts` — expose the field on `updateWorkspace`. - `workspace.service.ts` — `PermissionFlagType.SECURITY` (same as the other discovery/security toggles). - Fast instance command adding the column (default `true`, so no existing workspace is hidden). **Frontend** - Settings > Security: a **"List in workspace directory"** toggle (shown only in multi-workspace mode) that flips the flag via `updateWorkspace`, mirroring the existing `isInternalMessagesImportEnabled` toggle. - Threaded the field through the current-user fragment, `CurrentWorkspace` type, and mock data. - Regenerated the metadata + client-sdk GraphQL types (`generated-metadata`, `twenty-client-sdk/.../generated`) — generated against a server booted from this branch. ## Verification - `tsgo` typecheck: 0 errors. `oxlint`: 0/0. `oxfmt`: clean. - Codegen diff verified to contain **only** the new field (no unrelated drift). - Tests not run locally; CI covers unit/integration + the codegen/migration freshness checks. --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> |
||
|
Etienne
|
29e48e16ba |
[Breaking change] fix: make pageLayout type field required (#22450)
fixes https://github.com/twentyhq/twenty/issues/22251 **Summary** - Fixes #22251 — NavigationMenuItem with type PAGE_LAYOUT returns 404 "Off track" for custom standalone pages - Makes type a required field in PageLayoutManifest instead of relying on a fallback default to RECORD_PAGE - Adds PageLayoutType enum to twenty-shared and exports it from the SDK for app developers - Adds build-time validation in definePageLayout to reject manifests missing type - Updates the CLI add command to prompt users to select a page layout type interactively **Root cause** When definePageLayout was called without type, the manifest converter defaulted to RECORD_PAGE. The frontend route guard at /page/:id then rejected it (only STANDALONE_PAGE is allowed), producing a 404. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22450?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
neo773
|
7cf8b58f1b |
feat(emailing): local unsubscribe URL + full-content log driver (#22412)
In LOG mode, emit a working http://unsubscribe.<subdomain>.localhost/emailing/unsubscribe link (from SERVER_URL + workspace subdomain) instead of empty content, and log the full text/html body + List-Unsubscribe header so the flow is inspectable locally. buildUnsubscribeUrls now takes a full base URL (field renamed httpsUrl -> webUrl). <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22412?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
4aaf171d63 |
feat(ai): add ask_questions interactive clarifying-question tool (#22346)
## What & why Adds an `ask_questions` tool that lets the in-app **Ask AI** assistant **pause a turn to ask the user one or more multiple-choice questions** (per the [Figma design](https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=105959-117153)) and resume once answered — instead of guessing on ambiguous/consequential decisions. The tool is **harness-only**: an interactive question UI is meaningless without a user to answer it, so it must be absent from MCP and from head-less workflow agents. ## Design — true tool-result resume (not a synthetic user message) The user's answer is a **structured tool result bound to the `toolCallId`**, and the **same agent turn resumes** — exactly how Anthropic (`tool_result` by `tool_use_id`) and OpenAI (`function_call_output`) model human-in-the-loop. The naive form of this (leave the tool call in `input-available` to mean "pending") is **impossible** here: `finalizeDanglingToolParts` rewrites `input-available` → `output-error` ("Tool execution was interrupted") on both the persist path (`addMessage`) and the model-reload path (`chat-execution.service.ts`). That util is a load-bearing safety net, so weakening it is the wrong move. Instead: - `ask_questions` is an **inline, chat-only tool with an `execute` that returns a `status: 'pending'` result immediately**, so the tool part is always `output-available` and **immune to `finalizeDanglingToolParts`**. `stopWhen(hasToolCall('ask_questions'))` halts the turn right after the call (the model never sees the placeholder). - A nullable **`thread.pendingQuestionMessageId`** marker records that a turn is awaiting an answer. - The new **`answerAgentChatQuestion`** mutation atomically *claims* the question (clears the marker, marks the thread streaming), **writes the answer onto the same tool part** (`status: 'answered'`), and **re-enqueues the turn via the existing `existingTurnId` plumbing** (`isResume` bypasses the per-turn dedup guard). On resume `finalizeDanglingToolParts` leaves the `output-available` part untouched and `convertToModelMessages` emits `assistant(tool_use)` + `tool_result(answers)`, so the model continues. This achieves the platform-aligned semantics **without** weakening the finalize safety net or inventing a fragile new part state. ### Meets the two requirements - **Survives refresh, scoped per-thread** — the pending state is a normal persisted `output-available` part + the thread marker; the frontend card is derived per-thread from the loaded messages, so it re-appears on reload and only on its own thread. - **Takes priority over the queue** — a unified `isBlocked = activeStreamId || pendingQuestionMessageId` gate is applied in both `sendChatMessage` (new messages queue) and `flushNextQueuedMessage` (the drain). The queue cannot unpile until the question is answered and the resumed turn completes. ### Harness-only by construction `ask_questions` is added **only** to the chat's inline `activeTools` (like `learn_tools`/`execute_tool`/`load_skills`). It never enters the tool registry/catalog, so it is invisible to MCP and to workflow agents — no `MCP_EXCLUDED_TOOL_NAMES` entry needed. ## UX While a question is pending, the **composer is replaced by the question card** (matching the Figma): question title + pager (`1/2`), numbered option rows (`IconSquareNumber*`) with per-option info-icon descriptions and a "Recommended" badge, and the normal composer as the free-text fallback ("Type anything to do differently."). The transcript shows a compact "Asking questions…" status line that becomes an answered summary. ## Changes **twenty-shared** - `ai/types/AskQuestionsToolTypes.ts` — `AskQuestionItem/Option/Answer/Result`, `ASK_QUESTIONS_TOOL_NAME`. **twenty-server** - `ai-chat/tools/ask-questions.tool.ts` — inline tool factory (pending-result `execute`, zod schema, 1–4 questions × 2–4 options). - `chat-execution.service.ts` — add to `activeTools` + `preloadedToolNames`; `hasToolCall` in `stopWhen`. - `chat-system-prompts.const.ts` — when-to-use guidance. - `entities/agent-chat-thread.entity.ts` — `pendingQuestionMessageId` column. - `stream-agent-chat.job.ts` — set the marker on a question pause; bypass the dedup guard on resume; suppress the no-text warning for question pauses. - `agent-chat-streaming.service.ts` — gate `flushNextQueuedMessage`; `enqueueResumeStream`. - `agent-chat.resolver.ts` — gate `sendChatMessage`; `answerAgentChatQuestion` mutation. - `agent-chat.service.ts` — `resolvePendingQuestion` (atomic claim + write answer). - `dtos/agent-chat-question-answer.input.ts`, `ai.exception.ts` (`QUESTION_NOT_PENDING`), `utils/find-pending-question-part.util.ts`. **twenty-front** - `components/AiChatQuestionCard.tsx` — the interactive card (matches Figma tokens) + `__stories__/AiChatQuestionCard.stories.tsx`. - `components/AiChatEditorSection.tsx` — swap the composer for the card while pending. - `components/AiChatQuestionStatusRenderer.tsx` + branch in `AiChatAssistantMessageRenderer.tsx`. - `states/selectors/agentChatPendingQuestionComponentSelector.ts`, `types/AgentChatPendingQuestion.ts`. - `hooks/useSubmitQuestionAnswer.ts` + `utils/markQuestionAnswered.ts` (optimistic) + `graphql/mutations/answerAgentChatQuestion.ts`. A design doc lives at `packages/twenty-server/docs/ASK_USER_QUESTION_TOOL_PLAN.md`. ## Migration Adds a nullable `pendingQuestionMessageId` (uuid) column to `core.agentChatThread`. Needs a generated **fast instance command** (`database:migrate:generate --name addThreadPendingQuestion --type fast`) — see "Verification status". ## Tests - Server: `ask-questions.tool.spec.ts` (pending echo + schema bounds), `find-pending-question-part.util.spec.ts`. - Front: `markQuestionAnswered.test.ts`, plus the Storybook story. ## Verification status (please read) This branch was authored in an environment where the monorepo `yarn install` repeatedly failed on transient TLS resets from the package registry, so I could **not** locally run the mechanical gates. The logic was reviewed by hand and the `ai@6.0.97` exports used (`hasToolCall`, `stepCountIs`, `generateId`) were confirmed against the package's type defs. Still **TODO** (will rely on CI / a follow-up once deps install): - [ ] `nx run twenty-shared:generateBarrels` (the `ai/index.ts` export was added by hand; regen to reconcile) - [ ] `nx run twenty-front:graphql:generate` (new mutation + input type) - [ ] generate the fast instance command (migration) for the new column - [ ] `typecheck` + `lint:diff-with-main` (front + server) — expect minor import-ordering autofixes - [ ] run the unit tests **Screenshots:** reproducing the live flow needs an AI provider API key (to get the model to actually call `ask_questions`), which isn't available here. The card can be screenshotted from its **Storybook story** (`AiChatQuestionCard.stories.tsx`) with no API key — I'll add that image once deps install, or a reviewer can run `nx storybook twenty-front`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01AArS8H3y3Z1Qwm763xhPLB --- _Generated by [Claude Code](https://claude.ai/code/session_01AArS8H3y3Z1Qwm763xhPLB)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22346?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
neo773
|
198f1e4916 |
feat(emailing): forward SES events to Tatami Monitor (#22407)
Add AwsSesObservabilityService, which adds an SNS event destination to each workspace's SES configuration set (gated on TATAMI_SNS_TOPIC_ARN) so deliverability events reach Tatami. Tag sends with tenant_id for per-workspace breakdowns. Requires to be merged https://github.com/twentyhq/twenty-infra/pull/765 <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22407?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
d709467902 |
feat(ai): surface AI chat stream failures through one typed error channel (#22434)
## Context Investigating a report where the AI chat showed only a `...` spinner while the network response clearly contained `No AI models are available`. Root cause: terminal stream failures reach the client on **two mismatched channels**. | Representation | Persisted (survives reload) | Rendered by client | |---|---|---| | AI-SDK `error` chunk (inside `stream-chunk`) | ✅ RPUSH'd to Redis | ❌ dropped by `readUIMessageStream` (no message part, no error state) | | typed `stream-error` event | ❌ never persisted | ✅ sets the error atom | Live, the `stream-error` event renders. But on reload, `chatStreamCatchupChunks` replays only the persisted **error chunk** — which the reducer discards — and the streaming indicator never clears. ## Change Collapse to a single typed error contract: - **Suppress the opaque `error` chunk** in the stream job; every failure is surfaced through the typed `stream-error` event. Errors are mapped via `mapErrorToStreamError` so an `AiException` keeps its `AiExceptionCode` (e.g. `API_KEY_NOT_CONFIGURED` → the existing "AI not configured" banner) instead of leaking a raw string. - **Persist the terminal error** next to the accumulated chunks and expose it as an explicit `error { code message }` field on `ChatStreamCatchupChunks`, so a client catching up after a reload recovers it — no dependency on the AI SDK's internal chunk shape. - **Reset per-thread stream state at job start**, so a failed turn's leftover chunks/error never replay on the next stream. - **Client replays the catchup error** as a terminal `stream-error` event, which clears the streaming indicator and renders the error (fixes the infinite spinner on a stream that ended in error). ## Notes - `ChatStreamError` is a new metadata GraphQL type; generated types (twenty-front metadata + client-sdk) were hand-updated to keep the tree consistent and will be reconciled by CI's `graphql:generate` check if anything differs. - Server unit test added for the error mapping. No schema/DB migration. ## Test plan - [ ] With no AI provider configured, send a chat message → error renders immediately (not a spinner). - [ ] Reload the thread → the error still renders (recovered from catchup), indicator not spinning. - [ ] Configure a provider and send again → normal streaming; no stale error from the previous failed turn. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22434?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Paul Rastoin
|
47689e676b |
feat(server): improve traceability of flat-entity map mutation errors (#22396)
## Context
cc @rashad
Twenty applies metadata changes optimistically to in-memory *flat entity
maps* before persisting them. The utils that mutate these maps throw
`FlatEntityMapsException` on invariant violations, which surface in
Sentry (e.g. during `InstallApplication`) as a **hardcoded, generic
message with no identifying data**:
```
GraphQLError: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists
```
There was no way to know *which* entity collided — making triage
impossible.
## What this does (two layers)
**Layer 1 — leaf utils emit identifiers**
- `FlatEntityMapsException` gains an optional structured `context`
(`universalIdentifier` / `id` / `applicationId` / `metadataName` /
`relatedMetadataName` / `operation`), read by the Sentry driver's
existing `'context' in exception` → `setExtra` channel.
- All **9 leaf throw sites** append their in-scope identifiers to the
message **and** populate `context`.
**Propagation — context survives the re-wraps**
- On the install path the collision throws in the (unwrapped)
`compute()` step, so the raw exception + context reaches app-sync
intact.
- For the run/build-phase paths, the migration runner and
build-orchestrator re-wraps copy only `.message`; they now also
**forward `context`** so structured data survives there too.
**Layer 2 — human installation error**
- `synchronizeFromManifest` catches flat-entity failures, resolves the
offending `universalIdentifier` to a manifest **object/field label**,
and rethrows `ApplicationException(APPLICATION_INSTALLATION_FAILED)`
with a safe, human `userFriendlyMessage`.
- The leaf `userFriendlyMessage` stays `STANDARD_ERROR_MESSAGE` — the
detailed message never leaks to end users.
- `APPLICATION_INSTALLATION_FAILED` surfaces with the dedicated
`ErrorCode.APPLICATION_INSTALLATION_FAILED` GraphQL code (mirroring the
workspace-migration runner formatter), not `INTERNAL_SERVER_ERROR`.
### Result — client-facing GraphQL error envelope
```json
{
"extensions": {
"code": "APPLICATION_INSTALLATION_FAILED",
"subCode": "APPLICATION_INSTALLATION_FAILED",
"userFriendlyMessage": "We couldn't install \"Test Application\". Its Invoice could not be applied to your workspace."
},
"message": "Installing application 'Test Application' failed [object: Invoice]: addUniversalFlatEntityToUniversalFlatEntityMapsThroughMutationOrThrow: flat entity to add already exists (universalIdentifier: ...)",
"name": "GraphQLError"
}
```
## Where the identifier shows up (not just Sentry)
The offending `universalIdentifier` reaches every consumer, not only
Sentry:
- **Sentry (server):** structured `context` extras + the enriched
message (fingerprinted by `code`, so no issue fragmentation).
- **GraphQL response `message`:** un-masked (no `useMaskedErrors`; the
error-handler hook passes `BaseGraphQLError` through as-is), so it
travels over the wire.
- **App-author SDK/CLI terminal:** `twenty-sdk` captures
`errors[0].message`; for this error `formatManifestValidationErrors`
returns `null` (no `extensions.errors`/`summary`), so the orchestrator
falls back to printing the full message, e.g.:
```
✗ Sync failed with error: Installing application 'X' failed [object:
Invoice]: … already exists (universalIdentifier: b1b2c3d4-…)
ℹ Hint: a metadata conflict was detected. Preview the plan with `yarn
twenty dev --once --dry-run`; …
```
The `already exists` / `universalidentifier` substrings also trigger
`getSyncErrorRecoveryHint`, so the author gets an actionable next step.
- **End-user (CRM UI):** only the safe rendered `userFriendlyMessage`
(no UUIDs).
## Design note
`userFriendlyMessage` behaviour of the leaf exceptions is intentionally
unchanged (guardrail). Layer 2 resolves labels for **objects and
fields** (the bulk of metadata); other manifest entity kinds fall back
to an app-name-only human message to avoid brittle manifest-walking —
easy to extend. A future first-class option would be structured
`extensions` (like `METADATA_VALIDATION_FAILED`) + a dedicated SDK
formatter; deferred since the message path already surfaces the detail
in the terminal.
## Tests
- **Unit:** existing through-mutation + runner-exception specs still
pass (they assert on exception **code**, not message). Added a spec for
the enrichment util.
- **Response-format snapshot (verified, green):**
`application-exception-filter.spec.ts` runs the exception filter and
snapshots the exact client-facing GraphQL error envelope shown above.
- **Integration:**
`failing-sync-application-flat-entity-map-conflict.integration-spec.ts`
syncs a manifest whose two objects share a `universalIdentifier`
(collision during manifest map build, before validation) and snapshots
the GraphQL error response via
`expectOneNotInternalServerErrorSnapshot`.
- ⚠️ The integration `.snap` was authored from the identical
deterministic path (verified by the filter unit snapshot) because the
integration suite couldn't be executed in the authoring sandbox. Please
regenerate/confirm with `nx test:integration:with-db-reset` (or `-u`) in
a seeded env.
## Status
Draft — opening for review.
|
||
|
Félix Malfait
|
1cba0cdf49 |
fix(server): apply row-level security predicates to API key and application principals (#22456)
## What Row-level security predicates were only resolved for **user** principals. For API key and application principals, object-level and field-level permissions were resolved (via `resolveRolePermissionConfig`), but the row-level predicate role was left `undefined`, so: - on the read path, `buildRowLevelPermissionRecordFilter` returned `null` and no `WHERE` clause was added; and - on the write path, `validateRLSPredicatesForRecords` returned early and skipped post-write validation. The result was that a role carrying row-level predicates constrained users as intended, but the same role applied to an API key or installed application was subject only to its object/field permissions — not its row filters. ## Changes - Add `resolveRoleIdFromAuthContext`, a single helper that resolves the effective role id for user, API key, and application principals. - Use it in `applyRowLevelPermissionPredicates` (read) and `validateRLSPredicatesForRecords` (write) so row-level predicates are enforced for all principal types. - Thread `apiKeyRoleMap` through `WorkspaceInternalContext` (it was already available on the ORM workspace context). - Refactor `resolveRolePermissionConfig` to reuse the same helper, so object-, field-, and row-level checks all resolve the role identically. `workspaceMember`-relative predicate values are still only bound for user contexts (API keys/applications have no workspace member), matching existing behaviour. ## Notes - Enterprise-gated RLS code paths only. - Could not run `nx typecheck`/lint in this environment (dependencies not installed); changes reviewed manually. CI will validate. https://claude.ai/code/session_01N2RkG8aMwgfFU2jBghMgCQ --- _Generated by [Claude Code](https://claude.ai/code/session_01N2RkG8aMwgfFU2jBghMgCQ)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22456?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
||
|
Félix Malfait
|
38fbff465f |
chore(server): ship the 2.20 standardOverrides drop as a dormant command (#22448)
Follow-up to #22417, per [this thread](https://github.com/twentyhq/twenty/pull/22417#discussion_r3512187719): migrate the `2-20/README.md` placeholder into a real command using the `TWENTY_NEXT_VERSIONS` mechanism. ### What - Add `DropMetadataStandardOverridesColumnFastInstanceCommand`, registered against `2.20.0`. It boots (`2.20.0` is in `TWENTY_ALL_VERSIONS`) but stays **dormant** — the upgrade sequence only runs `TWENTY_CROSS_UPGRADE_SUPPORTED_VERSIONS` (previous + current), so it never executes during the 2.19 deploy and activates automatically when `nx version:bump` promotes 2.20 to current. - Name constant + unit test (SQL parity, registration against `2.20.0`, name-constant parity). - Register it in `instance-commands.constant.ts`. - Update the `standardOverrides` `@deprecated` comments on object/field metadata to point at the shipped command. - Delete `2-20/README.md`. - Document the "ship a command for a future version" flow in `docs/UPGRADE_COMMANDS.md` and `.cursor/rules/server-migrations.mdc` (the mechanism was previously undocumented). ### Note / correction to the README's plan The old README implied both the command **and** `@WasRemovedInUpgrade` could be added at 2.20 time. Only the command can ship now: the decorator's validator runs against the active sequence, so referencing a still-dormant 2.20 step fails boot with `unknown-step-name`. So the entity keeps its `WasRemovedInUpgrade<T>` type wrapper for now; the decorator gets wired (one line, via the name constant) once 2.20 is current — same deferred-drop shape as `isUIReadOnly`. ### Verification Could not run `jest`/`typecheck`/`lint` in this environment: `yarn install` is blocked by egress policy on a git-based transitive dep (`github.com/electron/node-gyp.git`). Verified by review against the sibling 2-19 add-column and 2-12 drop commands. **Please let CI run before merge.** https://claude.ai/code/session_01KMArJvdEmsX3eAmJLbS1b6 --- _Generated by [Claude Code](https://claude.ai/code/session_01KMArJvdEmsX3eAmJLbS1b6)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22448?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> |
