Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/slot-lookup-sweep-b5-plugins.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
---
"@objectstack/plugin-approvals": patch
"@objectstack/plugin-sharing": patch
"@objectstack/plugin-reports": patch
"@objectstack/plugin-email": patch
"@objectstack/plugin-pinyin-search": patch
"@objectstack/plugin-webhooks": patch
"@objectstack/plugin-audit": patch
---

fix(plugins): sweep the service-lookup erasures out of the plugin composition roots, and fix the two alias-only HTTP reads it exposed (#4251 B5)

Batch B5 of the #4251 sweep: the seven remaining `packages/plugins/*` composition
roots. 35 lookup sites that had been erased to `any` now carry the slot's
contract, so the compiler checks what each plugin actually calls on the service
it resolved. The ratchet drops 143 sites / 32 files to 108 / 25.

**Two real defects, both of the shape this sweep exists to find.** Approvals'
actionable-link pages (ADR-0043) and sharing's public share-link REST routes each
read the HTTP server under `http-server` *only* — the deprecated alias. The
ledger records `http.server` as canonical and as the only name present on every
provider path: `runtime.ts`'s `config.server` path registers no alias at all. On
that path both lookups threw, the surrounding `catch` swallowed it, and the
routes silently never mounted — approval e-mail action links 404'd and the
share-link surface was absent, with nothing in the log to say so. Both reads are
now canonical-first with the alias as fallback, each name in its own `try`
because `getService` throws on an empty slot (so `a() ?? b()` inside one `try`
never reaches `b` — the same correction #4393 made in metadata and
cloud-connection).

Typing choices follow the batch method: pure data-plane consumers take the
narrow contract (`IDataEngine` in reports), consumers that bind hook or
middleware seams take the engine seen whole (`IObjectQLEngine` in approvals,
sharing and pinyin-search), and slots with no contract get a **named** local
surface rather than `any` — plugin-email's `MailSettingsSurface`, and the
surfaces the consuming packages already declared (`ApprovalMessagingSurface`,
`SharingSecurityProbe`, `ReportEmail`). A named surface that omits a member
still makes the compiler name every call site; `any` says nothing.

No behaviour change beyond the two alias reads. No contract changes.
47 changes: 38 additions & 9 deletions packages/plugins/plugin-approvals/src/approvals-plugin.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import type { Plugin, PluginContext } from '@objectstack/core';
import type {
IHttpServer,
II18nService,
IJobService,
IObjectQLEngine,
} from '@objectstack/spec/contracts';
import { SysApprovalRequest } from './sys-approval-request.object.js';
import { SysApprovalAction } from './sys-approval-action.object.js';
import { SysApprovalApprover } from './sys-approval-approver.object.js';
Expand All@@ -12,6 +18,7 @@ import {
ESCALATION_JOB_NAME,
ESCALATION_SCAN_INTERVAL_MS,
type ApprovalEngine,
type ApprovalMessagingSurface,
} from './approval-service.js';
import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js';
import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js';
Expand DownExpand Up@@ -55,7 +62,7 @@ export class ApprovalsServicePlugin implements Plugin {

private readonly options: ApprovalsPluginOptions;
private service?: ApprovalService;
private engine?: any;
private engine?: IObjectQLEngine;
private escalationJobScheduled = false;

constructor(options: ApprovalsPluginOptions = {}) {
Expand DownExpand Up@@ -93,7 +100,7 @@ export class ApprovalsServicePlugin implements Plugin {
if (typeof (ctx as any).hook === 'function') {
(ctx as any).hook('kernel:ready', async () => {
try {
const i18n = ctx.getService<any>('i18n');
const i18n = ctx.getService<II18nService>('i18n');
if (i18n && typeof i18n.loadTranslations === 'function') {
const { ApprovalsTranslations } = await import('./translations/index.js');
for (const [locale, data] of Object.entries(ApprovalsTranslations)) {
Expand All@@ -108,9 +115,14 @@ export class ApprovalsServicePlugin implements Plugin {

async start(ctx: PluginContext): Promise<void> {
if (this.options.disableService) return;
let engine: any = null;
try { engine = ctx.getService<any>('objectql'); }
catch { try { engine = ctx.getService<any>('data'); } catch { /* ignore */ } }
// This plugin needs the engine SEEN WHOLE, not the data plane: it binds
// `registerHook` / `unregisterHooksByPackage` below. That is `objectql`,
// whose ledger entry (`CoreServiceContracts`) records it as "the SAME
// instance as `data`, seen whole" — so the alias fallback resolves the same
// object and is typed as the same contract.
let engine: IObjectQLEngine | null = null;
try { engine = ctx.getService<IObjectQLEngine>('objectql'); }
catch { try { engine = ctx.getService<IObjectQLEngine>('data'); } catch { /* ignore */ } }
if (!engine) {
ctx.logger.warn('ApprovalsServicePlugin: no ObjectQL engine — service NOT registered');
return;
Expand DownExpand Up@@ -158,7 +170,11 @@ export class ApprovalsServicePlugin implements Plugin {
// remind / request-info / comment) notify users when present; without it
// they degrade to audit-only.
try {
const messaging = ctx.getService<any>('messaging');
// `messaging` has no contract in the ledger, so this is the named local
// surface the service itself already declares — not `any`. It omits
// members on purpose; omitting one it USES would be a compile error at
// the `attachMessaging` call, which is the whole point.
const messaging = ctx.getService<ApprovalMessagingSurface>('messaging');
if (messaging && typeof messaging.emit === 'function') {
this.service.attachMessaging(messaging);
}
Expand All@@ -171,7 +187,7 @@ export class ApprovalsServicePlugin implements Plugin {
// service → SLA stays display-only.
const wireEscalationClock = async () => {
try {
const jobs = ctx.getService<any>('job');
const jobs = ctx.getService<IJobService>('job');
if (!jobs || typeof jobs.schedule !== 'function' || !this.service) return;
const svc = this.service;
const intervalMs = this.options.escalationScanIntervalMs ?? ESCALATION_SCAN_INTERVAL_MS;
Expand DownExpand Up@@ -213,7 +229,20 @@ export class ApprovalsServicePlugin implements Plugin {
// happens exclusively on the POST (mail-gateway prefetch safe).
const mountActionPages = async () => {
try {
const http = ctx.getService<any>('http-server');
// [#4251 B5] Canonical name FIRST. This read was `http-server`-only,
// and `http-server` is the deprecated alias: the ledger records
// `http.server` as canonical and as "the ONLY name present on all
// provider paths" — `runtime.ts`'s `config.server` path registers no
// alias at all. On that path this lookup threw, the catch below
// swallowed it, and the ADR-0043 action pages silently never mounted,
// so every approval e-mail link 404'd. Same latent alias-only miss
// #4393 fixed in metadata/cloud-connection; per-name `try` because
// `getService` THROWS on an empty slot, so `a() ?? b()` in one `try`
// never reaches `b`.
const readServer = (name: string): IHttpServer | undefined => {
try { return ctx.getService<IHttpServer>(name); } catch { return undefined; }
};
const http = readServer('http.server') ?? readServer('http-server');
const rawApp = http && typeof http.getRawApp === 'function' ? http.getRawApp() : null;
if (!rawApp || !this.service) return;
const svc = this.service;
Expand DownExpand Up@@ -304,7 +333,7 @@ export class ApprovalsServicePlugin implements Plugin {
async stop(ctx: PluginContext): Promise<void> {
if (this.escalationJobScheduled) {
try {
const jobs = ctx.getService<any>('job');
const jobs = ctx.getService<IJobService>('job');
await jobs?.cancel?.(ESCALATION_JOB_NAME);
} catch { /* ignore */ }
this.escalationJobScheduled = false;
Expand Down
4 changes: 2 additions & 2 deletions packages/plugins/plugin-audit/src/audit-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,7 @@

import type { Plugin, PluginContext } from '@objectstack/core';
import { resolveLocalizationContext } from '@objectstack/core';
import type { IDataEngine, ISharingService } from '@objectstack/spec/contracts';
import type { IDataEngine, II18nService, ISharingService } from '@objectstack/spec/contracts';
import { SysAuditLog, SysActivity, SysComment } from './objects/index.js';
// `sys_notification` was parked here "until that [ADR-0030] migration lands".
// It has landed, so the contribution moved to @objectstack/service-messaging —
Expand DownExpand Up@@ -61,7 +61,7 @@ export class AuditPlugin implements Plugin {
if (typeof (ctx as any).hook === 'function') {
(ctx as any).hook('kernel:ready', async () => {
try {
const i18n = ctx.getService<any>('i18n');
const i18n = ctx.getService<II18nService>('i18n');
if (i18n && typeof i18n.loadTranslations === 'function') {
const { AuditTranslations } = await import('./translations/index.js');
for (const [locale, data] of Object.entries(AuditTranslations)) {
Expand Down
47 changes: 44 additions & 3 deletions packages/plugins/plugin-email/src/email-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,7 +28,11 @@ import {
type EmailTransportProvider,
} from './transports/index.js';
import { BUILTIN_AUTH_TEMPLATES } from './templates/auth-templates.js';
import type { EmailTemplateDefinition as EmailTemplate } from '@objectstack/spec/system';
import type {
EmailTemplateDefinition as EmailTemplate,
SettingsChangeHandler,
SettingsUnsubscribe,
} from '@objectstack/spec/system';
import {
bootstrapDeclaredEmailTemplates,
upsertDeclaredEmailTemplate,
Expand DownExpand Up@@ -120,6 +124,43 @@ const QUEUE_DELIVERY_BACKOFF: QueueBackoffPolicy = {
maxDelayMs: 5 * 60_000,
};

/**
* The `settings` slot as THIS plugin consumes it.
*
* [#4251 B5] `service-settings` registers its `SettingsService` here and the
* slot carries no `packages/spec` contract, so the four members this plugin
* calls are declared structurally — plugin-email must not take a runtime
* dependency on service-settings, which is optional. Same shape and reasoning
* as plugin-auth's and rest's `SettingsReadSurface`; each consumer names the
* slice it uses, so omitting a member it calls is a compile error rather than
* silence. The change-bus types are the SPEC's, not re-declared here.
*/
interface MailSettingsSurface {
/**
* Capability probe only — never called. Its presence is what the call site
* reads as "this settings service can serve a live client", so it is typed
* as a member of unknown shape rather than given a fictional signature.
*/
createClient?: unknown;
/** Resolve the whole `mail` namespace as `key → { value, source }`. */
getNamespace(
namespace: string,
ctx?: Record<string, unknown>,
): Promise<{ values: Record<string, { value?: unknown; source?: string } | undefined> }>;
/** Rebuild the transport when the namespace changes. Optional: no change bus → the boot read stands. */
subscribe?(namespace: string | undefined, handler: SettingsChangeHandler): SettingsUnsubscribe;
/** Override service-settings' validate-only `mail/test` fallback with a real send. */
registerAction?(
namespace: string,
action: string,
handler: (input: {
values?: Record<string, unknown>;
payload?: Record<string, unknown>;
ctx?: { body?: Record<string, unknown> };
}) => Promise<unknown>,
): void;
}

/**
* Resolve a queue service that can actually carry a durable email job, or
* `undefined`.
Expand DownExpand Up@@ -302,7 +343,7 @@ export class EmailServicePlugin implements Plugin {
// without restarting the process. Env-locked fields still win at
// the resolver level, so config-via-env keeps its precedence.
try {
const settings = ctx.getService<any>('settings');
const settings = ctx.getService<MailSettingsSurface>('settings');
if (settings && typeof settings.createClient === 'function') {
const applySettings = async (phase: 'boot' | 'saved' = 'boot') => {
try {
Expand DownExpand Up@@ -625,7 +666,7 @@ export class EmailServicePlugin implements Plugin {
// the true attempt count. One message is one row; `attempt_count`
// accumulates on it across redeliveries.
try {
const queue: any = ctx.getService<any>('queue');
const queue= ctx.getService<IQueueService>('queue');
if (queue && typeof queue.subscribe === 'function' && this.service) {
const svc = this.service;
await queue.subscribe(EMAIL_SEND_QUEUE, async (msg: any) => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,7 +18,10 @@
* `pinyin-pro` is never imported.
*/

import type { Plugin, PluginContext } from '@objectstack/core';
// `IObjectQLEngine` via the core barrel, which re-exports it from
// `@objectstack/spec/contracts`: this package does not depend on spec directly,
// and the slot's contract is not a reason to add a dependency.
import type { IObjectQLEngine, Plugin, PluginContext } from '@objectstack/core';
import { resolveSearchPinyinEnabled } from '@objectstack/types';
import {
bindSearchCompanionHooks,
Expand DownExpand Up@@ -91,12 +94,18 @@ export class PinyinSearchPlugin implements Plugin {
}
}

private resolveEngine(ctx: PluginContext): any {
/**
* [#4251 B5] The engine SEEN WHOLE — `bindSearchCompanionHooks` binds
* `registerHook`, which lives on `IObjectQLEngine`, not on the data plane.
* The ledger records `objectql` as "the SAME instance as `data`, seen whole",
* so the alias fallback resolves the same object under the same contract.
*/
private resolveEngine(ctx: PluginContext): IObjectQLEngine | null {
try {
return ctx.getService<any>('objectql');
return ctx.getService<IObjectQLEngine>('objectql');
} catch {
try {
return ctx.getService<any>('data');
return ctx.getService<IObjectQLEngine>('data');
} catch {
return null;
}
Expand Down
34 changes: 25 additions & 9 deletions packages/plugins/plugin-reports/src/reports-plugin.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import type { Plugin, PluginContext } from '@objectstack/core';
import type {
IDataEngine,
IJobService,
ISecurityService,
SecurityContext,
} from '@objectstack/spec/contracts';
import {
SysSavedReport,
SysReportSchedule,
Expand DownExpand Up@@ -46,7 +52,7 @@ export class ReportsServicePlugin implements Plugin {
private service?: ReportService;
private intervalHandle?: ReturnType<typeof setInterval>;
private jobName?: string;
private jobService?: any;
private jobService?: IJobService;

constructor(options: ReportsPluginOptions = {}) {
this.options = options;
Expand All@@ -68,16 +74,23 @@ export class ReportsServicePlugin implements Plugin {

async start(ctx: PluginContext): Promise<void> {
ctx.hook('kernel:ready', async () => {
let engine: any = null;
try { engine = ctx.getService<any>('objectql'); }
catch { try { engine = ctx.getService<any>('data'); } catch { /* ignore */ } }
// `IDataEngine`, not the whole engine: `ReportEngine` is a pure data-plane
// slice (find/findOne/insert/update/delete), so the narrow contract is the
// honest one for BOTH names — `objectql` strictly widens `data` (#4404),
// and nothing here reaches past the data plane.
let engine: IDataEngine | null = null;
try { engine = ctx.getService<IDataEngine>('objectql'); }
catch { try { engine = ctx.getService<IDataEngine>('data'); } catch { /* ignore */ } }
if (!engine) {
ctx.logger.warn('ReportsServicePlugin: no ObjectQL engine — service NOT registered');
return;
}

let email: ReportEmail | undefined;
try { email = ctx.getService<any>('email'); } catch { /* email is optional */ }
// `ReportEmail` — the named surface this plugin consumes. `IEmailService`
// passes straight through it (see the declaration); naming the slice is
// what makes a drift in `send`'s shape a compile error here.
try { email = ctx.getService<ReportEmail>('email'); } catch { /* email is optional */ }
if (!email) {
ctx.logger.warn('ReportsServicePlugin: no email service — schedules will fire without delivery');
}
Expand All@@ -92,10 +105,13 @@ export class ReportsServicePlugin implements Plugin {
// permission sets anywhere) → the axis does not apply, matching the REST
// export route's fail-open.
const canExport = async (object: string, context: unknown): Promise<boolean> => {
let security: any;
try { security = ctx.getService<any>('security'); } catch { return true; }
let security: ISecurityService | undefined;
try { security = ctx.getService<ISecurityService>('security'); } catch { return true; }
if (!security || typeof security.canExport !== 'function') return true;
return await security.canExport(object, context);
// `ReportService` hands this callback an `unknown` context (it is the
// caller's execution envelope, opaque to reports); the security service
// types it as a partial `ExecutionContext`.
return await security.canExport(object, context as SecurityContext | undefined);
};

this.service = new ReportService({
Expand DownExpand Up@@ -124,7 +140,7 @@ export class ReportsServicePlugin implements Plugin {
// Prefer the platform job service when available — it lets ops
// see report dispatch alongside every other scheduled job.
try {
const job = ctx.getService<any>('job');
const job = ctx.getService<IJobService>('job');
if (job && typeof job.schedule === 'function') {
this.jobService = job;
this.jobName = 'reports.dispatch';
Expand Down
Loading
Loading