Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .agent/architect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Identity: Staff Architect

**Role:** Senior Principal Software Architect for Fluxnex.

**Specialization:** C4 Model, System Design, and Trade-off Analysis.

---

## SYSTEM INSTRUCTIONS

### 1. THE RULE OF LAW

- You **DO NOT** write implementation code.
- You **DO** design interfaces, data structures, and architectural boundaries.
- Your output is the "Blueprint"; the Coding Agent's output is the "Bricklaying".

### 2. MANDATORY CAPABILITIES

- **C4 Model:** All system descriptions must use Context, Container, and Component levels.
- **Visuals:** Always provide diagrams using **Mermaid.js** syntax.
- **Decision Records:** When suggesting a technology or pattern, output a mini-ADR (Architecture Decision Record) listing:
- Context (The problem)
- Decision (What we are doing)
- Alternatives (What we rejected)
- Consequences (Pros/Cons)

### 3. PROJECT CONTEXT

- **Project:** Fluxnex (Data Sync Platform).
- **Stack:** NestJS, Drizzle ORM, Frappe, React.
- **Key Integrations:** QuickBooks Desktop (Web Connector), EDI Standards.

### 4. OUTPUT FORMAT

Structure your response as:

1. **High-Level Design** (Text)
2. **Visual Diagram** (Mermaid Code Block)
3. **Interface/Schema Definitions** (TypeScript/SQL)
35 changes: 35 additions & 0 deletions .agent/reviewer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Identity: Lead Reviewer

**Role:** Lead Security & Quality Auditor.

**Personality:** Strict, adversarial, and production-focused.

---

## SYSTEM INSTRUCTIONS

### 1. THE GOAL

Your job is to catch bugs BEFORE they merge. You are not here to be nice; you are here to ensure stability.

### 2. THE AUDIT CHECKLIST

Before approving ANY code, verify these 4 pillars:

* **🛡️ Security:** Scan for OWASP Top 10 (SQL Injection, XSS, Hardcoded Secrets).
* **🧠 Deep Logic:** Look for race conditions, unhandled null states, and off-by-one errors.
* **✅ Tool Compliance:**
* Ask: *"Has Qodo generated tests for this?"* (If not, block it).
* Ask: *"Are there active SonarLint warnings?"* (If yes, block it).
* **⚡ Performance:** Flag O(n^2) loops or heavy computations on the main thread.
Comment on lines +21 to +24

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.

🧹 Nitpick | 🔵 Trivial

Make tooling checks tool‑agnostic to avoid hard process lock‑ins.

Mandating a block if Qodo tests aren’t generated or if SonarLint warnings exist will create false blockers in environments where those tools aren’t required or used. Consider rephrasing to “if applicable/available” or referencing “project‑approved tooling” instead of specific vendors.

♻️ Suggested wording
-* **✅ Tool Compliance:**
-  * Ask: *"Has Qodo generated tests for this?"* (If not, block it).
-  * Ask: *"Are there active SonarLint warnings?"* (If yes, block it).
+* **✅ Tool Compliance:**
+  * Ask: *"Have required tests (per project policy) been generated or updated?"* (If not, block it).
+  * Ask: *"Are there active lint/static‑analysis warnings from the project‑approved tooling?"* (If yes, block it).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* **✅ Tool Compliance:**
* Ask: *"Has Qodo generated tests for this?"* (If not, block it).
* Ask: *"Are there active SonarLint warnings?"* (If yes, block it).
* **⚡ Performance:** Flag O(n^2) loops or heavy computations on the main thread.
* **✅ Tool Compliance:**
* Ask: *"Have required tests (per project policy) been generated or updated?"* (If not, block it).
* Ask: *"Are there active lint/static‑analysis warnings from the project‑approved tooling?"* (If yes, block it).
* **⚡ Performance:** Flag O(n^2) loops or heavy computations on the main thread.
🤖 Prompt for AI Agents
In @.agent/reviewer.md around lines 21 - 24, Update the "Tool Compliance"
checklist entries (the lines containing "Ask: \"Has Qodo generated tests for
this?\"" and "Ask: \"Are there active SonarLint warnings?\"") to be
tool‑agnostic: replace explicit vendor names with conditional wording like "If
applicable/available, has project‑approved tooling (e.g., test generator or
static analysis) been run?" and change the blocking guidance to "block only if
tooling is applicable and indicates blocking issues" so the "Tool Compliance"
and "⚡ Performance" checks reference project‑approved tooling rather than
hardcoding Qodo or SonarLint.


### 3. OUTPUT FORMAT

Classify your findings into two categories:

* 🔴 **BLOCKING:** Critical bugs, security risks, or missing tests. (The code CANNOT merge).
* 🟡 **NITPICK:** Variable naming, formatting, or minor refactoring suggestions. (Optional fixes).

### 4. FINAL VERDICT

End every review with a clear: **PASS** or **FAIL**.
1 change: 1 addition & 0 deletions apps/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"@types/pg": "^8.16.0",
"@types/supertest": "^6.0.2",
"@types/uuid": "^11.0.0",
"dotenv": "^17.2.3",
"drizzle-kit": "^0.31.8",
"eslint": "^9.18.0",
"eslint-config-prettier": "^10.0.1",
Expand Down
10 changes: 6 additions & 4 deletions apps/api/src/modules/auth/auth.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ describe('AuthController', () => {
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument */
const res = { setHeader: jest.fn() } as any;

const result = await controller.completeInvite(body, res);
const mockRequest = { headers: {} } as unknown as Request;
const result = await controller.completeInvite(body, res, mockRequest);
/* eslint-enable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument */

expect(result).toBeDefined();
Expand Down Expand Up @@ -154,9 +155,10 @@ describe('AuthController', () => {
/* eslint-disable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument */
const res = { setHeader: jest.fn() } as any;

await expect(controller.completeInvite(body, res)).rejects.toThrow(
'Failed to accept invitation',
);
const mockRequest = { headers: {} } as unknown as Request;
await expect(
controller.completeInvite(body, res, mockRequest),
).rejects.toThrow('Failed to accept invitation');
/* eslint-enable @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument */
expect(mockBetterAuthIdentityProvider.deleteUser).toHaveBeenCalledWith(
'user-fail',
Expand Down
46 changes: 33 additions & 13 deletions apps/api/src/modules/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,13 @@ export class AuthController {
async completeInvite(
@Body() body: CompleteInvite,
@Res({ passthrough: true }) res: Response,
@Req() req: Request,
) {
// Race Condition Fix: Validate Invitation BEFORE creating user
const invitation = (await this.invitationsService.get(
const invitation = await this.invitationsService.get(
body.invitationId,
)) as { status: string; expiresAt: Date } | null;
req.headers,
);

if (!invitation) {
throw new BadRequestException('Invalid Invitation ID');
Expand All @@ -108,6 +110,7 @@ export class AuthController {

// Step 1: Check if user exists (Provisioned vs New)
let user = await this.authProvider.getUserByEmail(body.email);
let isNewUser = false;

if (user) {
if (user.emailVerified) {
Expand All @@ -122,23 +125,40 @@ export class AuthController {
await this.authProvider.setPassword(user.id, body.password);
} else {
// Create new user (standard flow)
user = await this.authProvider.createUser({
email: body.email,
password: body.password,
firstName: body.firstName,
lastName: body.lastName,
role: 'user',
});
isNewUser = true;
user = await this.authProvider.createUser(
{
email: body.email,
password: body.password,
firstName: body.firstName,
lastName: body.lastName,
role: 'user',
},
req.headers,
);
}

// Step 2: Accept Invitation (Atomic-ish)
try {
await this.invitationsService.accept(body.invitationId, user.id);
console.log(
'DEBUG: completeInvite - accepting invitation for user:',
user.id,
);
await this.invitationsService.accept(
body.invitationId,
user.id,
req.headers,
);
} catch (_e) {
// Rollback: Delete the user if acceptance fails to prevent orphans
await this.authProvider.deleteUser(user.id);
console.error('DEBUG: acceptInvitation failed');
// Rollback: Only delete if WE created the user in this transaction
if (isNewUser) {
console.log('DEBUG: Rolling back new user creation:', user.id);
await this.authProvider.deleteUser(user.id);
}
throw new BadRequestException(
'Failed to accept invitation (User creation rolled back)',
'Failed to accept invitation' +
(isNewUser ? ' (User creation rolled back)' : ''),
);
}

Expand Down
22 changes: 13 additions & 9 deletions apps/api/src/modules/auth/identity-provider.abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ import { CreateUser } from '../users/users.validation';
*/
import { User } from '../users/user.schema';
import { Session } from './auth.schema';
import { Invitation } from '../invitations/invitation.interface';

/**
* Abstract Class defining the contract for Identity Providers.
* ...
*/
export abstract class IdentityProvider {
/**
* Creates a user in the external identity system.
* @param user The user details to create.
* @returns The created user object from the provider.
*/
abstract createUser(user: CreateUser): Promise<User>;
abstract createUser(
user: CreateUser,
headers?: Headers | Record<string, any>,
): Promise<User>;

/**
* Authenticates a user and returns a session/token.
Expand All @@ -45,7 +45,7 @@ export abstract class IdentityProvider {
* This is preferred over validateSession(token) when dealing with signed cookies.
*/
abstract getSessionFromHeaders(
headers: Headers,
headers: Headers | Record<string, any>,
): Promise<{ session: Session; user: User } | null>;

/**
Expand All @@ -71,20 +71,24 @@ export abstract class IdentityProvider {
organizationId: string | null;
expiresIn?: number;
inviterId: string;
headers?: Headers;
headers?: Headers | Record<string, any>;
}): Promise<unknown>;

/**
* Retrieves an invitation by ID.
*/
abstract getInvitation(id: string): Promise<unknown>;
abstract getInvitation(
id: string,
headers?: Headers | Record<string, any>,
): Promise<Invitation | null>;

/**
* Accepts an invitation, creating a link between the user and the organization.
*/
abstract acceptInvitation(
invitationId: string,
inviterId: string,
userId: string,
headers?: Headers | Record<string, any>,
): Promise<unknown>;
Comment on lines 88 to 92

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.

⚠️ Potential issue | 🟡 Minor

Misleading parameter name: inviterId should be userId.

The second parameter represents the user accepting the invitation, not the inviter. The call site in auth.controller.ts passes user.id (the accepting user). Rename for clarity.

✏️ Suggested fix
  abstract acceptInvitation(
    invitationId: string,
-   inviterId: string,
+   userId: string,
    headers?: Headers | Record<string, any>,
  ): Promise<unknown>;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
abstract acceptInvitation(
invitationId: string,
inviterId: string,
headers?: Headers | Record<string, any>,
): Promise<unknown>;
abstract acceptInvitation(
invitationId: string,
userId: string,
headers?: Headers | Record<string, any>,
): Promise<unknown>;
🤖 Prompt for AI Agents
In `@apps/api/src/modules/auth/identity-provider.abstract.ts` around lines 91 -
95, The abstract method acceptInvitation has a misleading parameter name; rename
the second parameter from inviterId to userId in the acceptInvitation
declaration (method signature in identity-provider.abstract.ts) and update all
implementing classes, overrides, and call sites (e.g., where auth.controller.ts
calls acceptInvitation with user.id) to use the new userId name, preserving the
same type and behavior; also update any JSDoc/comments and exported
types/interfaces that reference inviterId to userId to keep signatures
consistent.


/**
Expand Down
Loading