Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
28 changes: 18 additions & 10 deletions apps/agent/agent/lib/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ export async function claimDue(
const onlyMode = "only" in kinds;

const claimed = await db.$queryRaw<LeasedTask[]>`
UPDATE "agentTask" AS t
SET "leasedUntil" = ${until},
"startedAt" = COALESCE(t."startedAt", ${now}),
"attempts" = t."attempts" + 1
FROM (
WITH due AS MATERIALIZED (
SELECT t2.id FROM "agentTask" AS t2
WHERE t2."finishedAt" IS NULL
AND t2."dueAt" <= ${now}
Expand All @@ -59,7 +55,12 @@ export async function claimDue(
ORDER BY t2."priority" DESC, t2."dueAt" ASC
LIMIT ${limit}
FOR UPDATE SKIP LOCKED
) AS due
)
UPDATE "agentTask" AS t
SET "leasedUntil" = ${until},
"startedAt" = COALESCE(t."startedAt", ${now}),
"attempts" = t."attempts" + 1
FROM due
WHERE t.id = due.id
RETURNING t.id, t."contactId", t."companyId", t."dealId", t.kind, t.reason, t.payload,
t.budget, t.attempts, t.priority, t."dueAt";
Expand All @@ -72,23 +73,30 @@ export async function claimDue(

export async function retireExhausted(
limit: number = DISPATCH.reconcile.retire,
kinds?: { only: readonly string[] },
): Promise<TaskSubject[]> {
const only = kinds?.only ?? [];
if (kinds && only.length === 0) return [];

const now = new Date();

return db.$queryRaw<TaskSubject[]>`
UPDATE "agentTask" AS t
SET "finishedAt" = ${now},
"outcome" = ${RETIRED_OUTCOME}
WHERE t.id IN (
WITH exhausted AS MATERIALIZED (
SELECT c.id
FROM "agentTask" AS c
WHERE c."finishedAt" IS NULL
AND c."attempts" >= ${MAX_ATTEMPTS}
AND (c."leasedUntil" IS NULL OR c."leasedUntil" < ${now})
AND (${only}::text[] = '{}' OR c.kind = ANY(${only}::text[]))
ORDER BY c."dueAt" ASC
LIMIT ${limit}
FOR UPDATE SKIP LOCKED
)
UPDATE "agentTask" AS t
SET "finishedAt" = ${now},
"outcome" = ${RETIRED_OUTCOME}
FROM exhausted
WHERE t.id = exhausted.id
RETURNING t.id, t."contactId", t."companyId", t."dealId", t.kind;
`;
}
Expand Down
69 changes: 62 additions & 7 deletions apps/agent/test/tasks.integration.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { db } from "@crm/db";
import { DIRECT_KINDS } from "@crm/db/agent-tasks";
import {
claimDue,
completeTask,
Expand All @@ -9,9 +8,14 @@ import {
scheduleTask,
} from "../agent/lib/tasks";

const kind = "test-lease";
const suffix = process.env.TEST_RUN_ID ?? "tasks-integration-spec";
const kind = `test-lease-${suffix}`;

const RESEARCH = { except: DIRECT_KINDS } as const;
const RESEARCH = { only: [kind] } as const;

function retire(limit?: number) {
return retireExhausted(limit, RESEARCH);
}

async function clear() {
await db.agentTask.deleteMany({ where: { kind } });
Expand Down Expand Up @@ -44,6 +48,13 @@ async function expire(taskId: string) {
});
}

async function exhaust() {
for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
const claimed = await claimDue(10, RESEARCH);
for (const task of claimed) await expire(task.id);
}
}

async function someone() {
return db.contact.create({
data: {
Expand Down Expand Up @@ -141,6 +152,27 @@ describe("claimDue", () => {

expect(await claimDue(10, RESEARCH)).toHaveLength(0);
});

it("gives each due row to one caller and no caller more than its limit", async () => {
const mine: string[] = [];
for (let row = 0; row < 8; row++) mine.push((await queue()).id);

const results = await Promise.all(
Array.from({ length: 4 }, () => claimDue(2, RESEARCH)),
);
for (const claimed of results) {
expect(claimed.length).toBeLessThanOrEqual(2);
}

const ids = results.flatMap((claimed) => claimed.map((task) => task.id));
expect(new Set(ids).size).toBe(ids.length);
for (const id of ids) expect(mine).toContain(id);

const leased = await db.agentTask.count({
where: { id: { in: mine }, attempts: 1, leasedUntil: { not: null } },
});
expect(leased).toBe(ids.length);
});
});

describe("retireExhausted", () => {
Expand All @@ -153,7 +185,7 @@ describe("retireExhausted", () => {
await expire(task.id);
}

const retired = await retireExhausted();
const retired = await retire();
expect(retired.map((t) => t.id)).toContain(task.id);
expect(retired.find((t) => t.id === task.id)?.contactId).toBe(contact.id);

Expand All @@ -170,14 +202,14 @@ describe("retireExhausted", () => {
if (attempt < MAX_ATTEMPTS - 1) await expire(task.id);
}

expect(await retireExhausted()).toHaveLength(0);
expect(await retire()).toHaveLength(0);
});

it("leaves work that still has attempts left", async () => {
await queue();
await claimDue(10, RESEARCH);

expect(await retireExhausted()).toHaveLength(0);
expect(await retire()).toHaveLength(0);
});

it("retires no more rows than the limit allows", async () => {
Expand All @@ -190,14 +222,37 @@ describe("retireExhausted", () => {
}

for (let pass = 0; pass < 3; pass++) {
expect((await retireExhausted(2)).length).toBeLessThanOrEqual(2);
expect((await retire(2)).length).toBeLessThanOrEqual(2);
}

const open = await db.agentTask.count({
where: { id: { in: mine }, finishedAt: null },
});
expect(open).toBe(0);
});

it("gives each exhausted row to one caller and no caller more than its limit", async () => {
const mine: string[] = [];
for (let row = 0; row < 8; row++) mine.push((await queue()).id);

await exhaust();

const results = await Promise.all(
Array.from({ length: 4 }, () => retire(2)),
);
for (const retired of results) {
expect(retired.length).toBeLessThanOrEqual(2);
}

const ids = results.flatMap((retired) => retired.map((task) => task.id));
expect(new Set(ids).size).toBe(ids.length);
for (const id of ids) expect(mine).toContain(id);

const open = await db.agentTask.count({
where: { id: { in: mine }, finishedAt: null },
});
expect(open).toBe(mine.length - ids.length);
});
});

describe("completeTask", () => {
Expand Down
1 change: 1 addition & 0 deletions apps/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"zod": "^4.4.3"
},
"devDependencies": {
"@better-auth/api-key": "1.6.25",
"@crm/typescript-config": "workspace:*",
"@nestjs/testing": "^11.0.1",
"@types/bun": "^1.3.14",
Expand Down
12 changes: 1 addition & 11 deletions apps/api/src/archive/archive-retention.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
ApiTags,
} from "@nestjs/swagger";
import { AllowAnonymous } from "@thallesp/nestjs-better-auth";
import { timingSafeEquals } from "../common/timing-safe-equal";
import { CompaniesService } from "../companies/companies.service";
import type { EnvironmentVariables } from "../config/env.validation";
import { ContactsService } from "../contacts/contacts.service";
Expand Down Expand Up @@ -103,14 +104,3 @@ export class ArchiveRetentionController {
return { retentionDays, companies, contacts, deals };
}
}

function timingSafeEquals(a: string, b: string): boolean {
if (a.length !== b.length) return false;

let mismatch = 0;
for (let index = 0; index < a.length; index += 1) {
mismatch |= a.charCodeAt(index) ^ b.charCodeAt(index);
}

return mismatch === 0;
}
10 changes: 10 additions & 0 deletions apps/api/src/common/timing-safe-equal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { timingSafeEqual } from "node:crypto";

export function timingSafeEquals(a: string, b: string): boolean {
const bufferA = Buffer.from(a);
const bufferB = Buffer.from(b);

if (bufferA.length !== bufferB.length) return false;

return timingSafeEqual(bufferA, bufferB);
}
6 changes: 6 additions & 0 deletions apps/api/src/config/env.validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Max,
Min,
MinLength,
ValidateIf,
validateSync,
} from "class-validator";

Expand Down Expand Up @@ -120,7 +121,12 @@ export class EnvironmentVariables {
AGENT_URL?: string;

@IsOptional()
@ValidateIf((_environment, value) => value !== "")
@IsString()
@MinLength(32, {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
message:
"AGENT_BRIDGE_SECRET must be at least 32 characters. Generate one with: openssl rand -base64 32",
})
AGENT_BRIDGE_SECRET?: string;

@IsOptional()
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/create-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export async function createApp(): Promise<NestExpressApplication> {

restBridge = createOpenApiExpressMiddleware({
router: appRouter,
createContext: ({ req }) => createBaseTrpcContext(req),
createContext: ({ req, res }) => createBaseTrpcContext(req, res),
});

return app;
Expand Down
12 changes: 1 addition & 11 deletions apps/api/src/currency/rates.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ApiTags,
} from "@nestjs/swagger";
import { AllowAnonymous } from "@thallesp/nestjs-better-auth";
import { timingSafeEquals } from "../common/timing-safe-equal";
import type { EnvironmentVariables } from "../config/env.validation";
import { ConversionService } from "./conversion.service";
import { RatesService } from "./rates.service";
Expand Down Expand Up @@ -81,14 +82,3 @@ export class RatesController {
return { ...refresh, converted: filled.converted, missing: filled.missing };
}
}

function timingSafeEquals(a: string, b: string): boolean {
if (a.length !== b.length) return false;

let mismatch = 0;
for (let index = 0; index < a.length; index += 1) {
mismatch |= a.charCodeAt(index) ^ b.charCodeAt(index);
}

return mismatch === 0;
}
2 changes: 1 addition & 1 deletion apps/api/src/logging/all-exceptions.filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class AllExceptionsFilter implements ExceptionFilter {
const payload = {
message: describe(exception),
method: request.method,
path: request.originalUrl,
path: request.path,
statusCode: status,
exception: exception?.constructor?.name,
};
Expand Down
4 changes: 3 additions & 1 deletion apps/api/src/logging/request-logger.middleware.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { randomUUID } from "node:crypto";
import { API_KEY_HEADER } from "@crm/auth";
import { Injectable, Logger, type NestMiddleware } from "@nestjs/common";
import type { NextFunction, Request, Response } from "express";
import { type RequestContext, runInRequestContext } from "./request-context";
Expand All @@ -18,7 +19,7 @@ export class RequestLoggerMiddleware implements NestMiddleware {
const context: RequestContext = {
requestId,
method: request.method,
path: request.originalUrl,
path: request.path,
};
const startedAt = process.hrtime.bigint();

Expand Down Expand Up @@ -56,6 +57,7 @@ export class RequestLoggerMiddleware implements NestMiddleware {
durationMs: Number(durationMs.toFixed(1)),
ip: request.ip,
userAgent: request.get("user-agent"),
apiKeyPresented: Boolean(request.get(API_KEY_HEADER)),

@cubic-dev-ai cubic-dev-ai Bot Sep 27, 2026 •

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.

P3: The hardening is untested: logging.spec.ts exercises only request-id behavior and never asserts the logged payload, so nothing guards the new query-string stripping (request.path vs request.originalUrl) or the apiKeyPresented field. Add a test that drives logCompleted output (capture the logger as in the existing spec) and asserts the payload's path excludes the query string and apiKeyPresented is true/false with/without x-api-key.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/logging/request-logger.middleware.ts, line 60:

<comment>The hardening is untested: `logging.spec.ts` exercises only request-id behavior and never asserts the logged payload, so nothing guards the new query-string stripping (`request.path` vs `request.originalUrl`) or the `apiKeyPresented` field. Add a test that drives `logCompleted` output (capture the logger as in the existing spec) and asserts the payload's `path` excludes the query string and `apiKeyPresented` is true/false with/without `x-api-key`.</comment>

<file context>
@@ -56,6 +57,7 @@ export class RequestLoggerMiddleware implements NestMiddleware {
 			durationMs: Number(durationMs.toFixed(1)),
 			ip: request.ip,
 			userAgent: request.get("user-agent"),
+			apiKeyPresented: Boolean(request.get(API_KEY_HEADER)),
 		};
 
</file context>
Fix with cubic

};

if (statusCode >= 500) {
Expand Down
12 changes: 1 addition & 11 deletions apps/api/src/sync/sync.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ApiTags,
} from "@nestjs/swagger";
import { AllowAnonymous } from "@thallesp/nestjs-better-auth";
import { timingSafeEquals } from "../common/timing-safe-equal";
import type { EnvironmentVariables } from "../config/env.validation";
import { MailboxSyncService } from "./mailbox-sync.service";

Expand Down Expand Up @@ -88,14 +89,3 @@ export class SyncController {
return this.sync.runDue();
}
}

function timingSafeEquals(a: string, b: string): boolean {
if (a.length !== b.length) return false;

let mismatch = 0;
for (let index = 0; index < a.length; index += 1) {
mismatch |= a.charCodeAt(index) ^ b.charCodeAt(index);
}

return mismatch === 0;
}
8 changes: 7 additions & 1 deletion apps/api/src/telemetry/rollup.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,13 @@ export class RollupService {
]);

return {
properties: { ...shape, ...agent, ...ledger, ...crm },
properties: {
...shape,
...agent,
...ledger,
...crm,
api_key_rate_limited: counters.api_key_rate_limited ?? 0,
},
counters,
};
}
Expand Down
12 changes: 1 addition & 11 deletions apps/api/src/telemetry/telemetry.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ApiTags,
} from "@nestjs/swagger";
import { AllowAnonymous } from "@thallesp/nestjs-better-auth";
import { timingSafeEquals } from "../common/timing-safe-equal";
import type { EnvironmentVariables } from "../config/env.validation";
import { RollupService } from "./rollup.service";

Expand Down Expand Up @@ -71,14 +72,3 @@ export class TelemetryController {
return this.rollup.run();
}
}

function timingSafeEquals(a: string, b: string): boolean {
if (a.length !== b.length) return false;

let mismatch = 0;
for (let index = 0; index < a.length; index += 1) {
mismatch |= a.charCodeAt(index) ^ b.charCodeAt(index);
}

return mismatch === 0;
}
Loading