chore(server): lint rule refusing a Promise used as a plain value, and the misses it found

server/eslint-rules/fixtures/file.ts

@@ -0,0 +1 @@
+export {};

server/eslint-rules/fixtures/strict.ts

@@ -0,0 +1 @@
+export {};

server/eslint-rules/fixtures/tsconfig.json

@@ -0,0 +1,16 @@
+{
+  // Fixture project for the RuleTester in tests/unit/eslint-rules/. It exists so
+  // typescript-eslint can build a real program for the virtual `file.ts` the
+  // tester feeds it. Kept outside src/ and tests/ on purpose: files here must not
+  // reach `tsc --noEmit`, `tsc -p tsconfig.tests.json` or the type-aware lint
+  // blocks, since the invalid cases are deliberately broken code.
+  "compilerOptions": {
+    "target": "ES2022",
+    "module": "commonjs",
+    "lib": ["ES2022"],
+    "strict": false,
+    "noEmit": true,
+    "skipLibCheck": true
+  },
+  "include": ["file.ts"]
+}

server/eslint-rules/fixtures/tsconfig.strict.json

@@ -0,0 +1,15 @@
+{
+  // Second fixture project, identical but with strictNullChecks ON. The server
+  // compiles with `strict: false`, where `Promise<T> | null` collapses to
+  // `Promise<T>` and the rule's nullable exemptions are unreachable — this one
+  // exists so those branches (`p ?? x`, `p === null`) are actually exercised.
+  "compilerOptions": {
+    "target": "ES2022",
+    "module": "commonjs",
+    "lib": ["ES2022"],
+    "strict": true,
+    "noEmit": true,
+    "skipLibCheck": true
+  },
+  "include": ["strict.ts"]
+}

server/eslint-rules/no-promise-as-value.mjs

@@ -0,0 +1,241 @@
+/**
+ * trek/no-promise-as-value — the blind spot the compiler cannot see.
+ *
+ * Phase 1 of the ORM migration turns hundreds of DB-touching methods `async`.
+ * TypeScript catches a missed `await` only when the promise is used in a way it
+ * can type-check: `promise.foo` (TS2339), a floating statement
+ * (no-floating-promises), a condition (no-misused-promises). It catches NOTHING
+ * when the promise is used as a plain value, because this project compiles with
+ * `strict: false` / `noImplicitAny: false`:
+ *
+ *     settings['metric']            // element access on a promise -> `any`
+ *     if (svc.get(id) === 'x')      // always false, no error
+ *     `${svc.count(id)}`            // "[object Promise]"
+ *     Object.keys(svc.all())        // []
+ *     JSON.stringify(svc.row(id))   // "{}"
+ *     for (const r of svc.rows())   // not iterable, at runtime only
+ *
+ * This rule reports those shapes. It is DELIBERATELY CONSERVATIVE and has no
+ * fixer: it fires only when the resolved type is the global `Promise`/
+ * `PromiseLike` itself (never a merely-thenable library type such as MikroORM's
+ * QueryBuilder or supertest's Test, both of which are chained on purpose), only
+ * when every non-nullish member of a union is one, and never inside
+ * `Promise.all`/`allSettled`/`race`/`any`, on `.then`/`.catch`/`.finally`, on
+ * `expect(p).resolves`/`.rejects`, or in an existence/identity check
+ * (`p === undefined`, `p === null`, `p ?? q`, `a === b` between two promises) —
+ * see isExistenceOrIdentityCheck below. A finding is a bug: add the `await`.
+ */
+import ts from 'typescript';
+
+const NULLISH_FLAGS = ts.TypeFlags.Null | ts.TypeFlags.Undefined | ts.TypeFlags.Void;
+const PASSTHROUGH_PROPS = new Set(['then', 'catch', 'finally', 'resolves', 'rejects']);
+const COMBINATORS = new Set(['all', 'allSettled', 'race', 'any']);
+const EQUALITY_OPERATORS = new Set(['==', '!=', '===', '!==']);
+// Callbacks whose result is consumed as a value, where a promise is always a bug
+// (every promise is truthy, so the predicate degenerates). `.map`/`.reduce` are
+// excluded on purpose: returning promises there feeds Promise.all legitimately.
+const PREDICATE_METHODS = new Set([
+  'filter',
+  'find',
+  'findLast',
+  'findIndex',
+  'findLastIndex',
+  'some',
+  'every',
+  'sort',
+]);
+const OBJECT_CONSUMERS = new Set(['keys', 'values', 'entries', 'assign', 'fromEntries']);
+
+/** `String(p)` / `Number(p)` / `Object.keys(p)` / `JSON.stringify(p)` / `Array.isArray(p)`. */
+function isValueConsumerCall(callee) {
+  if (callee.type === 'Identifier') return callee.name === 'String' || callee.name === 'Number';
+  if (callee.type !== 'MemberExpression' || callee.computed) return false;
+  const { object, property } = callee;
+  if (object.type !== 'Identifier' || property.type !== 'Identifier') return false;
+  if (object.name === 'Object') return OBJECT_CONSUMERS.has(property.name);
+  if (object.name === 'JSON') return property.name === 'stringify';
+  if (object.name === 'Array') return property.name === 'isArray';
+  return false;
+}
+
+function isCombinatorCall(node) {
+  const callee = node.callee;
+  return (
+    callee.type === 'MemberExpression' &&
+    !callee.computed &&
+    callee.object.type === 'Identifier' &&
+    callee.object.name === 'Promise' &&
+    callee.property.type === 'Identifier' &&
+    COMBINATORS.has(callee.property.name)
+  );
+}
+
+/** Inside `Promise.all([...])` a promise IS the value; stop at the function boundary. */
+function isInsideCombinator(node) {
+  for (let cur = node.parent; cur; cur = cur.parent) {
+    if (cur.type === 'CallExpression' && isCombinatorCall(cur)) return true;
+    if (cur.type.includes('Function')) return false;
+  }
+  return false;
+}
+
+function isNullLiteral(node) {
+  return (node.type === 'Literal' && node.value === null) || (node.type === 'Identifier' && node.name === 'undefined');
+}
+
+export default {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Disallow using a Promise as a plain value (element access, comparison, interpolation, spread, iteration) — the missing-await shapes the compiler cannot see with strict mode off.',
+    },
+    schema: [],
+    messages: {
+      promiseAsValue: 'Promise used as a value (missing await?): {{ text }}',
+    },
+  },
+
+  create(context) {
+    const services = context.sourceCode.parserServices;
+    if (!services?.program || !services.esTreeNodeToTSNodeMap) return {};
+    const checker = services.program.getTypeChecker();
+    const reported = new WeakSet();
+
+    const isNullish = (type) => (type.flags & NULLISH_FLAGS) !== 0;
+
+    function isPromiseType(type, tsNode) {
+      if (!type) return false;
+      if (type.isUnion()) {
+        const parts = type.types.filter((t) => !isNullish(t));
+        return parts.length > 0 && parts.every((t) => isPromiseType(t, tsNode));
+      }
+      const symbol = type.getSymbol() ?? type.aliasSymbol;
+      const name = symbol?.getName();
+      if (name !== 'Promise' && name !== 'PromiseLike') return false;
+      const then = type.getProperty('then');
+      if (!then) return false;
+      return checker.getTypeOfSymbolAtLocation(then, tsNode).getCallSignatures().length > 0;
+    }
+
+    function typeOf(node) {
+      if (!node || node.type === 'AwaitExpression' || node.type === 'PrivateIdentifier') return null;
+      const tsNode = services.esTreeNodeToTSNodeMap.get(node);
+      return tsNode ? { type: checker.getTypeAtLocation(tsNode), tsNode } : null;
+    }
+
+    function isPromise(node) {
+      const resolved = typeOf(node);
+      return !!resolved && isPromiseType(resolved.type, resolved.tsNode);
+    }
+
+    function report(node) {
+      if (!node || reported.has(node) || isInsideCombinator(node)) return;
+      reported.add(node);
+      const raw = context.sourceCode.getText(node).replace(/\s+/g, ' ');
+      const text = raw.length > 60 ? `${raw.slice(0, 57)}...` : raw;
+      context.report({ node, messageId: 'promiseAsValue', data: { text } });
+    }
+
+    function check(node) {
+      if (isPromise(node)) report(node);
+    }
+
+    /**
+     * `p === undefined` / `p === null` is how this codebase asks whether an
+     * in-flight promise is already parked in a map — a truthiness test there is
+     * what no-misused-promises rejects, so the explicit comparison IS the
+     * correct idiom (weather, collab, maps, exchange-rates, idempotency, the
+     * plugin drain, atlas). `chains.get(userId) === next` likewise asks whether
+     * the promise it stored is still the current one. Neither can be told apart
+     * from a missing await by type alone — with strictNullChecks off `Map.get()`
+     * is not even typed as nullable — so equality against nullish, and equality
+     * between two promises, are both exempt. `p === 'x'` still reports.
+     */
+    function isExistenceOrIdentityCheck(node) {
+      if (isNullLiteral(node.left) || isNullLiteral(node.right)) return true;
+      return isPromise(node.left) && isPromise(node.right);
+    }
+
+    return {
+      MemberExpression(node) {
+        const key =
+          node.property.type === 'Identifier' && !node.computed
+            ? node.property.name
+            : node.property.type === 'Literal'
+              ? node.property.value
+              : undefined;
+        if (typeof key === 'string' && PASSTHROUGH_PROPS.has(key)) return;
+        check(node.object);
+      },
+
+      BinaryExpression(node) {
+        // `p instanceof Promise` is the legitimate way to ask.
+        if (node.operator === 'instanceof') return;
+        if (EQUALITY_OPERATORS.has(node.operator) && isExistenceOrIdentityCheck(node)) return;
+        check(node.left);
+        check(node.right);
+      },
+
+      LogicalExpression(node) {
+        // `p ?? x` is the same existence check as `p === undefined`, and with
+        // strictNullChecks off the type cannot say whether it is a real one.
+        if (node.operator === '??') return;
+        // Only the left operand of `&&`/`||`: it is the one evaluated as a test,
+        // where a promise is unconditionally truthy. The right operand IS the
+        // expression's value and gets caught at whatever consumes it.
+        check(node.left);
+      },
+
+      TemplateLiteral(node) {
+        node.expressions.forEach(check);
+      },
+
+      SpreadElement(node) {
+        check(node.argument);
+      },
+
+      UnaryExpression(node) {
+        if (node.operator === 'typeof') check(node.argument);
+      },
+
+      ForOfStatement(node) {
+        if (!node.await) check(node.right);
+      },
+
+      ForInStatement(node) {
+        check(node.right);
+      },
+
+      CallExpression(node) {
+        if (isValueConsumerCall(node.callee)) {
+          node.arguments.forEach((arg) => {
+            if (arg.type !== 'SpreadElement') check(arg);
+          });
+          return;
+        }
+        const callee = node.callee;
+        if (
+          callee.type !== 'MemberExpression' ||
+          callee.computed ||
+          callee.property.type !== 'Identifier' ||
+          !PREDICATE_METHODS.has(callee.property.name)
+        ) {
+          return;
+        }
+        for (const arg of node.arguments) {
+          if (arg.type !== 'ArrowFunctionExpression' && arg.type !== 'FunctionExpression') continue;
+          const resolved = typeOf(arg);
+          if (!resolved) continue;
+          const signatures = resolved.type.getCallSignatures();
+          if (
+            signatures.length > 0 &&
+            signatures.every((sig) => isPromiseType(checker.getReturnTypeOfSignature(sig), resolved.tsNode))
+          ) {
+            report(arg.body.type === 'BlockStatement' ? arg : arg.body);
+          }
+        }
+      },
+    };
+  },
+};

server/eslint.config.mjs

@@ -4,6 +4,11 @@ import gitignore from 'eslint-config-flat-gitignore';
 import eslintConfigPrettier from 'eslint-config-prettier';
 import tseslint from 'typescript-eslint';
 
+import noPromiseAsValue from './eslint-rules/no-promise-as-value.mjs';
+
+// Local rules live in eslint-rules/ and are exposed under the `trek/` prefix.
+const trek = { rules: { 'no-promise-as-value': noPromiseAsValue } };
+
 export default tseslint.config(
   gitignore({ strict: false }),
   {
@@ -56,7 +61,14 @@ export default tseslint.config(
   },
   {
     files: ['src/**/*.ts', 'tests/**/*.ts'],
+    plugins: { trek },
     rules: {
+      // trek/no-promise-as-value closes the hole the two rules below leave open:
+      // with `strict: false`, a promise used as a plain VALUE (element access,
+      // comparison, interpolation, spread, iteration, Object.keys/JSON.stringify)
+      // type-checks clean and silently produces `any`, `false` or `[object
+      // Promise]`. See eslint-rules/no-promise-as-value.mjs.
+      'trek/no-promise-as-value': 'error',
       // --- The promise safety net for the Phase 1 async sweep (ORM migration) ---
       // Turning a DB-touching method `async` makes every unawaited call site a
       // silently reordered write, and every `if (svc.exists(id))` permanently

server/tests/unit/eslint-rules/no-promise-as-value.test.ts

@@ -0,0 +1,134 @@
+import noPromiseAsValue from '../../../eslint-rules/no-promise-as-value.mjs';
+import tsParser from '@typescript-eslint/parser';
+
+import { RuleTester, type Rule } from 'eslint';
+import path from 'path';
+
+// The rule only exists because it is TYPE-aware, so the tester runs against a
+// real TypeScript program: eslint-rules/fixtures/ holds a tsconfig whose single
+// file is the one the tester feeds code into. It lives outside src/ and tests/
+// so the deliberately-broken snippets below never reach tsc or the lint gates.
+const fixtureRoot = path.resolve(__dirname, '../../../eslint-rules/fixtures');
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parser: tsParser,
+    parserOptions: {
+      project: './tsconfig.json',
+      tsconfigRootDir: fixtureRoot,
+    },
+  },
+});
+
+const PRELUDE = `
+declare function getSettings(id: number): Promise<Record<string, string>>;
+declare function getName(id: number): Promise<string>;
+declare function getCount(): Promise<number>;
+declare function getRows(): Promise<string[]>;
+declare function maybeSettings(id: number): Promise<Record<string, string>> | null;
+declare function syncName(id: number): string;
+declare const rows: { id: number; active: boolean }[];
+declare const expect: (value: unknown) => { resolves: { toBe(value: unknown): void } };
+`;
+
+/** Every case shares the declarations above; offsets never matter, only counts. */
+const withPrelude = (code: string) => `${PRELUDE}${code}\n`;
+
+const valid = [
+  // Awaited first — the shape every finding is fixed into.
+  `const s = await getSettings(1); const v = s['metric']; void v;`,
+  `const hit = (await getName(1)) === 'x'; void hit;`,
+  `const msg = \`\${await getCount()}\`; void msg;`,
+  `const all = [...(await getRows())]; void all;`,
+  `for (const r of await getRows()) { void r; }`,
+  `const keys = Object.keys(await getSettings(1)); void keys;`,
+  // Promise-shaped uses that are correct by construction.
+  `const n = getName(1).then((x) => x.length); void n;`,
+  `const both = await Promise.all([getName(1), getCount()]); void both;`,
+  `const joined = \`\${(await Promise.all([getName(1)]))[0]}\`; void joined;`,
+  `expect(getName(1)).resolves.toBe('x');`,
+  `const isP = getName(1) instanceof Promise; void isP;`,
+  // Existence / identity checks on a parked promise: the in-flight-map idiom the
+  // server uses everywhere, and the one a truthiness test would get wrong.
+  `const inflight = new Map<string, Promise<string>>(); const hit = inflight.get('k'); if (hit !== undefined) { void hit; }`,
+  `let building: Promise<string> = getName(1); if (building === null) { void 0; }`,
+  `const chains = new Map<number, Promise<void>>(); const next = Promise.resolve(); if (chains.get(1) === next) { void 0; }`,
+  `const chains = new Map<number, Promise<void>>(); const prev = chains.get(1) ?? Promise.resolve(); void prev;`,
+  `void getName(1);`,
+  // Non-promise values, and the array callbacks a promise legitimately flows through.
+  `const same = syncName(1) === 'x'; void same;`,
+  `const active = rows.filter((r) => r.active); void active;`,
+  `const pending = rows.map((r) => getName(r.id)); void pending;`,
+].map((code) => ({ code: withPrelude(code), filename: path.join(fixtureRoot, 'file.ts') }));
+
+const invalid = [
+  // Element access: compiles to `any` with strict mode off — the live miss Task 2 found.
+  `const v = getSettings(1)['metric']; void v;`,
+  // Property access on a promise-typed object, bracket-free.
+  `const v = getSettings(1).metric; void v;`,
+  // Comparisons: always false, no error.
+  `if (getName(1) === 'x') { void 0; }`,
+  `const n = getCount() + 1; void n;`,
+  `const has = 'metric' in getSettings(1); void has;`,
+  // Interpolation: "[object Promise]".
+  `const msg = \`\${getCount()}\`; void msg;`,
+  // Spread: neither iterable nor enumerable at runtime.
+  `const a = [...getRows()]; void a;`,
+  `const o = { ...getSettings(1) }; void o;`,
+  // Plain-value consumers.
+  `Object.keys(getSettings(1));`,
+  `JSON.stringify(getSettings(1));`,
+  `Array.isArray(getRows());`,
+  `const t = typeof getName(1); void t;`,
+  // Iteration.
+  `for (const r of getRows()) { void r; }`,
+  `for (const k in getSettings(1)) { void k; }`,
+  // Logical left operand: always truthy, so the fallback is dead code.
+  `const v = getName(1) || 'x'; void v;`,
+  // A predicate callback that returns a promise: every element passes.
+  `const active = rows.filter((r) => getName(r.id)); void active;`,
+].map((code) => ({
+  code: withPrelude(code),
+  filename: path.join(fixtureRoot, 'file.ts'),
+  errors: [{ messageId: 'promiseAsValue' }],
+}));
+
+ruleTester.run('trek/no-promise-as-value', noPromiseAsValue as unknown as Rule.RuleModule, {
+  valid,
+  invalid,
+});
+
+// Under strictNullChecks a promise-returning call can genuinely be nullable, and
+// `p ?? x` / `p === null` are then real checks rather than missing awaits. The
+// server compiles with `strict: false`, where that union collapses and these
+// branches are unreachable — so they get their own project to be tested in.
+const strictTester = new RuleTester({
+  languageOptions: {
+    parser: tsParser,
+    parserOptions: { project: './tsconfig.strict.json', tsconfigRootDir: fixtureRoot },
+  },
+});
+
+const strictFile = path.join(fixtureRoot, 'strict.ts');
+
+strictTester.run('trek/no-promise-as-value (strictNullChecks)', noPromiseAsValue as unknown as Rule.RuleModule, {
+  valid: [
+    { code: withPrelude(`const f = maybeSettings(1) ?? null; void f;`), filename: strictFile },
+    { code: withPrelude(`const missing = maybeSettings(1) === null; void missing;`), filename: strictFile },
+  ],
+  invalid: [
+    {
+      // Nullable or not, indexing it is still a missing await.
+      code: withPrelude(`const v = maybeSettings(1)!['metric']; void v;`),
+      filename: strictFile,
+      errors: [{ messageId: 'promiseAsValue' }],
+    },
+    {
+      // A union of `Promise<T> | null` is still a promise once the nullish
+      // members are stripped — interpolating it is "[object Promise]".
+      code: withPrelude(`const msg = \`\${maybeSettings(1)}\`; void msg;`),
+      filename: strictFile,
+      errors: [{ messageId: 'promiseAsValue' }],
+    },
+  ],
+});

server/tests/unit/plugins/plugin-instance-settings.test.ts

@@ -362,8 +362,8 @@ describe('instance-scope actions (admin)', () => {
       .run(pluginId, key, key, scope);
   }
   const adminReq = { user: { id: 42 } } as unknown as Request;
-  function controller(invoke = vi.fn(async () => ({ ok: true, message: 'pong' }))) {
-    const rt = createPluginRuntime(new DatabaseService(dbConn));
+  async function controller(invoke = vi.fn(async () => ({ ok: true, message: 'pong' }))) {
+    const rt = await createPluginRuntime(new DatabaseService(dbConn));
     // isActive normally reflects the supervisor's live child map, which nothing here
     // spawns — so it's stubbed to read the same DB status the test itself flips,
     // mirroring what an actually-activated plugin would report.
@@ -375,25 +375,25 @@ describe('instance-scope actions (admin)', () => {
 
   beforeEach(() => { testDb.prepare('DELETE FROM plugin_actions').run(); });
 
-  it('ACT-ADM-001 — GET config lists the instance actions and none of the user ones', () => {
+  it('ACT-ADM-001 — GET config lists the instance actions and none of the user ones', async () => {
     install('p');
     declareAction('p', 'purge', 'instance');
     declareAction('p', 'testConnection', 'user');
-    const { c } = controller();
+    const { c } = await controller();
     expect(c.getConfig('p').actions).toEqual([{ key: 'purge', label: 'purge', hint: undefined, danger: false, scope: 'instance' }]);
   });
 
   it('ACT-ADM-002 — POST runs the action as the clicking admin in the instance scope', async () => {
     install('p');
     testDb.prepare("UPDATE plugins SET status = 'active', enabled = 1 WHERE id = 'p'").run();
-    const { c, invoke } = controller();
+    const { c, invoke } = await controller();
     expect(await c.runAction('p', 'purge', adminReq)).toEqual({ ok: true, message: 'pong' });
     expect(invoke).toHaveBeenCalledWith('p', 'purge', 42, 'instance');
   });
 
   it('ACT-ADM-003 — an inactive plugin answers 404 like the user route', async () => {
     install('p');
-    const { c, invoke } = controller();
+    const { c, invoke } = await controller();
     await expect(c.runAction('p', 'purge', adminReq)).rejects.toMatchObject({ status: 404, response: { error: 'Plugin is not active' } });
     expect(invoke).not.toHaveBeenCalled();
   });
@@ -401,7 +401,7 @@ describe('instance-scope actions (admin)', () => {
   it('ACT-ADM-004 — a refused key is a failed RESULT, not a server error', async () => {
     install('p');
     testDb.prepare("UPDATE plugins SET status = 'active', enabled = 1 WHERE id = 'p'").run();
-    const { c } = controller(vi.fn(async () => { throw new Error('plugin p did not declare action "x" in scope instance'); }));
+    const { c } = await controller(vi.fn(async () => { throw new Error('plugin p did not declare action "x" in scope instance'); }));
     expect(await c.runAction('p', 'x', adminReq)).toEqual({ ok: false, message: 'plugin p did not declare action "x" in scope instance' });
   });
 });