feat(llc): one shared people directory, with roles scoping involvement

[mrveiss]

Closes #13998

Thinking Path

Owner's model, which is what this implements:

Company OS — companies are like departments. You can have one big company, or multiple small ones
split into IT, marketing and sales companies with their own agenda, so you can build different
business directions but under one real company.

real employees and contacts are shared across the Company OS — that means shared

So there is one real company and Company OS "companies" are its departments. People belong to the
business; departments are internal divisions of it. One installation-wide directory of humans
follows directly — a department never re-creates a person the business already employs or deals with.

What is department-scoped is the role a person holds and the work it touches. That is already
modelled: llc_role_assignments carries holder_type = contact (#14221 step 2).

What Changed From The First Draft

The first version of this PR added llc_contact_company_links plus a migration, gating directory
visibility per company. The owner's correction removed the need for it entirely, so the table and
its migration are gone. Net effect of this PR: no schema change, no migration.

Worth stating plainly because it is the substantive point: the correction did not add work, it
deleted a table I had proposed. Involvement was already expressible through role tenures, and having
two ways to say "this person is involved with this department" would have let them drift apart.

Consistency with #13992

A shared directory is not shared access. Knowing a supplier exists is not permission to see the
work they do — that still requires a role in the department, and role permissions still gate what a
holder may do. Need-to-know is unchanged.

What Changed

ContactDirectoryService (llc/services/contact_directory.py, new):

• list_directory() — installation-wide, and deliberately takes no company_id, so a filter
  cannot creep back in and quietly re-introduce the duplication this removes.
• companies_for_contact() — derived from open role tenures, not stored.
• delete() — refuses while the contact holds a role anywhere, and the error names which
  departments to look at.
• merge() — moves role tenures to the survivor, then deletes the duplicate.
• find_duplicate_candidates() — suggests groups sharing a mailbox, case-insensitively.

Four new routes on llc/api/contacts.py:

• GET /contacts/directory — the shared directory, no company_id.
• GET /contacts/directory/duplicates — duplicate candidates, ids only.
• POST /contacts/{company_id}/merge — fold one contact into another; company-scoped because the
  authority to merge is a department admin's, even though the directory itself is shared.
• DELETE /contacts/{company_id}/directory/{contact_id} — remove a person from the shared directory;
  409 (not 400) while they still hold a role, naming which departments depend on them.

Review fix: the legacy DELETE /contacts/{company_id}/{contact_id} route still called
ContactService.delete, filtered only on (id, company_id) and gated only by assert_company_access
(bare tenant-membership, not admin) — it never consulted companies_for_contact, so it bypassed the
new guard entirely. A plain member of the contact's legacy company could hard-delete a contact who
still held an active role in a company they weren't even a member of, leaving
LLCRoleAssignment.holder_contact_id an unresolvable reference (no FK/cascade, by design). Fixed by
delegating delete_contact to ContactDirectoryService.delete through the same
_delete_via_directory helper the new directory-delete route uses, so both doors share one guard.

Deletion is the sharp edge

In a shared directory a delete is global. Without a guard, an admin of the marketing department could
empty the IT department's org chart without ever seeing it happen. delete() therefore refuses while
any open tenure exists — from either delete route, since both now call the same guard.

Merging stays explicit

Deciding two rows are the same human is a judgement. Matching on email would fuse two people's PII on
a guess — irreversible and invisible afterwards; two people legitimately share info@supplier. So
detection suggests and merge acts, only on ids a caller named.

Verification

9 service-level tests, every guard mutation-proved:

Mutation	Reddens
delete without checking held roles	test_delete_is_refused_while_the_contact_holds_a_role
merge without moving tenures	test_merge_moves_role_tenures_to_the_survivor
count ended tenures as involvement	test_an_ended_tenure_no_longer_counts_as_involvement

Those nine all entered through ContactDirectoryService directly, which is exactly why the suite
stayed green while the legacy route's bypass was open — a guard tested only through the door it's
mounted on says nothing about the other door. Two router-level tests close that
(llc/tests/test_contacts_delete_route_guard.py), seeded from real rows and driven through the real
FastAPI route via httpx.AsyncClient, not a mocked service call:

• test_legacy_delete_route_refuses_when_the_contact_holds_a_role_elsewhere — an admin of the
  contact's own company, contact holds a role elsewhere → 409, contact untouched. Against the PR head
  before this fix this returned 204 and deleted the row.
• test_legacy_delete_route_refuses_a_plain_member_regardless_of_roles — the review's concrete
  scenario verbatim: a plain member (not admin) of the contact's own company → 403, contact untouched.
  Before this fix, assert_company_access doesn't check membership role, so this also returned 204.

llc/tests/test_contacts_idor.py updated to patch ContactDirectoryService.delete (not
ContactService.delete) for the delete-route tests, matching the new delegation.

find_duplicate_candidates is plain SQL rather than array_agg: that is Postgres-only, and a helper
that cannot run on the SQLite harness is one whose behaviour is never verified.

alembic heads unchanged from base, since this PR adds no migration.

Not in this PR

The frontend UI for the directory, merge and duplicate-review flows, and migrating
ContactService's existing per-company readers (list_by_company, get, update, create) onto
the directory service. LLCContact.company_id is untouched here — no reader is broken, and dropping
the column is a follow-up once nothing depends on it.

Model Used

Claude Opus 5 (1M context)

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[mrveiss]

Review — BLOCK, one critical item

The design is right and I'm not relitigating it: an installation-wide directory with roles scoping involvement rather than visibility is the recorded owner decision on #14466, and the module docstring already said companies here are organisational units, not tenants. The unscoped GET /directory read is by design.

What blocks is that this PR's headline safety property is bypassable through a route the PR didn't touch.

delete_contact (llc/api/contacts.py, the legacy DELETE /{company_id}/{contact_id}) still calls ContactService.delete filtered on (id, company_id) and gated only by assert_company_access — which is a bare tenant-id comparison, i.e. membership, not admin. It never consults companies_for_contact, so it walks straight past the ContactInUseError guard this PR adds in contact_directory.py.

Concretely: a contact holds an active role in company B. A plain member of company A — the contact's legacy company_id, not an admin — calls the old delete route. The contact and its PII are hard-deleted, no 409 is raised, and because LLCRoleAssignment.holder_contact_id carries no FK or cascade by design, company B is left with an unresolvable role-holder reference. That is precisely the harm the PR describes as its reason for existing — "an admin of one company could otherwise empty another company's org chart without ever seeing it happen" — except it doesn't even take an admin.

The guard isn't wrong; it's just not on every door. Two ways to close it, either is fine:

• delegate delete_contact to the directory service so both routes share the one guard, or
• retire the legacy route in favour of /directory/{contact_id}, now that contacts are shared.

A test must cover the cross-endpoint path. The nine new tests all discriminate, but every one of them enters through ContactDirectoryService. The bypass lives at the router level, which is exactly why the suite is green while the hole is open — a guard tested only through the door it's mounted on can't tell you about the other door.

Two documentation items, non-blocking

• The PR body's "Not in this PR — routes and the frontend" is stale: the diff adds four routes (/directory, /directory/duplicates, /{company_id}/merge, /{company_id}/directory/{contact_id}). Worth fixing so reviewers aren't misled about scope.
• decision(llc): can one contact be shared across companies, or does each company get its own row? #13998's only comment still describes the abandoned llc_contact_company_links per-company-visibility design. The superseding decision lives on docs(llc): record what a Company OS company is — departments of one real company, not tenants #14466 and was never linked back. Anyone reading decision(llc): can one contact be shared across companies, or does each company get its own row? #13998 alone concludes this PR implements the wrong model — cross-link both ways.

Noted for later, no action here: ContactResponse.company_id no longer answers "which companies is this person involved with" now that involvement is multi-company. The PR already defers the column, so this is only a flag for whoever wires the frontend — don't let it become ground truth.

Confirmed clean: no migration touched (no data-loss risk), merge() correctly moves role tenures before deleting the duplicate so it doesn't reproduce the dangling reference, mutation paths reuse require_company_admin/assert_company_access rather than inventing a parallel check, audit logging goes through activity_log.record with ids only and no PII, all new functions ≤30 lines, and api.ts is bot-generated and consistent with the Python schemas rather than hand-edited.

[mrveiss]

The red here is inherited from the base — do not "fix" it in this PR

code-quality (and python_file_size_ratchet_test.py::test_recorded_ceilings_match_the_files_today in the Python suite) are failing on:

autobot-backend/chat_workflow/tool_handler.py: 4081 lines, over its recorded ceiling of 4063.

This PR did not cause it. Dev_new_gui itself is over the ceiling — from my merge of #14470 (0bd3326cb, closing #14420) — and Actions checks out the pull-request merge commit, so every open PR fails on the base's content regardless of what it changes.

Do not raise KNOWN_LARGE to make this green. The ratchet exists to refuse exactly that, and its message says so: "the exemption freezes the size it was granted for, it does not license more."

Fix in flight: PR #14497 (#14495) extracts the six _enforce_* dispatch guards into chat_workflow/tool_dispatch_guards.py, taking the file 4081 → 3863 and lowering the ceiling to match. Once that lands, rebase or push an empty commit and this goes green on its own.

Related: #14498 records a gap in the ratchet itself — its header says "raising a ceiling must fail" while the test only blocks raises above a frozen baseline, so a shrink can be spent back one compliant-looking PR at a time.

[mrveiss]

The People tab now reads the shared model — and states what it cannot explain

The directory work created an inconsistency I had to resolve rather than leave: the People tab read
/contacts/{company_id}, which filters on the legacy company_id column, while the directory
returns everyone. Two answers to "who are our people".

Neither obvious fix was right:

• Point it at the whole directory — every department shows an identical list, and an org chart
  stops being about that department.
• Point it at role-holders only — correct under the model, and empty today, because no
  contact holds a role yet. People would silently vanish from departments already using them, and an
  empty list reads as "this department has nobody" rather than "the query changed".

Two groups, labelled

GET /api/llc/contacts/{company_id}/involved returns:

• with_role — people holding an open role tenure here. Under the owner's model this is what
  "part of this department" means.
• unassigned — people carrying the legacy per-company stamp with no role to explain them.

They are returned and rendered separately. Merging asserts an involvement nobody recorded;
dropping the second group makes people disappear. Showing them under a "no role" label states the
truth — they are here, and nothing explains why — which is the only version that prompts anyone to
fix it. The group empties itself as roles are assigned, so it disappears when the migration is
genuinely complete rather than on a date someone guessed.

A bug the new test caught immediately

The first placement of the label put it inside v-if="person.orgNodeId" — the branch for hierarchy
members. A comment two lines above says "a contact has no org-chart node, so it is plain text", so
the badge was unreachable for exactly the people it describes. It is now outside both name
branches, and the reason is recorded there.

Verification

266 frontend tests pass (27 in the People suite, 14 backend directory tests). Mutation-proved on
both sides:

Mutation	Reddens
merge the two groups	test_department_view_separates_role_holders_from_legacy_contacts
drop the legacy group	same, plus test_a_legacy_contact_moves_group_once_it_holds_a_role
list a role-holder twice	test_a_role_holder_is_never_listed_twice
label only hierarchy members	labels a contact no role explains, and only that one
stop tracking unassigned ids	same
drop the unassigned group in the view	same

The 18 existing People tests that failed were a deliberate contract change, not a regression:
they mocked the old URL. Updated to the new endpoint and shape, with a fixture parameter so a test
can put someone in either group.

vue-tsc → 0 errors against a baseline of 0. oxlint clean. Two new strings in all 11 locales.