Libra CodeHub

CedarCopilot/cedar-mail

Branch: staging

fix(config,sql): make the definition.id / fieldKey ambiguity actually unrepresentable

merged#2904CedarCopilot

CedarCopilot wants to merge 3 commits into staging from worktree-send-it-field-id-ambiguity

Potential production issues

custom field identifiers are disambiguated in the schema

A behavior previously repaired by PR #2897 passes on the current base and fails on this PR revision.

When the application executes the behavior implemented in apps/server/src/mastra/tools/config/config-tool-shared.ts.

Suggested fix: Restore the behavior preserved by PR #2897, then rerun this check against both revisions.

Technical evidence

A regression fixed by PR #2897 worked before this change and returned after it.

Observed difference: the list-fields id description warns that id is not a query key and directs callers to fieldKey

Related changed code: apps/server/src/mastra/tools/config/config-tool-shared.ts.

Verified behaviors

  • Field definitions expose rename tracking separately from the query key

    Libra ran this behavior against the change and confirmed the check detects when it breaks.

    Technical evidence
    • a custom field definition exposes renameTrackingId instead of id while its sibling fieldKey remains the query reference

    Changed code: config-tool-shared.ts.

  • Field identifier mismatches include organization and member definitions only

    Libra ran this behavior against the change and confirmed the check detects when it breaks.

    Technical evidence
    • the mismatch map includes distinct mismatches from the organization AOP and a user AOP belonging to that organization
    • the mismatch map excludes definitions belonging to a user in another organization

    Changed code: execute.ts.

Live on prod, 1 of 6 surfaces working, 2 days leftTimeline and evidence
  1. Opened
    Sep 23, 2026, 2:41 AM
  3. Merged
    Sep 23, 2026, 3:17 AM
  4. Live on prod
    Sep 23, 2026, 3:17 AM
  5. Observed 7 hours, 6 surfaces, 423 requests
    Sep 23, 2026, 3:17 AM
  6. Watching

    Live on prod, 1 of 6 surfaces working, 2 days left

    Sep 23, 2026, 3:17 AM
  7. Pipelines steady after this deploy
    Sep 23, 2026, 3:17 AM

Behaviors Libra is checking

Config field listings expose the internal rename-tracking identifier as `renameTrackingId` instead of `id`, leaving `fieldKey` as the only field reference key agents should use for queries and filters.Not checked
prod
Successful `create-field` and `update-field` config actions return field definitions with `renameTrackingId` instead of the ambiguous `id` property.Not checked
prod
Queries whose text does not mention `field_id` skip the organization custom-field mismatch lookup while continuing through normal guarded execution.Not checked
prod
The guarded SQL path loads every organization- and user-level custom-field definition for the tenant and maps any differing rename-tracking id to its actual `fieldKey`, without including same-value taxonomy fields or other tenants.Not checked
prod
Guarded queries against `crm_conversation_field_values` are rejected when an equality or `IN` filter uses a known custom-field rename-tracking id instead of the stored `fieldKey`, and the error identifies the correct key.Not checked
prod
The field-id ambiguity check resolves qualified and unqualified `field_id` references within their own SELECT or CTE scope so unrelated columns in sibling CTEs or subqueries are not falsely rejected.Not checked
prod

Libra measured 1 of 6 production surfaces on prod. 1 surface had under 20 requests, so Libra has not judged it; 3 surfaces had no requests at all. Libra checks hourly for 3 days after each deploy.

Summary

  • Follow-up to #2897 (merged). That PR added a .describe() warning to list-fields's output schema and skill-doc gotchas , but it didn't work: an agent hit the exact same field_id = 'cf_close_date' mistake again in a live session, minutes after the fix shipped. Root cause: Mastra's tool-output validation strips a tool's return value down to whatever keys the output zod schema declares, but the schema still declared a key literally named id , so the misleading value (and its name) reached the model unchanged. .describe() text is schema-validation metadata; it is never serialized into the actual JSON a tool call returns.
  • This PR does the real fix, in two independent layers:
    1. Renames the exposed key from id to renameTrackingId everywhere list-fields (and create-field/update-field, which share the same output schema , a write-side regression this same effort found and fixed) build a field-definition response, so nothing about the live JSON can be mistaken for a query key anymore.
    2. A run-sql query-time guard (checkFieldIdAmbiguity in sql-guard.ts) that rejects a query filtering crm_conversation_field_values.field_id against a literal matching some field's definition.id instead of its real fieldKey, with an error naming the correct value , defense in depth in case a wrong value ever reaches a query anyway.
  • Also fixes two stale doc cross-references left behind by #2897's rename, and adds real DB-backed integration test coverage for the new guard's loader (field-id-mismatches.ts) and its wiring into execute.ts, using this repo's existing DATABASE_URL-gated describeIfDb pattern.
  • Notable near-miss caught in review: an earlier version of the integration-test coverage added a new devDependency (@electric-sql/pglite-socket) to get a real Postgres-wire-protocol connection for testing. That single dependency addition caused pnpm to re-resolve peer-dependency instantiations workspace-wide , better-auth's resolved zod peer flipped from 3.25.76 to 4.6.5 , which broke an unrelated apps/mail test and caused 16 failed test files / 59 failed tests across the full apps/server suite, entirely unrelated to this change. Confirmed via bisection (reverting just the lockfile restored everything), then backed the dependency out entirely and rewrote the tests on the existing DB-gated pattern instead, with zero new dependencies and zero scenarios dropped.

Test plan

  • apps/server/src/mastra/tools/config/__tests__/config-read-output-schema.test.ts, configTool.test.ts, apps/server/src/services/aop/__tests__/list-fields.test.ts , updated for the rename, including the write-side regression fix; 204+ tests pass.
  • apps/server/src/mastra/tools/run-sql-query/__tests__/sql-guard.test.ts , new checkFieldIdAmbiguity suite (wrong value rejected with correct fieldKey named, correct fieldKey passes, unknown/typo value not flagged, background/taxonomy field passes, IN-list handling, table-scoping); 93 tests pass.
  • apps/server/src/services/crm-sql/__tests__/field-id-mismatches.test.ts, execute.test.ts , new, DATABASE_URL-gated integration tests (org-level and user-level-AOP mismatches, cross-org isolation, background-field exclusion, real end-to-end guard rejection through runGuardedSql, and a spy-verified proof the perf early-exit actually skips the DB round-trip for non-field_id queries). Skip cleanly with no DATABASE_URL configured, matching this repo's existing convention for this class of test.
  • pnpm --filter @zero/server exec vitest run src/ (full suite) , 1136 test files, 14089+ tests pass, 0 failures (re-verified after backing out the dependency incident).
  • pnpm --filter @zero/mail exec jest --config jest.config.cjs --no-coverage --ci under TZ=UTC (matching CI's env for that step) , full suite green, including LinkedColumns.test.tsx (the test the dependency incident broke).
  • pnpm --filter @zero/server run types (tsc -b) , clean.
  • `pn
Show production surfaces and changed-file mapping

Production surfaces

SurfaceRequestsErrorsp95UsersVerdict
handle_execute_sync_thread55 → 1420 → 0 (0%)213430 ms → 17456 ms65Working
142 requests since the deploy with 0 errors (0 errors in the 55-request baseline before it). p95 17456 ms, was 213430 ms.
gmail.users.threads.get332 → 26424 → 11 (4%)446 ms → 552 ms79Pre-existing
11 of 264 requests failed (4.2%) since the deploy, matching the 7.2% baseline error rate before it. Not attributable to this change.
handle_execute_meeting7 → 170 → 0 (0%)2222 ms → 2367 ms10Insufficient traffic
17 requests, under the 20 Libra needs
handle_execute_scheduled_execution8 → 00 → 0 (0%)118947 ms → not measured0No traffic
No requests recorded since this deploy.
handle_execute_external_crm27 → 00 → 0 (0%)11162 ms → not measured0No traffic
No requests recorded since this deploy.
execute_tool update-conversation-fields4 → 00 → 0 (0%)1064 ms → not measured0No traffic
No requests recorded since this deploy.

Changed files → surfaces

  • apps/server/src/services/crm-sql/execute.tsgmail.users.threads.gethandle_execute_sync_threadhandle_execute_meetinghandle_execute_scheduled_executionhandle_execute_external_crmexecute_tool update-conversation-fields
  • apps/server/.claude/skills/field-query/SKILL.mdno production surface mapped
  • apps/server/src/db/aop-schema.tsno production surface mapped
  • apps/server/src/mastra/tools/config/__tests__/config-read-output-schema.test.tsno production surface mapped
  • apps/server/src/mastra/tools/config/__tests__/configTool.test.tsno production surface mapped
  • apps/server/src/mastra/tools/config/config-tool-shared.tsno production surface mapped
  • apps/server/src/mastra/tools/config/config-write-tool.tsno production surface mapped
  • apps/server/src/mastra/tools/run-sql-query/__tests__/sql-guard.test.tsno production surface mapped
  • apps/server/src/mastra/tools/run-sql-query/sql-guard.tsno production surface mapped
  • apps/server/src/services/aop/__tests__/list-fields.test.tsno production surface mapped
  • apps/server/src/services/aop/list-fields.tsno production surface mapped
  • apps/server/src/services/crm-sql/__tests__/execute.test.tsno production surface mapped
  • apps/server/src/services/crm-sql/__tests__/field-id-mismatches.test.tsno production surface mapped
  • apps/server/src/services/crm-sql/field-id-mismatches.tsno production surface mapped
  • apps/server/src/services/crm-sql/sql-guard.tsno production surface mapped