[WIP] Fix onboarding integration tests: proper server teardown and JWT user alignment

[Copilot]

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

Create a pull request implementing fix A for PR #741.

Context:

• Failing job: 68384740045 in MightyPrytanis/codebase
• Ref/commit from logs: d9da985
• Failure involves onboarding integration tests in Cyrano/tests/routes/onboarding.test.ts.

Implement Fix A:

1. Fix test teardown bug in Cyrano/tests/routes/onboarding.test.ts:

   • Replace afterAll usage of undefined stopServer with proper server close logic.
   • Ensure server.close() is awaited and server is nulled.

2. Align tests with authentication-derived user identity:

   • The tests currently send arbitrary userId values in request body/query, but JWT-derived userId is used by routes.
   • Update tests to avoid mismatched user identities.
   • Create two JWT tokens/headers (userId=1 and userId=2) to support "existing user" and "new user" scenarios.
   • For "new user" tests, use authHeaders for userId=2 and remove/avoid userId=new-user query parameter.
   • For flows that currently use flowUserId strings, switch to numeric user IDs that correspond to the JWT or generate per-flow JWTs.

Acceptance criteria:

• npm run test:coverage should pass in CI environment.
• Onboarding integration tests should no longer fail with stopServer is not defined.
• Onboarding API Integration Tests should pass with consistent user identity handling.

Files to modify:

• Cyrano/tests/routes/onboarding.test.ts

Notes:

• Keep changes scoped to test fixes (do not change production route behavior).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for failing job 68384740045. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The job is failing for two concrete reasons shown in the logs:

1. A real runtime error in the test teardown

• Log: ReferenceError: stopServer is not defined at tests/routes/onboarding.test.ts:106:5
• In Cyrano/tests/routes/onboarding.test.ts, afterAll references stopServer, but no such function exists in the file.

Fix

Replace the afterAll block to correctly close the server created in beforeAll.

Patch (edit Cyrano/tests/routes/onboarding.test.ts around lines 105–109, ref d9da985b6b7ceb2d030567163c2830c4be7d05eb):

afterAll(async () => {
  if (server) {
    await new Promise<void>((resolve, reject) => {
      server!.close(err => (err ? reject(err) : resolve()));
    });
    server = null;
  }
});

This removes the undefined symbol and ensures the HTTP server is properly shut down.

---

2. Most onboarding integration tests are returning response.ok === false

• Multiple failures are expected false to be true at lines like:
  • expect(response.ok).toBe(true) (e.g., line 128, 147, 219, 283, 293, 312, 337, 362, 370, etc.)
• This indicates the endpoints are responding with non-2xx (commonly 401/403/500), not that the assertions themselves are wrong.

Most likely root cause from the test file

Your tests send userId in the body/query (e.g., test-onboarding-user, new-user, etc.), but the comment in the test says:

“All routes extract userId from the JWT … regardless of any userId value sent in the request body or query string.”

So the API likely keys onboarding state by the JWT userId (here always 1), while your tests expect behavior for arbitrary userId values like new-user and flow-test-*. Depending on the implementation, that mismatch can cause:

• authorization/validation failures (if userId must match token),
• or status/progress endpoints always returning the same user’s state (making “new-user” tests invalid),
• or DB constraints failing if userId type differs (string vs numeric).

Fix option A (recommended): Make tests consistent with “userId comes from JWT”

Update tests to stop sending arbitrary userId values and instead assert against the token user (userId=1). Concretely:

• Remove userId from request bodies and query strings where possible.
• For “new user” scenarios, generate a different JWT with a different numeric userId (e.g., 2) and use that token in headers for those requests.

Example adjustment pattern:

const tokenUser1 = generateAccessToken({ userId: 1, email: 'test@example.com', role: 'admin' });
const authHeadersUser1 = { Authorization: `Bearer ${tokenUser1}`, 'Content-Type': 'application/json' };

const tokenUser2 = generateAccessToken({ userId: 2, email: 'test2@example.com', role: 'admin' });
const authHeadersUser2 = { Authorization: `Bearer ${tokenUser2}`, 'Content-Type': 'application/json' };

Then in “new user” tests, call:

const response = await fetchWithRetry(`${baseUrl}/api/onboarding/status?appId=lexfiat`, { headers: authHeadersUser2 });
expect(response.ok).toBe(true);

And in bodies, omit userId (or...

This pull request was created from Copilot chat.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Copilot wasn't able to review any files in this pull request.