test(server): load the company-skills route module graph once per file (#12426)

## Thinking Path

> - Paperclip is an open source app that helps people manage AI agents
for work.
> - The server provides company-scoped routes for company skills and
their test runs.
> - The authorization tests for these routes fail intermittently in
continuous integration.
> - The failure returns HTTP 500 instead of the expected HTTP 403.
> - The test file resets and rebuilds its module graph before each test.
> - This rebuild imports an unmocked issue service and raises a
TypeError.
> - This pull request loads the module graph once per describe block.
> - The change keeps the test result stable and preserves all 53 tests.

## Linked Issues or Issue Description

**What happened?**

The company-skill test-run authorization tests failed intermittently in
continuous integration. One test returned HTTP 500 instead of HTTP 403.

**Expected behavior**

Each unauthorized request must return HTTP 403. The test file must keep
all 53 tests and skip none.

**Steps to reproduce**

1. Run `npx vitest run
server/src/__tests__/company-skills-routes.test.ts` before this change.
2. Repeat the run in continuous integration.
3. Observe the intermittent HTTP 500 result in an authorization case.

**Paperclip version or commit**

Commit `651d26a96f6e24811d336759d3e67ff3abb5ec29`.

**Deployment mode**

Built from source. Continuous integration runs the test suite.

**Agent adapter(s) involved**

Not adapter-specific. This issue affects server test module setup.

**Database mode**

Not database-related.

## What Changed

- Load the mocked route module graph once for each describe block.
- Use the existing `hoistModuleGraph` helper, as the cost service test
does.
- Remove twelve per-test `vi.doUnmock` calls and the redundant module
rebuild.
- Keep the change in
`server/src/__tests__/company-skills-routes.test.ts` only.

## Verification

- Run `npx vitest run
server/src/__tests__/company-skills-routes.test.ts`.
- Confirm that 53 tests pass and 0 tests skip.
- Confirm that the diff changes only
`server/src/__tests__/company-skills-routes.test.ts`.
- Confirm that all Paperclip continuous integration checks pass.

## Risks

Low risk. The change affects test setup only. It does not change
production code, route behavior, or assertions.

## Model Used

OpenAI GPT-5 (Codex), exact model ID `gpt-5`, tool use and code
execution enabled. The runtime does not expose the context window size.

## 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 either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [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 (e.g. `docs/...`, `fix/...`)
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: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky LeachandPaperclip authored and GitHub committed 2026-08-28 08:09:30 -07:00
1 parent dbf052577d
commit 5f1e25c112
1 file changed
+22 -31
@@ -1,6 +1,7 @@
import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { hoistModuleGraph } from "./helpers/hoist-module-graph.js";
const mockAgentService = vi.hoisted(() => ({
getById: vi.fn(),
@@ -195,39 +196,29 @@ function registerModuleMocks() {
}));
}
async function createApp(actor: Record<string, unknown>) {
const [{ companySkillRoutes }, { errorHandler }] = await Promise.all([
vi.importActual<typeof import("../routes/company-skills.js")>("../routes/company-skills.js"),
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
]);
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = actor;
next();
});
app.use("/api", companySkillRoutes({} as any));
app.use(errorHandler);
return app;
}
describe("company skill mutation permissions", () => {
const routeModules = hoistModuleGraph(registerModuleMocks, async () => {
const [{ companySkillRoutes }, { errorHandler }] = await Promise.all([
vi.importActual<typeof import("../routes/company-skills.js")>("../routes/company-skills.js"),
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
]);
return { companySkillRoutes, errorHandler };
});
function createApp(actor: Record<string, unknown>) {
const { companySkillRoutes, errorHandler } = routeModules.value;
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = actor;
next();
});
app.use("/api", companySkillRoutes({} as any));
app.use(errorHandler);
return app;
}
beforeEach(() => {
vi.resetModules();
vi.doUnmock("@paperclipai/shared/telemetry");
vi.doUnmock("../telemetry.js");
vi.doUnmock("../services/access.js");
vi.doUnmock("../services/activity-log.js");
vi.doUnmock("../services/agents.js");
vi.doUnmock("../services/company-skills.js");
vi.doUnmock("../services/company-skill-policy.js");
vi.doUnmock("../services/skills-catalog.js");
vi.doUnmock("../services/change-consent-gate.js");
vi.doUnmock("../services/index.js");
vi.doUnmock("../routes/company-skills.js");
vi.doUnmock("../routes/authz.js");
vi.doUnmock("../middleware/index.js");
registerModuleMocks();
vi.clearAllMocks();
mockGetTelemetryClient.mockReturnValue({ track: vi.fn() });
mockCompanySkillService.importFromSource.mockResolvedValue({