mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 20:46:28 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip runs the server and runner test suites with Vitest. > - Vitest 5 removes the deprecated `describe.sequential` property, so the pending Vitest 5 upgrade fails the type-check and test jobs. > - `describe.sequential` only changes behaviour inside a `describe.concurrent` suite, or when `sequence.concurrent` is on. > - This repository has neither, so the modifier changed nothing at run time. > - The benefit is that the Vitest 5 upgrade can land, and the test files lose a modifier that did no work. ## Linked Issues or Issue Description Refs: #12969 ## What Changed - Replace every `describe.sequential` use with a plain `describe` call. - Drop the `{ concurrent: false }` suite option from the two runner test files. - Add a comment to `server/vitest.config.ts` that records why these suites must run one test at a time. - Leave the package manifests and the lockfile unchanged. ## Why the modifier did nothing The Vitest documentation states that `describe.sequential` is useful to run tests in sequence inside a `describe.concurrent` suite, or with the `--sequence.concurrent` option. `sequence.concurrent` defaults to `false`. This repository satisfies neither condition: - No test file uses `describe.concurrent`, `it.concurrent`, or `test.concurrent`. - `server/vitest.config.ts` sets `sequence.concurrent: false`, with `maxWorkers: 1`, `maxConcurrency: 1`, and `isolate: true`. - `packages/paperclip-runner/vitest.config.ts` sets no `sequence` block, so the `false` default applies. `packages/db` and `cli` already run the same embedded-Postgres suites with a plain `describe`, and those jobs are green. The server package was the only outlier. The modifier did carry one real piece of knowledge: these suites need their tests to run one at a time. The new comment in `server/vitest.config.ts` records that reason next to the setting that enforces it. ## Verification - `git grep` for `describe.sequential` returns nothing outside `node_modules`. - The author ran the changed server test files under the installed Vitest 4, and the results match the results without this change. - Two very large embedded-Postgres test files exceeded the author's local memory limit, so the CI test jobs cover those two. - The two changed runner test files have pre-existing local failures caused by a missing Rust toolchain and a missing global `pnpm` binary. The failures are identical with and without this change. - The author type-checked the changed files and found no new error. - CI must pass the typecheck, build, server test, and runner verify jobs. ## Risks - Low risk. Suite execution stays serial, because the Vitest config enforces it. - The change adds no dependency and changes no package manifest or lockfile. - A future change that turns `sequence.concurrent` on would break these suites. The new config comment warns against it. ## Model Used - Claude Sonnet 5 — code edits and local verification. - OpenAI Codex, GPT-5 — the earlier revision of this branch. ## Test plan - [x] Every CI check reaches a terminal green state. A pending or queued check is not a pass. - [x] The `Typecheck + Release Registry` job passes. This change must not introduce a type error. - [x] The `Build` job passes. - [x] The server test jobs and the runner verify jobs pass. - [x] Greptile re-reviews this commit set and posts a passing verdict. The dependabot waiver does not apply to this pull request. - [x] `mergeable` reads `MERGEABLE` as a terminal value. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have linked the related public issue with `Refs: #12969` - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-Authored-By: Priya Raman <priya.raman@paperclip.ing> --------- Co-authored-by: Priya Raman <priya.raman@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: nickyleach <331803+nickyleach@users.noreply.github.com>
328 lines
9.1 KiB
TypeScript
328 lines
9.1 KiB
TypeScript
import express from "express";
|
|
import request from "supertest";
|
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import { errorHandler } from "../middleware/index.js";
|
|
import { authRoutes } from "../routes/auth.js";
|
|
|
|
function createSelectChain(rows: unknown[]) {
|
|
return {
|
|
from() {
|
|
return {
|
|
where() {
|
|
return Promise.resolve(rows);
|
|
},
|
|
};
|
|
},
|
|
};
|
|
}
|
|
|
|
function createUpdateChain(row: unknown) {
|
|
return {
|
|
set(values: unknown) {
|
|
return {
|
|
where() {
|
|
return {
|
|
returning() {
|
|
return Promise.resolve([{ ...(row as Record<string, unknown>), ...(values as Record<string, unknown>) }]);
|
|
},
|
|
};
|
|
},
|
|
};
|
|
},
|
|
};
|
|
}
|
|
|
|
function createDb(row: Record<string, unknown>) {
|
|
return {
|
|
select: () => createSelectChain([row]),
|
|
update: () => createUpdateChain(row),
|
|
} as any;
|
|
}
|
|
|
|
function createApp(actor: Express.Request["actor"], row: Record<string, unknown>) {
|
|
const app = express();
|
|
app.use(express.json());
|
|
app.use((req, _res, next) => {
|
|
req.actor = actor;
|
|
next();
|
|
});
|
|
app.use("/api/auth", authRoutes(createDb(row)));
|
|
app.use(errorHandler);
|
|
return app;
|
|
}
|
|
|
|
describe("auth routes", () => {
|
|
const baseUser = {
|
|
id: "user-1",
|
|
name: "Jane Example",
|
|
email: "jane@example.com",
|
|
image: "https://example.com/jane.png",
|
|
};
|
|
const originalSentryDsn = process.env.SENTRY_DSN;
|
|
const originalSentryDsnFrontend = process.env.SENTRY_DSN_FRONTEND;
|
|
const originalSentryDsnBackend = process.env.SENTRY_DSN_BACKEND;
|
|
|
|
afterEach(() => {
|
|
vi.unstubAllEnvs();
|
|
if (originalSentryDsn === undefined) delete process.env.SENTRY_DSN;
|
|
else process.env.SENTRY_DSN = originalSentryDsn;
|
|
if (originalSentryDsnFrontend === undefined) delete process.env.SENTRY_DSN_FRONTEND;
|
|
else process.env.SENTRY_DSN_FRONTEND = originalSentryDsnFrontend;
|
|
if (originalSentryDsnBackend === undefined) delete process.env.SENTRY_DSN_BACKEND;
|
|
else process.env.SENTRY_DSN_BACKEND = originalSentryDsnBackend;
|
|
});
|
|
|
|
it("returns the persisted user profile in the session payload", async () => {
|
|
vi.stubEnv("SENTRY_ENVIRONMENT", undefined);
|
|
delete process.env.SENTRY_DSN;
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toEqual({
|
|
session: {
|
|
id: "paperclip:session:user-1",
|
|
userId: "user-1",
|
|
},
|
|
user: baseUser,
|
|
sentryDsn: null,
|
|
sentryEnvironment: null,
|
|
});
|
|
});
|
|
|
|
it("sends sentryDsn for a board actor when SENTRY_DSN is set", async () => {
|
|
process.env.SENTRY_DSN = "https://public@o0.ingest.sentry.io/1";
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryDsn).toBe("https://public@o0.ingest.sentry.io/1");
|
|
});
|
|
|
|
it("sends a null sentryDsn when SENTRY_DSN is unset", async () => {
|
|
delete process.env.SENTRY_DSN;
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryDsn).toBe(null);
|
|
});
|
|
|
|
it("sends sentryDsn for a board actor when only SENTRY_DSN_FRONTEND is set", async () => {
|
|
delete process.env.SENTRY_DSN;
|
|
delete process.env.SENTRY_DSN_BACKEND;
|
|
process.env.SENTRY_DSN_FRONTEND = "https://public-frontend@o0.ingest.sentry.io/1";
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryDsn).toBe("https://public-frontend@o0.ingest.sentry.io/1");
|
|
});
|
|
|
|
it("sends a null sentryDsn when only SENTRY_DSN_BACKEND is set", async () => {
|
|
delete process.env.SENTRY_DSN;
|
|
delete process.env.SENTRY_DSN_FRONTEND;
|
|
process.env.SENTRY_DSN_BACKEND = "https://public-backend@o0.ingest.sentry.io/2";
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryDsn).toBe(null);
|
|
});
|
|
|
|
it("sends only the front-end DSN when SENTRY_DSN_FRONTEND and SENTRY_DSN_BACKEND differ", async () => {
|
|
delete process.env.SENTRY_DSN;
|
|
process.env.SENTRY_DSN_FRONTEND = "https://public-frontend@o0.ingest.sentry.io/1";
|
|
process.env.SENTRY_DSN_BACKEND = "https://public-backend@o0.ingest.sentry.io/2";
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryDsn).toBe("https://public-frontend@o0.ingest.sentry.io/1");
|
|
expect(res.body.sentryDsn).not.toBe("https://public-backend@o0.ingest.sentry.io/2");
|
|
});
|
|
|
|
it("answers 401 and sends no DSN when the actor type is none", async () => {
|
|
process.env.SENTRY_DSN = "https://public@o0.ingest.sentry.io/1";
|
|
const app = await createApp(
|
|
{
|
|
type: "none",
|
|
source: "none",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(401);
|
|
expect(res.body.sentryDsn).toBeUndefined();
|
|
});
|
|
|
|
it.each(["staging", "production", "preview"])(
|
|
"sends the configured Sentry environment %s to board actors",
|
|
async (environment) => {
|
|
vi.stubEnv("SENTRY_ENVIRONMENT", environment);
|
|
const app = createApp({ type: "board", userId: "user-1", source: "session" }, baseUser);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryEnvironment).toBe(environment);
|
|
},
|
|
);
|
|
|
|
it.each([undefined, ""])("sends null for an unset Sentry environment (%s)", async (environment) => {
|
|
vi.stubEnv("SENTRY_ENVIRONMENT", environment);
|
|
const app = createApp({ type: "board", userId: "user-1", source: "session" }, baseUser);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.sentryEnvironment).toBeNull();
|
|
});
|
|
|
|
it.each([
|
|
{ type: "none", source: "none" },
|
|
{ type: "agent", agentId: "agent-1", companyId: "company-1", source: "agent_key" },
|
|
] satisfies Express.Request["actor"][])("withholds Sentry settings from a $type actor", async (actor) => {
|
|
vi.stubEnv("SENTRY_ENVIRONMENT", "staging");
|
|
vi.stubEnv("SENTRY_DSN_FRONTEND", "https://public@o0.ingest.sentry.io/1");
|
|
const app = createApp(actor, baseUser);
|
|
|
|
const res = await request(app).get("/api/auth/get-session");
|
|
|
|
expect(res.status).toBe(401);
|
|
expect(res.body.sentryDsn).toBeUndefined();
|
|
expect(res.body.sentryEnvironment).toBeUndefined();
|
|
});
|
|
|
|
it("updates the signed-in profile", async () => {
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "local_implicit",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app)
|
|
.patch("/api/auth/profile")
|
|
.send({ name: "Board Operator", image: "" });
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toMatchObject({
|
|
id: "user-1",
|
|
name: "Board Operator",
|
|
email: "jane@example.com",
|
|
image: null,
|
|
});
|
|
});
|
|
|
|
it("preserves the existing avatar when updating only the profile name", async () => {
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "local_implicit",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app)
|
|
.patch("/api/auth/profile")
|
|
.send({ name: "Board Operator" });
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toMatchObject({
|
|
id: "user-1",
|
|
name: "Board Operator",
|
|
email: "jane@example.com",
|
|
image: "https://example.com/jane.png",
|
|
});
|
|
});
|
|
|
|
it("accepts Paperclip asset paths for avatars", async () => {
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app)
|
|
.patch("/api/auth/profile")
|
|
.send({ name: "Jane Example", image: "/api/assets/asset-1/content" });
|
|
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.image).toBe("/api/assets/asset-1/content");
|
|
});
|
|
|
|
it("rejects invalid avatar image references", async () => {
|
|
const app = await createApp(
|
|
{
|
|
type: "board",
|
|
userId: "user-1",
|
|
source: "session",
|
|
},
|
|
baseUser,
|
|
);
|
|
|
|
const res = await request(app)
|
|
.patch("/api/auth/profile")
|
|
.send({ name: "Jane Example", image: "not-a-url" });
|
|
|
|
expect(res.status).toBe(400);
|
|
});
|
|
});
|