feat(): add external storage for field + recently view

[AMoreaux]

Note

Introduce Redis-backed external field storage and per-user lastViewedAt tracking, integrate into query/filter/order pipelines, add migration and metadata support, trigger updates on record view, and switch queue Redis client usage.

• External Field Storage (Redis):
  • Add storage to fieldMetadata (migration) and propagate through metadata, mocks, builders, and sync utilities; skip non-Postgres fields in schema generation, migrations, selects, and formatting.
  • Implement Redis infrastructure: RedisFieldsDataSource, RedisFieldRepository, RedisStorageDriver, ExternalFieldDrivers provider/token; wire into TwentyORMModule and WorkspaceDatasourceFactory.
  • Extend WorkspaceSelectQueryBuilder to join external fields via CTEs, select aliased values (external_...), and support ordering; update GraphQL parsers to filter/order using external selectors and iterative addOrderBy.
  • Update helpers: buildColumnsToSelect, computeWhereConditionParts, formatData to respect non-Postgres storage.
• Per-user lastViewedAt:
  • Add standard lastViewedAt field to base/custom objects (virtual, Redis-backed) and expose in entities.
  • On findOne, write view timestamp to Redis; support sorting/selection of this field.
  • Frontend: RecordShowEffect triggers optimistic lastViewedAt set on view; useUpdateOneRecord allows optional input and optimistic-only updates.
• Analytics:
  • Add Object Record Viewed event and make audit service insert methods async.
• Admin/Worker Redis client:
  • Replace getQueueClient with getNoevictionClient across health/queue services and tests.
• Tests:
  • Add GraphQL query parser tests and adjust existing specs for new fields/clients.

^{Written by Cursor Bugbot for commit 2ef7c9d. This will update automatically on new commits. Configure here.}

[github-actions]

TODOs/FIXMEs:

• // todo: move this logic to the driver scoped function: packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts

Generated by 🚫 dangerJS against bfb1857

[sentry]

Bug: The buildColumnsToSelect function incorrectly removes the id field from the selection object, breaking downstream operations expecting id.
_{Severity: CRITICAL | Confidence: 1.00}

🔍 Detailed Analysis

The buildColumnsToSelect function explicitly removes the id field from the fieldsToSelect object using destructuring const { id: _, ...fieldsToSelectWithoutId } = fieldsToSelect;. This causes downstream functions, such as buildColumnsToReturn and various query runners, to operate without the primary key id, leading to incorrect database select statements and potential data integrity issues. This change directly contradicts existing test expectations where id: true is anticipated in the result.

💡 Suggested Fix

Modify buildColumnsToSelect to retain the id field in the returned object. Remove the destructuring that excludes id to ensure it's always included in the selection.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/build-columns-to-select.ts#L64-L66

Potential issue: The `buildColumnsToSelect` function explicitly removes the `id` field
from the `fieldsToSelect` object using destructuring `const { id: _,
...fieldsToSelectWithoutId } = fieldsToSelect;`. This causes downstream functions, such
as `buildColumnsToReturn` and various query runners, to operate without the primary key
`id`, leading to incorrect database select statements and potential data integrity
issues. This change directly contradicts existing test expectations where `id: true` is
anticipated in the result.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds external storage infrastructure for field metadata, specifically implementing Redis storage for the lastViewedAt field to track when users last viewed records.

Key Changes:

• Added storage column to fieldMetadata table (postgres/redis)
• Implemented Redis storage driver infrastructure with ZSet-based data collection
• Added lastViewedAt field to custom objects using Redis storage
• Modified ORM query builder to join external storage data via CTEs
• Updated frontend to track record views and update lastViewedAt

Critical Issues Found:

• SQL Injection Vulnerability: UUID values from Redis are directly interpolated into SQL strings without sanitization in RedisFieldSqlFactory.buildZSetDateTimeParts() (line 20-21)
• Incorrect Join Logic: leftJoin() API usage in WorkspaceSelectQueryBuilder.appendExternalFields() appears incorrect - passing CTE name as property and alias/condition parameters may not work as intended (line 114)
• React Effect Issues: RecordShowEffect has unstable dependencies in useEffect that could cause infinite loops, and uses effects instead of event handlers per repository guidelines

Additional Concerns:

• Hardcoded 100-entry limit in Redis driver without explanation
• No tests for the critical SQL generation logic
• Missing UUID format validation before SQL interpolation
• Type casting workarounds in Redis data source service suggest TypeScript issues

Confidence Score: 1/5

• This PR has critical security vulnerabilities and logic errors that make it unsafe to merge
• Score of 1 reflects two critical blocking issues: (1) SQL injection vulnerability in Redis field SQL factory where unsanitized UUIDs from Redis are directly interpolated into SQL strings, and (2) incorrect TypeORM leftJoin API usage that will cause join conditions to be ignored, breaking the core functionality. Additionally, the frontend implementation violates repository guidelines with unstable useEffect dependencies that risk infinite loops.
• Pay immediate attention to redis-field-sql.factory.ts (SQL injection), workspace-select-query-builder.ts (broken join logic), and RecordShowEffect.tsx (infinite loop risk)

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-server/src/engine/twenty-orm/factories/redis-field-sql.factory.ts	1/5	New factory for Redis field SQL generation with SQL injection vulnerability in UUID interpolation
packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts	1/5	Added external storage support with incorrect leftJoin API usage that may break join conditions
packages/twenty-front/src/modules/object-record/record-show/components/RecordShowEffect.tsx	2/5	Added lastViewedAt tracking with missing useEffect dependencies causing potential infinite loop
packages/twenty-server/src/database/typeorm/core/migrations/common/1760458320000-add-fieldmetadata-storage-column.ts	5/5	Simple migration adding nullable storage column to fieldMetadata table
packages/twenty-server/src/engine/twenty-orm/storage/drivers/redis-storage.driver.ts	3/5	New Redis storage driver implementing data collection from Redis ZSets with hardcoded 100-entry limit

Sequence Diagram

sequenceDiagram
    participant User
    participant RecordShowEffect
    participant useUpdateOneRecord
    participant GraphQL API
    participant WorkspaceSelectQueryBuilder
    participant RedisStorageDriver
    participant RedisFieldsRepository
    participant Redis
    participant PostgreSQL

    User->>RecordShowEffect: Views record page
    RecordShowEffect->>useUpdateOneRecord: updateOneRecord({idToUpdate, optimisticRecord: {lastViewedAt}})
    useUpdateOneRecord->>GraphQL API: updateOneRecord mutation
    GraphQL API->>RedisFieldsRepository: setZSetEntry(key, recordId, timestamp)
    RedisFieldsRepository->>Redis: ZADD key timestamp recordId
    Redis-->>RedisFieldsRepository: OK
    RedisFieldsRepository-->>GraphQL API: Success
    GraphQL API-->>useUpdateOneRecord: Updated record
    useUpdateOneRecord-->>RecordShowEffect: Record updated

    Note over User,PostgreSQL: Querying records with lastViewedAt

    User->>GraphQL API: Query records
    GraphQL API->>WorkspaceSelectQueryBuilder: findMany with external storage
    WorkspaceSelectQueryBuilder->>WorkspaceSelectQueryBuilder: prepareExternalStorage()
    WorkspaceSelectQueryBuilder->>RedisStorageDriver: collectData(userId, workspaceId, objectMetadataId)
    RedisStorageDriver->>RedisFieldsRepository: getZSetEntriesRange(key)
    RedisFieldsRepository->>Redis: ZREVRANGE key 0 -1 WITHSCORES
    Redis-->>RedisFieldsRepository: [recordId1, score1, recordId2, score2...]
    RedisFieldsRepository-->>RedisStorageDriver: [{id, score}]
    RedisStorageDriver->>RedisStorageDriver: buildZSetDateTimeParts(entries.slice(0,100))
    RedisStorageDriver-->>WorkspaceSelectQueryBuilder: {cteSql, joinOn, selectExpr}
    WorkspaceSelectQueryBuilder->>WorkspaceSelectQueryBuilder: addCommonTableExpression(cteSql)
    WorkspaceSelectQueryBuilder->>WorkspaceSelectQueryBuilder: leftJoin(cteName, alias, joinOn)
    WorkspaceSelectQueryBuilder->>PostgreSQL: SELECT with CTE join
    PostgreSQL-->>WorkspaceSelectQueryBuilder: Query results
    WorkspaceSelectQueryBuilder-->>GraphQL API: Records with lastViewedAt
    GraphQL API-->>User: Display records

Loading

_{72 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: SQL injection vulnerability: id from Redis is directly interpolated into SQL without sanitization

The id value from Redis entries is inserted directly into the SQL string using template literals. If malicious data ends up in Redis (e.g., through data corruption or a separate vulnerability), it could execute arbitrary SQL.

Suggested change

	({ id, score }) =>
	`('${id}', to_timestamp(${Number(score) || 0} / 1000)::timestamptz)`,
	({ id, score }) =>
	`($${parameterId++}, to_timestamp(${Number(score) || 0} / 1000)::timestamptz)`,

Use parameterized queries instead of string concatenation, or at minimum validate/sanitize the UUID format.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-server/src/engine/twenty-orm/factories/redis-field-sql.factory.ts
Line: 20:21

Comment:
**logic:** SQL injection vulnerability: `id` from Redis is directly interpolated into SQL without sanitization

The `id` value from Redis entries is inserted directly into the SQL string using template literals. If malicious data ends up in Redis (e.g., through data corruption or a separate vulnerability), it could execute arbitrary SQL.

```suggestion
        ({ id, score }) =>
          `($${parameterId++}, to_timestamp(${Number(score) || 0} / 1000)::timestamptz)`,
```

Use parameterized queries instead of string concatenation, or at minimum validate/sanitize the UUID format.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: Incorrect leftJoin API usage causes join condition to be ignored

The TypeORM leftJoin signature is leftJoin(property, alias, condition?). Here you're passing:

• parts.cteName (the CTE name, not a property)
• `${field.name}Records` (intended as alias)
• parts.joinOn (join condition)

But leftJoin expects the second parameter to be the alias and the third to be the condition. Since parts.cteName equals `${field.name}Records`, the alias becomes the join condition string, and the actual join condition is treated as additional ON clause parameters.

This may cause incorrect SQL generation where the join condition "lastViewedAtRecords"."recordId"::uuid = "alias".id::uuid isn't properly applied.

Use leftJoinAndSelect with proper parameters or verify the generated SQL matches expectations in tests.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts
Line: 114:114

Comment:
**logic:** Incorrect `leftJoin` API usage causes join condition to be ignored

The TypeORM `leftJoin` signature is `leftJoin(property, alias, condition?)`. Here you're passing:
- `parts.cteName` (the CTE name, not a property)
- `` `${field.name}Records` `` (intended as alias)  
- `parts.joinOn` (join condition)

But `leftJoin` expects the **second parameter to be the alias** and the **third to be the condition**. Since `parts.cteName` equals `` `${field.name}Records` ``, the alias becomes the join condition string, and the actual join condition is treated as additional ON clause parameters.

This may cause incorrect SQL generation where the join condition `"lastViewedAtRecords"."recordId"::uuid = "alias".id::uuid` isn't properly applied.

Use `leftJoinAndSelect` with proper parameters or verify the generated SQL matches expectations in tests.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

style: French comment in codebase: "Important: inclure lastViewedAt si ce n'est pas déjà dans la signature"

Suggested change

	// Important: inclure lastViewedAt si ce n’est pas déjà dans la signature
	// Important: include lastViewedAt if not already in the signature

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/object-record/record-show/components/RecordShowEffect.tsx
Line: 53:53

Comment:
**style:** French comment in codebase: "Important: inclure lastViewedAt si ce n'est pas déjà dans la signature"

```suggestion
    // Important: include lastViewedAt if not already in the signature
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: Missing dependencies in useEffect causes stale closure and infinite update loop risk

The effect depends on updateOneRecord and setRecordStore but these aren't stable references (created from hooks). On every render:

1. New updateOneRecord function is created
2. Effect re-runs
3. Triggers update which may re-render
4. Cycle repeats

Additionally, per repository rules (.cursor/rules/react-general-guidelines.mdc), effects should be moved to dedicated EffectComponent or use event handlers instead of useEffect for state updates.

Either:

• Wrap updateOneRecord call in a separate effect component
• Use useCallback to stabilize the references
• Move to an event handler pattern instead of effect-based updates

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/object-record/record-show/components/RecordShowEffect.tsx
Line: 60:68

Comment:
**logic:** Missing dependencies in `useEffect` causes stale closure and infinite update loop risk

The effect depends on `updateOneRecord` and `setRecordStore` but these aren't stable references (created from hooks). On every render:
1. New `updateOneRecord` function is created
2. Effect re-runs
3. Triggers update which may re-render
4. Cycle repeats

Additionally, per repository rules (`.cursor/rules/react-general-guidelines.mdc`), effects should be moved to dedicated `EffectComponent` or use event handlers instead of `useEffect` for state updates.

Either:
- Wrap `updateOneRecord` call in a separate effect component
- Use `useCallback` to stabilize the references
- Move to an event handler pattern instead of effect-based updates

How can I resolve this? If you propose a fix, please make it concise.

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:62515

This environment will automatically shut down when the PR is closed or after 5 hours.

[cursor]

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on December 15

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[cursor]

Bug: Mismatched Variable, Incorrect Return

Typo in variable name: udpatedRecord should be updatedRecord. This causes the function to return the wrong variable, as udpatedRecord is assigned the mutation result but updatedRecord refers to the promise object from the mutation call, not the actual data.

 

[cursor]

Bug: Default alias generation unexpectedly fails.

assertIsDefinedOrThrow(alias) is called but alias is optional in the method signature. When createQueryBuilder is called without an alias parameter, this will throw an error, breaking existing code that relies on TypeORM's default behavior of auto-generating aliases.

 

[FelixMalfait]

I'm surprised to see storage at that level, shouldn't it be within settings?

[FelixMalfait]

Comment probably not needed

[FelixMalfait]

Remove comment? Also is this a standard pattern? Maybe it is!

[FelixMalfait]

This change seems a bit odd? Why would we be allowed to call updateOneRecord without any update param?

[FelixMalfait]

Would it be better to solve at the root instead? Not sure where the root issue comes from?

[FelixMalfait]

French :)

[FelixMalfait]

I thought this was in settings?