Skip to content

feat(people): add a synced organization chart - #9628

Merged
waleedlatif1 merged 4 commits into
stagingfrom
codex/people-directory-sync
Nov 3, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
codex/people-directory-sync

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Build the People org chart with the workflow canvas controls, directory search, shareable profiles, and keyboard navigation.
  • Keep employees and reporting lines current from SCIM, Okta, or Microsoft Entra ID. Directory profiles can appear without granting an account or workspace access.
  • Checkpoint remote pages and reporting lines across scheduler runs, honor provider backoff, and publish the validated directory in one transaction. Show admins progress, failures, retry timing, and the last successful refresh.
  • Keep People gated by its existing organization feature flag and its guide unpublished.

Type of Change

  • Feature

Testing

Verified 10,241 Okta users, 10,007 Entra users, and 10,004 SCIM users against real PostgreSQL, including a manager on the last page, mixed success and throttling inside a Graph batch, retry and lease recovery, and preservation of the previous chart. Tested chart navigation, import, and sync administration in Chromium at desktop and mobile sizes. The two HTTP suites passed 52 checks over 309 requests. All 45 focused database integration tests passed. A separate HTTP contention regression confirmed that SCIM provisioning and retirement complete while publication holds the sync lock. Browser fixtures covered two distinct 2,048-character department names on a 390px screen, single-page People search continuation, and credential drafts across transient failures and access revocation. Okta and Microsoft Graph responses were simulated.

Validated 100,000-person wide and deep trees in PostgreSQL. One chart branch on a freshly published 10,007-person fixture took 36 ms, down from 9.16 s.

The repository test suite passed 42,660 tests. Type checks, lint, all 62 audits, migration safety, and block-registry validation passed.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Ignored Ignored Preview Nov 3, 2026 8:08pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Adds an organization chart backed by SCIM, Okta, or Microsoft Entra ID.

  • People now shows a searchable directory and a browsable organization chart.
  • Admins can preview and import profile data from files and connected directories.
  • Connected directories refresh in the background and publish complete charts.

The three earlier, unnumbered findings are addressed: long departments use digest-based filters, chart labels use OverflowText, and the reporting path uses shared scroll-edge fades. No new actionable issues were found.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Admin or scheduled refresh] --> B[Read directory pages]
  B --> C[Save progress privately]
  C --> D[Read reporting lines]
  D --> E[Check complete directory]
  E --> F[Publish chart in one transaction]
  B --> G[Wait and retry]
  D --> G
  G --> C
  F --> H[Directory and org chart]
Loading

Reviews (4) · Last reviewed commit: "fix(people): preserve sync drafts and la..." · Reviewed by Greptile

Comment thread apps/sim/lib/people/provider-import.ts Resolved

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

7 issues found across 85 files

Confidence score: 3/5

  • provider-request.ts rejects Graph continuation links when the collection path differs only in casing, which can stop directory sync. Keep the origin check and compare the path case-insensitively.
  • provision-user.ts can wait on the peopleDirectorySync row while SCIM publish holds it FOR UPDATE; the 5-second lock timeout may cause provisioning to fail. Avoid making this marker update contend with the publish transaction.
  • people-sync.ts stores and sends an Entra clientSecret with surrounding whitespace, which can make token requests fail. Trim it before validation.
  • provider-import.ts counts disabled Entra accounts toward the 10,000-person cap even though they are excluded from imported rows, so a directory with 10,000 or fewer enabled users can be rejected. Count only profiles kept for import.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/emcn/src/components/combobox/combobox.tsx">

<violation number="1" location="packages/emcn/src/components/combobox/combobox.tsx:132">
P3: The `onSearchChange` JSDoc now contradicts the new `searchMode` behavior. In `'server'` mode, local filtering of `options` is skipped, so the doc should say that. Update the paragraph to state that client-side filtering applies only in `searchMode='local'`, and that server mode relies on `onSearchChange` to supply matches.</violation>
</file>

<file name="apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/resource-no-results.tsx">

<violation number="1" location="apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-empty-state/resource-no-results.tsx:16">
P3: The new `filterScope` prop is inserted between `searchScope`'s doc comment and `searchScope`, so the "Where a search looked" comment now describes `filterScope`. Move the prop below `searchScope` and give `filterScope` its own doc comment.</violation>
</file>

<file name="apps/sim/lib/api/contracts/people-sync.ts">

<violation number="1" location="apps/sim/lib/api/contracts/people-sync.ts:27">
P2: `clientSecret` is not trimmed, unlike the Okta `token` in this schema, so a secret pasted with surrounding whitespace is stored and sent to Entra unchanged and token requests fail. Add `.trim()` before `.min(1)` to match the Okta token.</violation>
</file>

<file name="apps/sim/lib/people/provider-import.ts">

<violation number="1" location="apps/sim/lib/people/provider-import.ts:199">
P2: This row cap counts disabled Entra accounts, which are filtered out of the imported rows. A directory with 10,000 or fewer enabled users can fail with the 10,000-person error. Count the profiles that were kept instead, e.g. `count += page.people.length`.</violation>
</file>

<file name="apps/sim/lib/people/sync/provider-request.ts">

<violation number="1" location="apps/sim/lib/people/sync/provider-request.ts:17">
P2: This exact pathname equality rejects valid Graph continuation links whose path differs in casing, which would fail the directory sync. Keep the origin pin and compare the collection path case-insensitively instead of requiring an exact match.

(Based on your team's feedback about Graph nextLink path matching.)</violation>
</file>

<file name="apps/sim/ee/scim/lib/application/users/provision-user.ts">

<violation number="1" location="apps/sim/ee/scim/lib/application/users/provision-user.ts:232">
P2: This new marker UPDATE waits on the org's `peopleDirectorySync` row, which the SCIM publish holds `FOR UPDATE` for its whole transaction. The transaction-local 5 s `lock_timeout` set earlier by the membership lock carries over, so a provisioning request that arrives during a slow publish fails with a retryable 503 instead of completing. Consider whether the marker can run outside this transaction or after the membership lock is taken, or confirm that publish stays well under 5 s at the directory sizes you support.</violation>
</file>

<file name="apps/sim/app/o/[organizationId]/people/people-directory.tsx">

<violation number="1" location="apps/sim/app/o/[organizationId]/people/people-directory.tsx:169">
P2: This directory accumulates every loaded page but leaves `Resource.Table` virtualization disabled, so a large organization renders thousands of row subtrees and grows the DOM linearly. Pass `virtualized` to bound rendered rows as more pages load.</violation>
</file>

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/people/provider-import.ts Resolved
Comment thread apps/sim/hooks/queries/people-sync.ts Resolved
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 89 files

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

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/hooks/queries/people-sync.ts Resolved
Comment thread apps/sim/lib/people/sync/runner.ts Resolved
Comment thread apps/sim/lib/people/sync/service.ts Resolved
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 91 files

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

Turn on auto-fix | Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 91 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 2e513a8 into staging Nov 3, 2026
95 checks passed
@waleedlatif1
waleedlatif1 deleted the codex/people-directory-sync branch November 3, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant