refactor(ai-chat): single live+enabled role resolve in the repo (#95)

resolveRoleForRequest and resolveShareRole duplicated the security invariant
'role exists, not soft-deleted, enabled, workspace-scoped, else null'. Move it to
AiAgentRoleRepo.findLiveEnabled(id, workspaceId) (deletedAt IS NULL + enabled +
workspace scope) and have both services call it, preserving each one's roleId
derivation + null handling. (describeProviderError half of #95 was done earlier.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-21 03:49:52 +03:00
parent 3147b6ddf4
commit 3953ecdb17
6 changed files with 122 additions and 29 deletions

View File

@@ -0,0 +1,51 @@
import { AiAgentRoleRepo } from './ai-agent-roles.repo';
import type { KyselyDB } from '../../types/kysely.types';
/**
* Unit test for the SECURITY invariant carried by
* AiAgentRoleRepo.findLiveEnabled: it is the single source of truth shared by
* the authenticated chat and the anonymous public-share assistant for "resolve
* a roleId to a LIVE, ENABLED role scoped to the workspace, else undefined".
*
* A live Postgres is out of scope here; instead we record the query the repo
* builds and assert it pins ALL of the security filters: id, workspaceId,
* deletedAt IS NULL, and enabled = true. If any of those `where` clauses is
* dropped, the role scoping silently widens — this test guards exactly that.
*/
describe('AiAgentRoleRepo.findLiveEnabled', () => {
function makeRepoWithSpy(result: unknown) {
const where = jest.fn();
const builder = {
selectAll: jest.fn(() => builder),
where: jest.fn((...args: unknown[]) => {
where(...args);
return builder;
}),
executeTakeFirst: jest.fn().mockResolvedValue(result),
};
const db = {
selectFrom: jest.fn(() => builder),
} as unknown as KyselyDB;
return { repo: new AiAgentRoleRepo(db), db, where };
}
it('queries scoped to id + workspace, live (deletedAt null) AND enabled', async () => {
const role = { id: 'r-1', workspaceId: 'ws-1', enabled: true };
const { repo, db, where } = makeRepoWithSpy(role);
const result = await repo.findLiveEnabled('r-1', 'ws-1');
expect(result).toBe(role);
expect(db.selectFrom).toHaveBeenCalledWith('aiAgentRoles');
// Every security filter must be present.
expect(where).toHaveBeenCalledWith('id', '=', 'r-1');
expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1');
expect(where).toHaveBeenCalledWith('deletedAt', 'is', null);
expect(where).toHaveBeenCalledWith('enabled', '=', true);
});
it('returns undefined when no live+enabled role matches', async () => {
const { repo } = makeRepoWithSpy(undefined);
expect(await repo.findLiveEnabled('r-1', 'ws-1')).toBeUndefined();
});
});

View File

@@ -32,6 +32,29 @@ export class AiAgentRoleRepo {
.executeTakeFirst();
}
/**
* Single live (not soft-deleted) AND enabled role scoped to the workspace, or
* undefined. This is the SECURITY invariant shared by the authenticated chat
* and the anonymous public-share assistant: a role only applies its persona /
* model override when it currently exists, is not soft-deleted, and is enabled
* — a disabled or deleted role server-authoritatively degrades to the built-in
* universal assistant. Single source of truth so the two resolve paths cannot
* drift apart.
*/
async findLiveEnabled(
id: string,
workspaceId: string,
): Promise<AiAgentRole | undefined> {
return this.db
.selectFrom('aiAgentRoles')
.selectAll('aiAgentRoles')
.where('id', '=', id)
.where('workspaceId', '=', workspaceId)
.where('deletedAt', 'is', null)
.where('enabled', '=', true)
.executeTakeFirst();
}
/** All live roles for the workspace (management list + chat picker). */
async listByWorkspace(workspaceId: string): Promise<AiAgentRole[]> {
return this.db