From 3d6b48a4cd8225eeb000054d0f563d229cb0b411 Mon Sep 17 00:00:00 2001
From: Codeman maintainer <noreply@anthropic.com>
Date: Sat, 24 Oct 2026 13:53:13 +0200
Subject: [PATCH] fix: apply the review items for #683 at merge

findPersistedSessionOrFail now requires req (dropped the `?` and the dead `req &&` guard), like findSessionOrFail
The AST test in test/ownership-scoping.test.ts pins both declarations and asserts req's type is exactly FastifyRequest

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 src/web/route-helpers.ts       |  7 +++++--
 test/ownership-scoping.test.ts | 22 +++++++++++++---------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/web/route-helpers.ts b/src/web/route-helpers.ts
index 15966e7b..28f988b6 100644
--- a/src/web/route-helpers.ts
+++ b/src/web/route-helpers.ts
@@ -365,15 +365,18 @@ function lookupSessionOrFail(ctx: SessionPort, sessionId: string, req: FastifyRe
  * return, so this returns the persisted record instead. Same ownership
  * enforcement, same 404-not-403 leak protection — this is that function's
  * missing other half, not a separate check reimplemented inline.
+ *
+ * `req` is REQUIRED for the same reason as on `findSessionOrFail`: an optional
+ * one would let a caller that forgot it retire another user's row in multi-user mode.
  */
 export function findPersistedSessionOrFail(
   store: { getSession(id: string): SessionState | null },
   sessionId: string,
-  req?: FastifyRequest
+  req: FastifyRequest
 ): SessionState {
   const persisted = store.getSession(sessionId);
   if (!persisted) throw sessionNotFoundError(sessionId);
-  if (req && !canAccessOwned(getAuthUser(req), persisted.owner)) throw sessionNotFoundError(sessionId);
+  if (!canAccessOwned(getAuthUser(req), persisted.owner)) throw sessionNotFoundError(sessionId);
   return persisted;
 }
 
diff --git a/test/ownership-scoping.test.ts b/test/ownership-scoping.test.ts
index 62060c0b..dc1b3c31 100644
--- a/test/ownership-scoping.test.ts
+++ b/test/ownership-scoping.test.ts
@@ -166,18 +166,22 @@ describe('session-scoping helpers (multi-user)', () => {
     expect(() => findSessionUnscopedOrFail(ctxWith(map), 'missing')).toThrow();
   });
 
-  it('findSessionOrFail declares req as REQUIRED, so an unscoped call must use the explicit helper', async () => {
-    // tsc does not typecheck test/, so pin the signature from the source AST: an
+  it('findSessionOrFail and findPersistedSessionOrFail declare req as REQUIRED, so an unscoped call must use the explicit helper', async () => {
+    // tsc does not typecheck test/, so pin the signatures from the source AST: an
     // optional `req` would make forgetting it compile into an UNSCOPED lookup.
     const file = path.join(import.meta.dirname, '../src/web/route-helpers.ts');
     const src = ts.createSourceFile(file, await fs.readFile(file, 'utf8'), ts.ScriptTarget.Latest, true);
-    const decl = src.statements.find(
-      (s): s is ts.FunctionDeclaration => ts.isFunctionDeclaration(s) && s.name?.text === 'findSessionOrFail'
-    );
-    const req = decl?.parameters.find((p) => p.name.getText(src) === 'req');
-    expect(req).toBeDefined();
-    expect(req?.questionToken).toBeUndefined();
-    expect(req?.initializer).toBeUndefined();
+    for (const name of ['findSessionOrFail', 'findPersistedSessionOrFail']) {
+      const decl = src.statements.find(
+        (s): s is ts.FunctionDeclaration => ts.isFunctionDeclaration(s) && s.name?.text === name
+      );
+      const req = decl?.parameters.find((p) => p.name.getText(src) === 'req');
+      expect(req, name).toBeDefined();
+      expect(req?.questionToken, name).toBeUndefined();
+      expect(req?.initializer, name).toBeUndefined();
+      // Exactly `FastifyRequest`: a later `FastifyRequest | undefined` would be optional in all but syntax.
+      expect(req?.type?.getText(src), name).toBe('FastifyRequest');
+    }
   });
 
   it('per-user session cap counts only the owner sessions', () => {