Merge remote-tracking branch 'origin/main' into claude/issue-15555-strand-repairable-window

Author: claude

.changeset/approvals-bu-member-org-screen.md

@@ -0,0 +1,9 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+Fix: a `department` approver on a seeded business unit no longer routes the approval to another organization's members.
+
+`ApprovalService.expandBusinessUnitUsers` screened the `sys_business_unit` rows with the null-inclusive tenant predicate (#3807 — a seeded unit carries no organization and is admitted on purpose) but read `sys_business_unit_member` with no organization predicate at all, under a system context that carries no tenant either. A seeded unit id exists identically in every tenant, so a `department:<id>` approver on tenant A's request resolved the shared unit and then collected every tenant's membership rows hanging off it — approval authority over A's record, routed to B's users. The member read now carries a strict `organization_id` equality against the directory organization the approver resolves in: the same screen `plugin-sharing` applies to these rows, and the same posture this package already takes for `sys_team_member` and `sys_user_position`.
+
+The screen is strict rather than null-inclusive on purpose. `sys_business_unit_member.organization_id` is filled by REST/session writes but left NULL by seed replay and by elevated system-context writes (tracked in #14570), so a NULL on a membership row means unknown tenancy, not "platform-global", and routing fails closed on it. Declared cost: on a deployment whose membership rows (not merely its units) were seeded or system-written, a `department` approver on a request that carries an organization now expands to nobody — the slot falls to the `department:<id>` literal, the existing `expanded to nobody` warning (#3807) names it, and `onEmptyApprovers` governs the request as for any unstaffed target. The repair is to stamp those membership rows. A request that carries no organization is unchanged, and so is every unit-level screen.

.changeset/olive-spiders-refuse.md

@@ -0,0 +1,13 @@
+---
+"@objectstack/cli": minor
+---
+
+**BREAKING** `os create <type> <name>` now refuses a project name that npm refuses, and refuses it before it writes anything.
+
+`os create plugin "My App"` used to exit 0 having written `./plugin-My App/`, carrying a manifest that read `name: "@objectstack/plugin-My App"`. Nothing failed at scaffold time, so the invalid name surfaced later at `npm publish`, in the terminal of whoever ran it next. `os init` has always refused that same input before touching the disk. The rule set is now shared between the two scaffolders rather than restated in one of them, so they answer the same way.
+
+`os create` also refuses a name whose composed scoped package name exceeds npm's 214-character ceiling. `@objectstack/plugin-` spends 20 of those characters before the name begins, so a name that `os init` accepts can still compose to one npm rejects; that check sits next to the composition rather than in the shared rule set.
+
+A scripted invocation that passed an invalid name now exits 1 with the reason on stderr, where it previously exited 0 and produced a project that could not be published.
+
+<!-- adr-0087: not-required (no-migration-prescription) The change narrows what a CLI argument accepts at invocation time. No metadata surface, stored row or spec declaration is touched, so `objectstack migrate meta` has nothing to carry and the ledger has nothing to record. -->

.changeset/runtime-tenancy-posture-failure-discrimination.md

@@ -0,0 +1,14 @@
+---
+"@objectstack/runtime": patch
+---
+
+The runtime dispatcher door no longer admits a request on a tenancy posture it could not read.
+
+`resolveExecutionContext` reads the effective tenancy posture from the kernel's `tenancy` service, and both posture-conditional API-key refusals (`organization_required`, `organization_membership_ended`) run only when that posture is present. The read used to swallow every failure into "no posture", so a `tenancy` service that was **registered and failed to build** answered exactly like a deployment with no tenancy at all: the wall was skipped, and an API key stamped with an organization its owner had left — or carrying no organization — was admitted with full grants.
+
+The seam now carries the same discrimination the REST door already applies (#13906 decision 1, option A), by the registry's own brand rather than by message text:
+
+- **never registered** — the supported no-tenancy composition. Absorbed as before: no posture, no posture-conditional refusal, nothing changes for single-organization embedders.
+- **registered and failed to build** — re-raised as `AuthzStoreUnavailableError`, so the door answers `503 SERVICE_UNAVAILABLE` ("the authorization store could not be read"), which is an existing member of the closed error vocabulary. A posture that could not be read is not a posture that is absent.
+
+Two nets between the resolver and the transport envelope are told the same thing, in the one shape `@objectstack/core` already prescribes for such seams (`rethrowAuthzStoreUnavailable`): the dispatcher's service facade hands the resolver the classified rejection for `tenancy` instead of collapsing it to `undefined`, and the identity step's catch re-raises only the branded outage while every other fault still degrades to an anonymous request. A consequence worth knowing: an authorization-store read failure (`AuthzStoreUnavailableError` from the permission tables) now also reaches this door as 503 instead of being served as an anonymous request.

content/docs/permissions/system-context.mdx

@@ -145,7 +145,7 @@ The largest single consumer — **17 of the 105 sites**.
 |:--|:---|:---|:---|:---|
 | 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` |
 | 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` |
-| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3248`, `:3396`, `:3564`, `:3635`, `:3824`, `:3864` |
+| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3305`, `:3453`, `:3621`, `:3692`, `:3881`, `:3921` |
 | 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` |
 | 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` |
 | 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` |

content/docs/protocol/kernel/index.mdx

@@ -293,8 +293,11 @@ that order.
 
 // 1. Identity — resolveExecutionContext() reads the better-auth session (or
 //    API key), aggregates positions/permission sets/RLS membership, and layers
-//    locale + timezone on top. It always resolves; anonymous yields
-//    `{ isSystem: false, positions: [], permissions: [] }`.
+//    locale + timezone on top. Anonymous yields
+//    `{ isSystem: false, positions: [], permissions: [] }`. It rejects with
+//    AuthzStoreUnavailableError (503) only when an authorization input exists
+//    and could not be read — a failed permission-store read, or a tenancy
+//    service that is registered and failed to build.
 const context = await resolveExecutionContext({ getService, getQl, request });
 // → ExecutionContext { userId, tenantId, locale, timezone, positions,
 //                      permissions, isSystem, ... }

packages/cli/src/commands/create.ts

@@ -82,9 +82,11 @@ import path from 'path';
 import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel';
 import {
   getCliVersion,
+  NPM_PACKAGE_NAME_MAX_LENGTH,
   renderPnpmWorkspaceYaml,
   sanitizeNamespace,
   SCAFFOLD_PNPM_RANGE,
+  validateProjectName,
 } from './init.js';
 
 /**
@@ -165,6 +167,52 @@ function defineTemplate(t: Omit<CreateTemplate, 'files'>): CreateTemplate {
   };
 }
 
+/**
+ * The scoped package name a scaffold is about to write, READ BACK off the
+ * rendered manifest rather than recomposed here.
+ *
+ * Recomposing it would be a second copy of `@objectstack/plugin-${name}` that
+ * nothing keeps in step with the renderer — the same restatement that let this
+ * command's emitted name drift away from what `os init` enforces. Reading the
+ * rendered object measures the string that actually lands on disk, and a
+ * template added later is covered without being told to declare anything.
+ *
+ * `null` when the template emits no `package.json`, or emits one without a
+ * string `name`: there is then no package name to judge, which is not the same
+ * as judging one and finding it fine.
+ */
+export function emittedPackageName(
+  template: CreateTemplate,
+  placement: ScaffoldPlacement,
+  name: string,
+): string | null {
+  const render = template.filesFor(placement)['package.json'];
+  if (!render) return null;
+  const manifest = render(name) as { name?: unknown } | null | undefined;
+  return typeof manifest?.name === 'string' ? manifest.name : null;
+}
+
+/**
+ * The one rule `os create` needs and `os init` cannot.
+ *
+ * `init`'s argument IS the package name, so measuring the argument is the same
+ * measurement. `create` composes its argument into a SCOPED name, and npm's
+ * 214-character ceiling counts the scope: `@objectstack/plugin-` spends 20 of
+ * them before the user's first character. A 200-character name is therefore
+ * legal for `init` (measured: accepted) and illegal for `create` (measured:
+ * emits a 220-character name npm refuses) — which is why the shared validator
+ * is shared and this check is not.
+ */
+export function validateEmittedPackageName(packageName: string): string | null {
+  const over = packageName.length - NPM_PACKAGE_NAME_MAX_LENGTH;
+  if (over <= 0) return null;
+  return (
+    `The package name this would emit is ${packageName.length} characters; npm's limit is `
+    + `${NPM_PACKAGE_NAME_MAX_LENGTH}. Shorten the project name by at least ${over} character`
+    + `${over === 1 ? '' : 's'}.`
+  );
+}
+
 function toCamelCase(str: string): string {
   return str.replace(/-([a-z])/g, (g) => g[1].toUpperCase());
 }
@@ -464,12 +512,39 @@ export default class Create extends Command {
       console.log(chalk.dim(`Usage: objectstack create ${args.type} <name>`));
       process.exit(1);
     }
-    
+
+    // ⛔ BEFORE the first write, which is the whole property — a refusal that
+    // arrives after `mkdirSync` has fixed the message and not the defect.
+    //
+    // This command used to validate nothing it emitted, so `os create plugin
+    // "My App"` exited 0 having written `./plugin-My App/` with a manifest
+    // reading `name: "@objectstack/plugin-My App"` — a name npm refuses —
+    // while `os init "My App"` refused the same input and wrote nothing. The
+    // rule set is `init`'s, imported rather than restated: the two scaffolders
+    // already share four symbols, and the one they did not share is the one
+    // they disagreed on.
+    const nameError = validateProjectName(args.name);
+    if (nameError) {
+      console.error(chalk.red(`\n❌ ${nameError}`));
+      console.log(chalk.dim(`  Usage: objectstack create ${args.type} <name>`));
+      process.exit(1);
+    }
+
     const template = templates[args.type as keyof typeof templates];
     const cwd = process.cwd();
     const placement: ScaffoldPlacement = flags['in-repo'] ? 'in-repo' : DEFAULT_PLACEMENT;
     const projectDirName = template.dirName(args.name);
 
+    // The check `init` cannot need, on the string `init` never composes. Also
+    // before any write, and read off the rendered manifest so it measures what
+    // would land rather than a second copy of how it is built.
+    const willEmit = emittedPackageName(template, placement, args.name);
+    const packageNameError = willEmit ? validateEmittedPackageName(willEmit) : null;
+    if (packageNameError) {
+      console.error(chalk.red(`\n❌ ${packageNameError}`));
+      process.exit(1);
+    }
+
     // Refuse `--in-repo` outside a workspace rather than emit the one thing
     // this command is no longer allowed to emit: a project that cannot install.
     if (placement === 'in-repo' && !fs.existsSync(path.join(cwd, 'pnpm-workspace.yaml'))) {

packages/cli/src/commands/init.ts

@@ -676,13 +676,34 @@ export function detectPackageManager(env: NodeJS.ProcessEnv = process.env): 'npm
   return 'npm';
 }
 
+/**
+ * npm's hard ceiling on a package name, the scope included.
+ *
+ * Exported because `os create` does NOT validate the same string this file
+ * does: it composes its argument into a scoped name
+ * (`@objectstack/plugin-<name>`) and has to measure the COMPOSED string
+ * against this limit. Restating the number over there is exactly how the two
+ * scaffolders came to disagree in the first place.
+ */
+export const NPM_PACKAGE_NAME_MAX_LENGTH = 214;
+
 /**
  * Validate that `name` is a usable npm package name AND a safe directory
  * segment. Mirrors the subset of rules used by `npm init`/`create-vite`.
+ *
+ * Exported for `os create`, which took none of this and therefore accepted
+ * names npm refuses — `os create plugin "My App"` wrote `./plugin-My App/`
+ * carrying `name: "@objectstack/plugin-My App"`, while `os init "My App"`
+ * refused the same input before touching the disk. The rule set is shared
+ * rather than copied so a rule added here reaches both scaffolders; the one
+ * check `create` needs and `init` cannot (the length of the composed scoped
+ * name) lives next to the composition, in `create.ts`.
  */
-function validateProjectName(name: string): string | null {
+export function validateProjectName(name: string): string | null {
   if (!name) return 'Project name is required';
-  if (name.length > 214) return 'Project name must be ≤ 214 characters';
+  if (name.length > NPM_PACKAGE_NAME_MAX_LENGTH) {
+    return `Project name must be ≤ ${NPM_PACKAGE_NAME_MAX_LENGTH} characters`;
+  }
   if (/[A-Z]/.test(name)) return 'Project name must be lowercase';
   if (!/^[a-z0-9][a-z0-9._-]*$/.test(name)) {
     return 'Project name must start with a lowercase letter or digit and contain only [a-z0-9._-]';