Merge remote-tracking branch 'origin/main' into claude/issue-21009-json-column-text-operators

Author: claude

.changeset/20981-aggregation-flag-comparand-refused.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/objectql': minor
+---
+
+fix(objectql)!: a per-aggregation `filter` and a `having` refuse a non-boolean `$exists` / `$null` with `INVALID_FILTER` / 400, in the words every driver's `where` refuses it in, instead of reading `$exists` by truthiness and dropping `$null` (#20981)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (already-registered filter-query-face-comparands-refused-at-save) this narrows the engine's own in-process evaluator to the rule that registered entry already records: its reason states that every query face refuses a non-boolean $null / $exists flag, its surface names a query having, the data-engine aggregate call's having and an aggregation filter among the carriers, and its replacement is this change's whole migration (a flag is the boolean itself; $null true is "has no value", $exists true is "has a value"). This change makes that statement true on the per-aggregation filter and having positions, which the engine evaluates itself after the driver. No authorable key, spelling, export or published type moves, and no stored row is read or rewritten; a stored filter carrying such a flag is already refused when it is saved, by that entry. -->
+
+**BREAKING**: this narrows what `aggregate` accepts in two positions, `aggregations[i].filter` and `having`, on every driver. A `$exists` or `$null` comparand that is not a boolean (a string such as `"false"`, a number, `null`, an array) is now refused with `INVALID_FILTER` / 400, before any driver is asked for a row, so an empty table refuses it too, at any depth under `$and` / `$or` / `$not`. A plain object or `undefined` there is refused first by the comparand-type check, in its own words, as before; a `{ $field }` reference there, already refused as a reference outside a scalar comparison, is now refused in this entry's words. The published `applyInMemoryAggregation(rows, ast, timezone, fields)` narrows the same way, per row: it throws the same refusal for a row its per-aggregation filter judges on the flag, with or without a `fields` map (an empty `rows` array, or a row a `$or` branch settles first, is not judged there; `engine.aggregate` judges the whole filter once before any row). It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
+
+**Why a refusal.** `FieldOperatorsSchema` declares both flags as booleans, and every driver's `where` refuses any other comparand. The engine evaluates a per-aggregation `filter` and a `having` itself, and it read one anyway. Measured through `engine.aggregate` on the in-memory driver and on `SqlDriver` (SQLite), with identical answers: `$exists` was read by truthiness, so `"yes"`, `1` and the string `"false"` selected the rows and groups WITH a value, and `0` / `null` the ones without; and `$null` tested only `true` / `false`, so any other value constrained nothing, and every row and every group came back.
+
+**What an author sees now.** The message `driver-sql` gives the same flag, beginning `Operator "$exists" on field "FIELD" requires a boolean comparand (true or false).`, naming what arrived and the position (`aggregations[1].filter.stage.$exists`, `having.stage.$null`). Unlike a `where` on `SqlDriver`, the field and the value are not withheld: a per-aggregation `filter` and a `having` never carry a merged read scope.
+
+**What to write instead.** Write the boolean itself. `"$exists": true` and `"$null": false` match a field that has a value; `"$exists": false` and `"$null": true` match one that has none.
+
+**Who is affected.** A caller that reaches `engine.aggregate` without the REST query door's schema parse (server-side code, a flow or hook, the analytics bridge that lowers a dataset measure's filter into an aggregation filter, a host calling `applyInMemoryAggregation` directly) and read the count as a real answer. `POST /api/v1/data/:object/query` already refused all three positions with 400 `VALIDATION_FAILED` before the request reached the engine, and still does.
+
+**Unchanged.** `$exists: true` / `false` and `$null: true` / `false` answer exactly as before, on both positions. `$empty` and every other operator, and `where`.

.changeset/21062-picker-queryable-key.md

@@ -0,0 +1,44 @@
+---
+'@objectstack/rest': minor
+---
+
+fix(rest)!: a public form's lookup picker searches and sorts by the first display field the caller may query, so a picker whose first display field is masked for its caller serves its rows instead of answering 403 (#21062)
+
+Clause-②: yes (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) a change of which display field the public lookup picker (`GET /forms/:slug/lookup/:field`) searches and sorts by: the first display field the caller may query, by the security service's answer, where it used to be the first display field. No authorable key, spelling, export or stored shape moves: `@objectstack/rest` exports nothing new and nothing less, `FormFieldPublicPickerSchema` keeps parsing every value it parsed, and no stored row is read or rewritten. A picker's `displayFields` keep their meaning as the projected fields. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers which field a picker keys on (not `already-registered`); and the change is route behaviour, not a declaration (not `runtime-interface-only` / `type-surface-only`). -->
+
+**BREAKING**: this widens what the public lookup picker serves and narrows it in one composition. The narrowing: with a security service that lacks `ISecurityService.getQueryableFields`, or that answers no answer for the object, the picker passes over every display field whose declaration carries a `maskingRule`, for every caller, including a caller the rule is lifted for. So its search and order move to the next display field that declares no rule, and a picker whose display fields all declare a rule is refused `403 PERMISSION_DENIED` without the engine being asked, where it used to be served. The security service this repository ships implements the method, so a deployment using it is not narrowed. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
+
+**What changed.** The public lookup picker (`GET /forms/:slug/lookup/:field`)
+matches the visitor's search and orders its rows by one key. That key used to
+be the first entry of `publicPicker.displayFields`. It is now the first entry
+the caller may query on, as the security service answers it
+(`ISecurityService.getQueryableFields`). A field whose masking rule applies to
+a caller is served to that caller masked, and the engine refuses to search or
+sort on it with `403 PERMISSION_DENIED`. A picker whose first display field
+declares such a rule therefore answered `403` to every caller the rule applies
+to, on every request. It now serves its rows, sorted and searched on the next
+display field the caller may query. The masked field is still returned in each
+row, masked, as before.
+
+**When no display field is queryable** for the caller, the picker answers
+`403 PERMISSION_DENIED` with the engine's refusal for those fields, without
+running a query.
+
+**Unchanged.** A picker with no masked display field, and a caller the masking
+rule is lifted for, keep the first display field as the key, with the security
+service this repository ships. A deployment with no security service keeps the
+first display field.
+
+**What to do.** Nothing. To choose the field a picker searches when its first
+display field is masked for some of its callers, list a field those callers may
+query among `displayFields`: the first such entry is the one searched and
+sorted on. A picker whose only display fields are masked for its callers is
+refused, so give it one they may query.
+
+**What to do after upgrading, if your security service predates `getQueryableFields`.**
+Implement `getQueryableFields` on it: it answers which fields a caller may filter,
+sort, group or aggregate by, and the picker then keys on the first display field
+in that answer. Until it does, give each picker at least one display field that
+declares no `maskingRule`, or the picker is refused for every caller.

.changeset/21114-start-signal-forwarding.md

@@ -0,0 +1,31 @@
+---
+'@objectstack/cli': patch
+---
+
+fix(cli): `os start` forwards SIGTERM and SIGINT to its `serve` child and takes the child down when it exits
+
+Clause-②: no
+
+`os start` runs the server as a separate `serve` child process. It used to
+listen for that child's exit and nothing else. A SIGTERM or SIGINT sent to the
+`start` process alone (a plain `kill`, `docker stop`, a systemd stop, a CI step)
+ended `start` and left `serve` running with no parent. The port stayed bound,
+`/health` kept answering 200, and the next start on that port collided with it.
+
+`os start` now supervises its child the way `os dev` already does, through the
+same mechanism:
+
+- SIGTERM or SIGINT to `start` is forwarded to the child. `start` waits for the
+  child to shut down, then exits with the child's exit code, which is 0 after a
+  graceful shutdown. It used to die on the signal at once (shell status 143 for
+  SIGTERM, 130 for SIGINT) while the child kept running.
+- Whatever else ends `start`, the child is sent SIGTERM on the way out.
+- A child that exits on its own still ends `start` with the child's exit code,
+  as before.
+
+One visible side effect, the same one `os dev` already has: Ctrl-C at a terminal
+signals `start` and the child together, so the child now receives SIGINT twice
+and logs one `Shutdown already in progress, ignoring SIGINT` warning. The
+terminal prompt also returns only after the server has stopped, not before.
+
+No flag, port, environment variable, banner line or `os dev` behaviour changes.

packages/cli/src/commands/start.ts

@@ -13,6 +13,7 @@ import { redirectStdoutToStderr } from '../utils/json-stdout.js';
 import { redactConnectionUrl } from '../utils/connection-display.js';
 import { databaseDriverFlag } from '../utils/database-driver-flag.js';
 import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
+import { ServeRestartCoordinator } from '../utils/dev-restart.js';
 import { readEnvWithDeprecation } from '@objectstack/types';
 // The ONE port contract, shared with `dev` and with the `serve` child this
 // command spawns (#12673). ⛔ Nothing about ports is declared in this file —
@@ -445,18 +446,50 @@ export default class Start extends Command {
     }
 
     const binPath = process.argv[1];
-    const child = spawn(
-      process.execPath,
-      [
-        binPath,
-        'serve',
-        flags.ui ? '--ui' : '--no-ui',
-        ...(flags.verbose ? ['--verbose'] : []),
-        ...(flags['log-level'] ? ['--log-level', flags['log-level']] : []),
-      ],
-      { stdio: 'inherit', env: localEnv },
-    );
-    child.on('exit', (code) => process.exit(code ?? 0));
+
+    // ── The serve child's lifecycle: `os dev`'s mechanism, not a copy (#21114)
+    // `start` is a supervisor, so a signal sent to ITS pid alone — a plain
+    // `kill`, `docker stop`, a systemd stop, a CI step — has to reach the
+    // child too. This command used to listen for the child's `exit` and
+    // nothing else: a SIGTERM to the parent ended the parent and left `serve`
+    // running, reparented to init, port still bound and `/health` still
+    // answering 200.
+    //
+    // ⭐ Supervised by `ServeRestartCoordinator`, the ONE forwarding mechanism
+    // `os dev` already runs, used as is — `start` never calls
+    // `requestRestart`, so only the parts both commands need are reached:
+    // `beginShutdown` forwards SIGINT / SIGTERM and lets the child's exit end
+    // the parent; `killChildOnParentExit` reaps the child on whatever path
+    // ends the parent; and a child that exits on its own still ends the
+    // parent with `code ?? 0`, exactly the handler this replaces.
+    // ⛔ No forwarding or reaping logic of this command's own: what a signal
+    // DOES belongs in the coordinator, where `dev` gets the same change.
+    // Pinned end to end, with `os dev` as the control, by
+    // `test/start-signal-forwarding.e2e.test.ts`.
+    const coordinator = new ServeRestartCoordinator({
+      spawnChild: () => {
+        // `const child = spawn(` is the anchor
+        // `utils/port-contract-single-source.test.ts` reads to hold the port
+        // door above AHEAD of this spawn — keep the spelling.
+        const child = spawn(
+          process.execPath,
+          [
+            binPath,
+            'serve',
+            flags.ui ? '--ui' : '--no-ui',
+            ...(flags.verbose ? ['--verbose'] : []),
+            ...(flags['log-level'] ? ['--log-level', flags['log-level']] : []),
+          ],
+          { stdio: 'inherit', env: localEnv },
+        );
+        return child;
+      },
+      exitParent: (code) => process.exit(code),
+    });
+    process.on('SIGINT', () => coordinator.beginShutdown('SIGINT'));
+    process.on('SIGTERM', () => coordinator.beginShutdown('SIGTERM'));
+    process.on('exit', () => coordinator.killChildOnParentExit());
+    coordinator.start();
   }
 }