fix(tenancy): derive a directory person's institution instead of assuming one by satvikOS · Pull Request #25 · Tenurework/Tenure

satvikOS · 2026-08-17T02:57:01Z

What happened

The first version of this migration handled "exactly one institution" and refused otherwise. Run against the pilot, it refused:

ERROR: Refusing to guess: 172 DirectoryPerson rows but 2 institutions.

The refusal worked exactly as designed. Prisma rolled the transaction back, the deploy stopped before the service moved, and production carried on serving the previous version untouched — ADR-0001's structure doing its job.

But the assumption behind the original was wrong. The pilot has two institutions: rochester, and tenure-e2e-151851 — an end-to-end test fixture nobody cleaned up.

A correction to something I said earlier

I described the unfiltered directory read as latent "because there's effectively one tenant." There are two. It was live — 172 real students' and advisors' names and email addresses readable by any officer of either institution.

The fix

A directory person's institution was never a guess. It is written down twice over:

DirectoryPerson → SeatHolding → Role → Organization.institutionId
DirectoryPerson → OrganizationAdvisor → Organization.institutionId

The pilot census settles that this covers everyone:

total  reaching_a_tenant  reaching_none  reaching_several
172    172                0              0

No heuristic is needed, and none is used.

Two guards keep that a fact rather than a hope, and both name what they found:

Guard	Why
Straddler	A single `institutionId` cannot describe someone in two tenants; picking one hides half their records from the people who own them
Orphan	A person with no seat and no advisory link is genuinely underivable — say how many rather than placing them wherever looks likeliest

Verified against real PostgreSQL 16

Reproducing the pilot's shape — two institutions, people reachable only through seats and advisory links:

Each person lands in the institution their own relationships name (seat@r.edu→rochester, adv@r.edu→rochester, e2e@x.test→tenure-e2e-151851)
Straddler guard fires, naming the count
Orphan guard fires, naming the count and sample addresses
migrate diff --exit-code — no drift, exit 0
The full P3018 recovery was rehearsed end to end: reproduce the failure → migrate resolve --rolled-back → swap in this corrected file (a different checksum) → migrate deploy applies cleanly. The ledger keeps both the rolled-back and the applied record.

That last one matters because it is the exact procedure this needs on the pilot, and "does Prisma accept an edited migration after a rollback" is not a question worth discovering there.

Deploy order

main is currently wedged: the failed migration means no deploy can proceed until it is resolved. Sequence is merge this → Database recovery workflow with resolve-rolled-back → redeploy. Resolving before this merges would just retry the old SQL and fail again.

🤖 Generated with Claude Code

routing.ts, post.ts and policies.ts were written so the rules could not be bypassed — announceEvent takes an audience, never a channel. But nothing in the application could call it, because two things did not exist: anything that supplies a bot token and prior posts, and any concrete SlackPoster. The interface had no implementation, so the product could not post to Slack at all. announce.ts is the seam. It reads the workspace token from Secrets Manager via the Connection row's secretArn (never from a database column), reads prior posts for the event, calls announceEvent, and records a post that actually happened. It takes an audience, never a channel — restating the constraint so this wrapper cannot become the bypass that makes policies.ts advisory. Prior posts come from AuditEvent rather than a new table, following what the AI quota already does. A Slack post is an outward-facing act that belongs in the audit trail whether or not a quota needs it, and a separate table would have to be kept in step with that trail anyway. Only a post that happened consumes quota; a run of failures cannot exhaust an allowance nobody spent. Recording never throws — the message is already in Slack and cannot be unsent, so failing there would report an error for work that succeeded and invite a retry that posts twice. poster.ts is the implementation, and it exists to get one thing right: Slack answers a REFUSED call with HTTP 200 and {"ok": false}. The fetch Response.ok for that is true. Checking the wrong one reports a message as posted that Slack never delivered — and the quota then burns one of the club's three community slots on an announcement nobody saw. Both are checked, and Slack's own error code is carried out verbatim, because channel_not_found, not_in_channel and invalid_auth need three different responses from an administrator. 10 tests on the poster, the first of them that exact trap. 612 tests pass across 41 suites; tsc and lint clean. Still not provable end to end: posting needs a workspace bot token, which exists only after a Slack admin completes the install. This is the code that was missing behind that, not a substitute for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ming one The first version of this migration handled "exactly one institution" and refused otherwise. Run against the pilot it refused — correctly, and usefully: ERROR: Refusing to guess: 172 DirectoryPerson rows but 2 institutions. Two things follow. The refusal worked exactly as designed: Prisma rolled the transaction back, the deploy stopped before the service moved, and production carried on serving the previous version untouched. And the assumption behind the original was simply wrong — the pilot has TWO institutions, `rochester` and `tenure-e2e-151851`, the latter an end-to-end test fixture nobody cleaned up. That also corrects something stated earlier in this work: the unfiltered directory read was described as latent "because there is effectively one tenant". There are two. It was live. A directory person's institution was never a guess in the first place. It is written down twice over — in the seats they hold and in the clubs they advise — so the backfill now derives it: DirectoryPerson -> SeatHolding -> Role -> Organization.institutionId DirectoryPerson -> OrganizationAdvisor -> Organization.institutionId The pilot census settles that this covers everyone: of 172 people, 172 reach exactly one tenant, none reach zero, none reach several. No heuristic is needed and none is used. Two guards keep that a fact rather than a hope, and both name what they found: one refuses if any person straddles two institutions (a single institutionId cannot describe them, and picking one would hide half their records from the people who own them); the other refuses if any row is left unassigned rather than placing them wherever looks likeliest. Verified against a real PostgreSQL 16, reproducing the pilot's shape — two institutions, people reachable only through seats and advisory links: - each person lands in the institution their own relationships name - straddler guard fires, naming the count - orphan guard fires, naming the count and sample addresses - migrate diff --exit-code: no drift, exit 0 - the full P3018 recovery was rehearsed end to end: reproduce the failure, `migrate resolve --rolled-back`, swap in this corrected file (a different checksum), `migrate deploy` — applies cleanly, and the ledger keeps both the rolled-back and the applied record Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A high-effort review returned 15 findings on this branch. This works all of them. The first one changes the design. THE ID-IN-TEXT DESIGN COULD NOT WORK. `run.ts` finishes a successful call with `redactor.redact(result.text)`, and `redact.ts` replaces every `/\bc[a-z0-9]{24}\b/` with `[record N]`. Every id these tools printed — `entry`, `seat`, `holding`, `request` — is a Prisma `@default(cuid())`, so the model read `entry [record 1]`, never an id. The same fact killed `get_record_history`'s `resource_id` parameter: no tool in this build can emit a raw id into `text`, so the only value a model could ever pass back was the placeholder, which matches nothing. A parameter that reads well and can never be filled. Seventy tests were green over it, and all seventy called `tool.run` directly — upstream of the redactor. A unit test that mocks the layer where the defect lives cannot fail. `redaction-reaches-these-tools.test.ts` now dispatches through the real `runTool` with a real `createRedactor` and pins both halves: an id does NOT survive into the model's text, and a minted `Destination` DOES survive on the citation, which is not redacted. The redactor is right and the tools were wrong. So they mint instead — PR #259 landed the branded `Destination`, so rule 5's first branch applies. An audit row still has no page (`/admin/audit` calls `notFound()` without `audit.view`, so linking a club president there is the dead `/deliverables` of #244 again), so each row links to THE RECORD IT IS ABOUT: `/approvals/[id]` for an approval, `/orgs/[slug]/documents` for a document, the club's own page otherwise. A better answer than an id, and one the asker can open. SECURITY - `ApprovalRequest.organization` is a SINGLE-column relation, not composite, so the nested `select` for the club name was served with no tenant predicate. Second top-level query now. The header also claimed the one nested read was in `who_holds_seat` "on a composite-keyed relation" — wrong function, and false of the relation actually used. - `organization.name` and every person's name now go through `oneLine`. A display name is typed by a person and lands verbatim in a model's context. CONFIDENTLY WRONG ANSWERS - `updatedAt` is `@updatedAt` and moves on any write, so a request decided eleven months ago and edited yesterday was listed under "decided in the last 90 days" with its true date on the same line. The window now applies to the decision step. `updatedAt >= since` stays as a pre-filter and loses nothing: a decision is a write, so decidedAt <= updatedAt always. - the `person` argument was filtering the rows that decide the CURRENT holder, so "which seats has Dev held?" reported `Holds it now: vacant` for a seat with a sitting treasurer. It chooses which SEATS, not who holds them. - a truncated holdings scan could flip "vacant" on. `moreLine` is honest about a list; it does not retract a positive claim, and nobody reads "and more" as "actually there might be a president". - "Nothing has been recorded" was also said when rows existed and the seat rule withheld them all, and that branch dropped the truncation line too. - `Memory.CardCreated` carries no title and no sensitivity, so withholding it hid "you created a card" from its own author. Unresolvable memory rows are now KEPT with their metadata stripped — the metadata is the only part that can leak a deleted card's title. STRUCTURAL - `who_holds_seat` competes with `get_seat_history` over the same noun and was registered AHEAD of it, so "what did the last president leave behind" routed to the tool that returns no memory cards. It is exported separately and registered behind it. - `auditReadableOrgIds` assumes archived clubs are already gone; they were not. `visibleOrganizations` takes an opt-in `operableOnly` — opt-in because nine tools share it and whether an archived club still answers budget questions is a product call, not something to decide inside a branch about audit rows. - the decision-trail query had no `take`, the file's one unbounded read. - names were resolved for the whole scan set rather than the rows shown. - `get_approval_outcome` skips `NO_STANDING` on purpose (a requester keeps sight of their own request from a club they left) — now documented, and the test that claimed to cover "all three" says why it covers two. - a fixture pinned to a literal 2027 would have flipped to a false pass and then failed on 2027-06-01; it is relative to the clock now. The runner test caught one of my own fixtures: a 24-character id is not cuid-shaped, so it sailed through the redactor and the test failed for a reason unrelated to the runner. All fixtures verified at 25. Verified: tsc 307 (parity), 410 tests green in src/lib/ai/tools, full suite at the documented 3 pristine-main failures plus activation-timing, which is the known flake in #25 (zero files touched under auth/, passes 2 runs in 3). All three required mutations redden, and so does one mutation per review finding — ten in total, each md5-guarded and restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…257) * Give the assistant the reach the 216 questions need, as three tools A 37-surface sweep of 457 real officer questions found 216 the assistant could not answer because no tool could see the row. They cluster on six models, and three of those six already had a tool that names rows — Event (list_upcoming_events), MemoryRecord (find_institutional_memory), and the OPEN half of ApprovalRequest (list_open_approvals). Adding a second tool over any of them would not have added reach, it would have added a competitor: workspace-tools.ts records what happened the last time two plausible tools mentioned the same noun. So this adds a tool only where there was no way to see the row at all. get_record_history AuditEvent (59). Nothing in the build could read the audit trail, so "who changed this" got a plausible guess rather than an answer or a refusal. who_holds_seat RoleAssignment (39) + Role (37). list_club_roster names holders but prints no date and is keyed by club, so "since when" and "which seats does this person hold" were both unaskable. get_approval_outcome ApprovalRequest (25), the decided half. The moment a request is approved or rejected it left the assistant's sight entirely. The five rules, and where each is enforced: TENANCY. Every related row is a second TOP-LEVEL query bounded by an id set resolved under institutionId — never an include, because the tenancy extension fires once per top-level delegate call. The one nested read is a composite-keyed relation and says so. SEAT, NOT JUST TENANT. lib/memory-moves.ts writes a card's TITLE and SENSITIVITY onto its audit row, and summarizeAuditMetadata prints them. Returning a tenant's audit rows without canSeeMemoryCard would publish through the trail exactly what the card withholds — a leak within a tenant. A card that cannot be resolved is withheld, not shown. BOUNDED READS WITH AN HONEST REMAINDER. Every tool takes a limit, every query is take: cap + 1, and both the scan cap and the limit report through moreLine. The count of rows withheld by the SEAT rule is deliberately not reported: that would turn the honesty rule into a disclosure channel. AS-OF, FROM THE READ. readAt is a bare new Date() after the await, not context.now (when the model asked) and not requestClock() (which is react/cache'd and returns the request's start inside an RSC render). It goes into the result TEXT as well as the citation, so the model can qualify the sentence and not only the footnote. NAMES AND IDENTIFIERS. Every row is printed with the person who acted and with its own row id. None is printed with a route: an AuditEvent has no page, and /admin/audit calls notFound() unless the viewer holds audit.view — so a hand-built link there would be DEAD for most of the people these tools answer, which is the /deliverables defect from #244 with a different spelling. There is no branded Destination to mint through in this build; minting is the follow-up. Two of my own tests were caught overclaiming by the mutation sweep and are fixed rather than left: - the who_holds_seat cross-tenant fixture put the foreign seat in org_chess, where the holdings query's predicate excluded it. It passed with the seat scope deleted, so it was proving nothing. - "re-checks each row with canViewApproval" did not. Replacing that filter with `true` leaves the suite green, because the query's OR and canViewApproval agree by construction. It is defence in depth and now says so, in the source and in the test. assignment-queries-are-effective-dated.test.ts flagged the roleAssignment read: its scanner cannot tell `select: { status: true }` from a where filter, and should not try — narrowing it to the where clause is how it would lose the nested-relation door that cost it two real sites. The query loads every holding with its dates and narrows in code with withEffectiveStatus, so it takes an allowlist entry with that reason. Verified: tsc 307 (parity), jest 3 failing suites (parity — connectors/ audience, nothing-manufactures-the-member-seat, identity/onboarding-form), 56 new tests, lint clean. All three required mutations proven to redden with md5 no-op guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Name a seat nobody has ever held, instead of answering as if it is not there Self-review finding, caught while both PR reviewers were unavailable (CodeRabbit rate-limited, Greptile out of trial credits — so the green CodeRabbit check on this PR is a limit notice, not a review). `who_holds_seat` filtered its seat list on "has at least one holding", unconditionally. A seat that has never been filled therefore vanished from the answer, and asking "who is the treasurer?" about one produced: No seat in Chess Club has anybody on record against it. which names no seat and reads as "there is no such seat". Those are two different facts. "Nobody has ever held it" is an answer, and frequently the one the asker most needs — it is the seat somebody has to fill. "That seat does not exist" is the other one, and it was already returned above when the seat query came back empty. Conflating them is the same class of defect as a bounded read under an unbounded promise: the sentence is confident and the reader draws the wrong conclusion. The holding filter now applies only when a `person` argument was given, which is the case where "seats this person has something to do with" is what was asked. Without one, every scanned seat is covered and a vacant one reports "Holds it now: vacant" — the same treatment `list_club_roster` already gives its vacant seats. Two tests pin it, and the fix is proven load-bearing: reverting to the unconditional filter reddens "names a seat nobody has ever held". tsc 307 (parity), 58 tests in the new suite, lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Mint the link instead of printing an id the model never sees A high-effort review returned 15 findings on this branch. This works all of them. The first one changes the design. THE ID-IN-TEXT DESIGN COULD NOT WORK. `run.ts` finishes a successful call with `redactor.redact(result.text)`, and `redact.ts` replaces every `/\bc[a-z0-9]{24}\b/` with `[record N]`. Every id these tools printed — `entry`, `seat`, `holding`, `request` — is a Prisma `@default(cuid())`, so the model read `entry [record 1]`, never an id. The same fact killed `get_record_history`'s `resource_id` parameter: no tool in this build can emit a raw id into `text`, so the only value a model could ever pass back was the placeholder, which matches nothing. A parameter that reads well and can never be filled. Seventy tests were green over it, and all seventy called `tool.run` directly — upstream of the redactor. A unit test that mocks the layer where the defect lives cannot fail. `redaction-reaches-these-tools.test.ts` now dispatches through the real `runTool` with a real `createRedactor` and pins both halves: an id does NOT survive into the model's text, and a minted `Destination` DOES survive on the citation, which is not redacted. The redactor is right and the tools were wrong. So they mint instead — PR #259 landed the branded `Destination`, so rule 5's first branch applies. An audit row still has no page (`/admin/audit` calls `notFound()` without `audit.view`, so linking a club president there is the dead `/deliverables` of #244 again), so each row links to THE RECORD IT IS ABOUT: `/approvals/[id]` for an approval, `/orgs/[slug]/documents` for a document, the club's own page otherwise. A better answer than an id, and one the asker can open. SECURITY - `ApprovalRequest.organization` is a SINGLE-column relation, not composite, so the nested `select` for the club name was served with no tenant predicate. Second top-level query now. The header also claimed the one nested read was in `who_holds_seat` "on a composite-keyed relation" — wrong function, and false of the relation actually used. - `organization.name` and every person's name now go through `oneLine`. A display name is typed by a person and lands verbatim in a model's context. CONFIDENTLY WRONG ANSWERS - `updatedAt` is `@updatedAt` and moves on any write, so a request decided eleven months ago and edited yesterday was listed under "decided in the last 90 days" with its true date on the same line. The window now applies to the decision step. `updatedAt >= since` stays as a pre-filter and loses nothing: a decision is a write, so decidedAt <= updatedAt always. - the `person` argument was filtering the rows that decide the CURRENT holder, so "which seats has Dev held?" reported `Holds it now: vacant` for a seat with a sitting treasurer. It chooses which SEATS, not who holds them. - a truncated holdings scan could flip "vacant" on. `moreLine` is honest about a list; it does not retract a positive claim, and nobody reads "and more" as "actually there might be a president". - "Nothing has been recorded" was also said when rows existed and the seat rule withheld them all, and that branch dropped the truncation line too. - `Memory.CardCreated` carries no title and no sensitivity, so withholding it hid "you created a card" from its own author. Unresolvable memory rows are now KEPT with their metadata stripped — the metadata is the only part that can leak a deleted card's title. STRUCTURAL - `who_holds_seat` competes with `get_seat_history` over the same noun and was registered AHEAD of it, so "what did the last president leave behind" routed to the tool that returns no memory cards. It is exported separately and registered behind it. - `auditReadableOrgIds` assumes archived clubs are already gone; they were not. `visibleOrganizations` takes an opt-in `operableOnly` — opt-in because nine tools share it and whether an archived club still answers budget questions is a product call, not something to decide inside a branch about audit rows. - the decision-trail query had no `take`, the file's one unbounded read. - names were resolved for the whole scan set rather than the rows shown. - `get_approval_outcome` skips `NO_STANDING` on purpose (a requester keeps sight of their own request from a club they left) — now documented, and the test that claimed to cover "all three" says why it covers two. - a fixture pinned to a literal 2027 would have flipped to a false pass and then failed on 2027-06-01; it is relative to the clock now. The runner test caught one of my own fixtures: a 24-character id is not cuid-shaped, so it sailed through the redactor and the test failed for a reason unrelated to the runner. All fixtures verified at 25. Verified: tsc 307 (parity), 410 tests green in src/lib/ai/tools, full suite at the documented 3 pristine-main failures plus activation-timing, which is the known flake in #25 (zero files touched under auth/, passes 2 runs in 3). All three required mutations redden, and so does one mutation per review finding — ten in total, each md5-guarded and restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * A bare take: cap cannot tell a truncated trail from a decision that isn't there CodeRabbit's rate limit lifted and it caught a real bug — one I introduced in the previous commit, closing the file's one unbounded read. `take: STEP_SCAN_CAP` reads exactly the cap, so the handler cannot distinguish a TRUNCATED scan from a real ABSENCE. Two things went wrong at once, and the second is worse: 1. It printed "no decision step is on record" for a request whose step exists and simply was not read. That is an assertion of absence made from a bounded read — the exact defect rule 3 of this file's header is about, committed by the fix for rule 3. 2. The window filter treats a missing decision as "cannot be placed" and KEEPS the row. The step query is ordered `occurredAt: desc`, so the OLDEST decisions drop first — quietly re-admitting a request decided eleven months ago into "decided in the last N days", reinstating the bug the previous commit had just fixed and which a test in this file was written to pin. `take: cap + 1` — the idiom the rest of the file already uses, and the one this query should have had from the start. The extra row makes the truncation a fact: the line says the trail was not fully read instead of claiming absence, and the answer discloses that a request shown without a decision date may fall outside the window. Keeping the row and stating the bound is the honest pair; dropping it would narrow the answer silently, and keeping it silently would present a window the call could not enforce. Two tests, and both redden on a bare `take: cap`. Building them found a fixture bug worth noting: steps pointed at an id no request in the store had, so the query returned nothing, the scan was never truncated, and the test would have passed for the wrong reason. The "fell off the end" case now needs what it describes — a full cap of another request's newer steps in front of this request's older one. tsc 307 (parity), 412 tests green in src/lib/ai/tools, lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Satvik <satvik@tenurework.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

satvikOS and others added 2 commits August 16, 2026 22:45

satvikOS merged commit fb6a9a4 into main Aug 17, 2026
4 checks passed

satvikOS deleted the fix/directory-backfill-from-relationships branch August 17, 2026 04:52

satvikOS mentioned this pull request Aug 25, 2026

Give the assistant the reach the 216 questions need, as three tools #257

Merged

Navigation Menu

Uh oh!

fix(tenancy): derive a directory person's institution instead of assuming one - #25

fix(tenancy): derive a directory person's institution instead of assuming one#25
satvikOS merged 2 commits into
mainfrom
fix/directory-backfill-from-relationships

satvikOS commented Aug 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

satvikOS commented Aug 17, 2026

What happened

A correction to something I said earlier

The fix

Verified against real PostgreSQL 16

Deploy order

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant