diff --git a/docs/qa/platform-checklist/areas/approvals.json b/docs/qa/platform-checklist/areas/approvals.json index ad216cc0dd9..b70babf9fff 100644 --- a/docs/qa/platform-checklist/areas/approvals.json +++ b/docs/qa/platform-checklist/areas/approvals.json @@ -508,12 +508,12 @@ "source": [ "#3358 §1 (the action-table evidence this matrix grounds in)", "packages/rest/src/rest-route-ledger.ts (approve/reject/recall/revise/resubmit/reassign/remind/request-info/comment + GET /:id/actions)", - "packages/rest/src/rest-server.ts#flowMoveRoute (flowMoveRoute: revise=pending approver, resubmit=submitter; threadRoute access per action; recall submitter-only)", + "packages/rest/src/approvals-routes.ts#flowMoveRoute (flowMoveRoute: revise=pending approver, resubmit=submitter; threadRoute access per action; recall submitter-only)", "packages/plugins/plugin-approvals/src/approval-service.ts#ApprovalService (ApprovalService.resubmit — 'traversal walks the declared back-edge into the approval node, whose executor opens the round-N+1 request'; the resubmitted row's status is never rewritten. sendBack counts prior rounds as returned siblings on flow_run_id + flow_node_id)", "packages/spec/src/contracts/approval-service.ts#APPROVAL_STATUSES (APPROVAL_STATUSES: pending|approved|rejected|recalled|returned)", "examples/app-showcase/src/automation/flows/index.ts#BudgetApprovalFlow (BudgetApprovalFlow — ADR-0044 revise loop, maxRevisions 2, exec step without a revise edge, lockRecord pair)", "packages/plugins/plugin-approvals/src/approval-service.ts#readBackRequest (readBackRequest — the write path's post-mutation echo; #12769 docblock: org-less rows are a live state, the null echo used to escape behind the callers' fresh! non-null assertions, now it refuses loudly; landed as PR #13181)", - "packages/rest/src/rest-server.ts#handleApprovalError (handleApprovalError prefix map — READ_BACK_FAILED deliberately unmapped, so it reaches the wire as the generic 500 APPROVAL__FAILED envelope carrying the named text)" + "packages/rest/src/approvals-routes.ts#handleApprovalError (handleApprovalError prefix map — READ_BACK_FAILED deliberately unmapped, so it reaches the wire as the generic 500 APPROVAL__FAILED envelope carrying the named text)" ], "history": [ { "revision": 1, "date": "2026-08-07", "change": "initial — decision-action matrix derived from the approvals REST route ledger and the ADR-0044 revise/resubmit flow shape", "ref": "claude/platform-test-checklist-ocwugl" }, diff --git a/docs/qa/platform-checklist/areas/records-forms.json b/docs/qa/platform-checklist/areas/records-forms.json index a4a507a28eb..5777847ee98 100644 --- a/docs/qa/platform-checklist/areas/records-forms.json +++ b/docs/qa/platform-checklist/areas/records-forms.json @@ -2414,7 +2414,7 @@ "source": [ "objectui: packages/plugin-detail/src/ConcurrentUpdateDialog.tsx (Reload/Overwrite UX), packages/plugin-form/src/occSave.tsx (If-Match → 409, conflict dialog, re-key overwrite)", "objectui: packages/plugin-detail/src/InlineEditSaveBar.tsx + packages/react/src/context/InlineEditContext.tsx (one save bar, changed-keys draft, ifMatch)", - "framework: packages/rest/src/rest-server.ts#mapping (CONCURRENT_UPDATE → 409 mapping), packages/metadata-protocol/src/protocol.ts#updateData updateData (If-Match / expectedVersion)", + "framework: packages/rest/src/approvals-routes.ts#mapping (CONCURRENT_UPDATE → 409 mapping), packages/metadata-protocol/src/protocol.ts#updateData updateData (If-Match / expectedVersion)", "PENDING-GAPS §C concurrent-edit-conflict + §E1 inline-edit atomic two-surface (objectui#2542/2549/2604); cross-ref records-forms.crud-roundtrip" ], "history": [ diff --git a/packages/qa/dogfood/test/authz-conformance.test.ts b/packages/qa/dogfood/test/authz-conformance.test.ts index fac0274c31c..efcadb33958 100644 --- a/packages/qa/dogfood/test/authz-conformance.test.ts +++ b/packages/qa/dogfood/test/authz-conformance.test.ts @@ -35,11 +35,25 @@ import { LEDGER_POPULATION_BASELINE, LEDGER_POPULATION_BASELINE_MAX, } from './authz-ledger-population.baseline.js'; +// [#23050 slice 3] THE list of the files `RestServer`'s registrars live in — +// see the two `rest-server.ts` realtime tripwires below. +import { + REST_SERVER_REGISTRAR_DIR, + REST_SERVER_REGISTRAR_FILES, +} from '../../../rest/src/rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); // packages/qa/dogfood/test → repo root. const REPO_ROOT = join(HERE, '../../../..'); +// [#23050 slice 3] Every file on the registrar list, repo-relative, and the ones +// beyond `rest-server.ts` that the realtime tripwires read WITH it. +const REST_SERVER_FILE = 'packages/rest/src/rest-server.ts'; +const REST_SERVER_REGISTRAR_PATHS: readonly string[] = + REST_SERVER_REGISTRAR_FILES.map((file) => `${REST_SERVER_REGISTRAR_DIR}/${file}`); +const REST_SERVER_REGISTRAR_ALSO_READS: readonly string[] = + REST_SERVER_REGISTRAR_PATHS.filter((file) => file !== REST_SERVER_FILE); + // ── #7976 — mutual row ↔ proof attribution ──────────────────────────────── // A proof file self-declares the rows it is the proof FOR with header // `// authz-row: ` lines, and `checkLedger` asserts the pairing both ways. @@ -123,6 +137,17 @@ interface Probe { * reading OF the table rather than of the file that carries it. */ within?: string; + /** + * [#23050 slice 3] More files read WITH `file`, as one text. Only the two + * `rest-server.ts` realtime tripwires declare it: they watch the code + * `RestServer`'s registrars run, and the split of `rest-server.ts` moves + * registrar bodies into modules of their own, so they read every file on the + * registrar list (`packages/rest/src/rest-server-registrar-files.ts`, + * imported above) and a transport wired in a moved body still trips them. + * `file` stays the one file the entry NAMES — what the probe-table census and + * the matrix header count — and the minted key is unchanged. + */ + alsoReads?: readonly string[]; key: (m: RegExpExecArray) => string; } @@ -430,12 +455,14 @@ const PROBES: readonly Probe[] = [ { kind: 'TRIPWIRE', file: 'packages/rest/src/rest-server.ts', + alsoReads: REST_SERVER_REGISTRAR_ALSO_READS, re: /handleUpgrade\s*\(|new\s+WebSocketServer\b|new\s+WebSocket\b|new\s+EventSource\b|upgradeWebSocket\b|WebSocketPair\b|Sec-WebSocket-|text\/event-stream|streamSSE\s*\(/g, key: () => tripwireKey('realtime:rest-server.ts:transport'), }, { kind: 'TRIPWIRE', file: 'packages/rest/src/rest-server.ts', + alsoReads: REST_SERVER_REGISTRAR_ALSO_READS, // Word-boundary lookaheads on `ws`/`stream` keep the file's 31 unrelated // response-streaming references (and paths like `/workspaces`) out: they // are matched only as a whole route segment, never as a prefix. @@ -520,7 +547,10 @@ const PROBES: readonly Probe[] = [ */ function probeSource(probe: Probe): string { const src = readFileSync(join(REPO_ROOT, probe.file), 'utf8'); - return probe.within ? scopedSource(src, probe.within) : src; + if (probe.within) return scopedSource(src, probe.within); + // [#23050 slice 3] `alsoReads` widens the read, never the scope: no probe + // declares both, and `within` above returns before it. + return [src, ...(probe.alsoReads ?? []).map((file) => readFileSync(join(REPO_ROOT, file), 'utf8'))].join('\n'); } /** @@ -1591,6 +1621,18 @@ describe('#9084 — the rest-server.ts transport tripwire keys off mechanics', ( const src = readFileSync(join(REPO_ROOT, REST_SERVER), 'utf8'); expect(hits(MECHANICS.re, src)).toEqual([]); expect(hits(ROUTES.re, src)).toEqual([]); + // [#23050 slice 3] …and on every file `RestServer`'s registrars live in: + // 0 over the list, whose length is pinned so a module added to it makes this + // re-read. The control is that both probes really READ the list — their + // text is every file on it, in order — so the zero is not a list ignored. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); + expect(REST_SERVER_REGISTRAR_PATHS[0]).toBe(REST_SERVER); + const listed = REST_SERVER_REGISTRAR_PATHS.map((file) => readFileSync(join(REPO_ROOT, file), 'utf8')); + for (const probe of [MECHANICS, ROUTES]) { + expect(probe.alsoReads).toEqual(REST_SERVER_REGISTRAR_PATHS.slice(1)); + expect(probeSource(probe)).toBe(listed.join('\n')); + } + expect(listed.reduce((n, text) => n + hits(MECHANICS.re, text).length + hits(ROUTES.re, text).length, 0)).toBe(0); // …and the surface walk agrees: no rest-server tripwire key is minted. const discovered = [...discoverAnonymousDenySurfaces()].filter((k) => k.startsWith('realtime:rest-server.ts:'), diff --git a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts index d82b09cac92..9abe1f0d6b0 100644 --- a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts +++ b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts @@ -238,6 +238,12 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, join } from 'node:path'; +// [#23050 slice 3] THE list of the files `RestServer`'s registrars live in — +// see the `rest-server.ts` row's population rule. +import { + REST_SERVER_REGISTRAR_DIR, + REST_SERVER_REGISTRAR_FILES, +} from '../../../rest/src/rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); // packages/qa/dogfood/test → repo root. @@ -511,9 +517,10 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ 'route registration sites — `this.routeManager.register(` call sites, LESS the one inside ' + '`registerPerItemRoute` (the shared forwarder, not a route; its extent is bounded by the declaration\'s own ' + 'indentation and the subtrahend is pinned at 1 by the `forwarder slice:` control, never inferred from a ' + - 'terminator spelling), PLUS `registerPerItemRoute(` call sites; counted over this file AND ' + - '`data-action-routes.ts` and `batch-routes.ts`, the modules the bodies of `registerDataActionEndpoints` ' + - 'and `registerBatchEndpoints` moved to; ' + + 'terminator spelling), PLUS `registerPerItemRoute(` call sites; counted over this file AND every ' + + 'module on the registrar list (`packages/rest/src/rest-server-registrar-files.ts`) — ' + + '`data-action-routes.ts`, `batch-routes.ts` and `approvals-routes.ts`, the modules the bodies of ' + + '`registerDataActionEndpoints`, `registerBatchEndpoints` and `registerApprovalsEndpoints` moved to; ' + 'reachable = those inside registerMetadataEndpoints', // [#15542 / #15854] ⭐ THE POPULATION RULE LEARNED A SECOND SPELLING, and // the numbers it produces did NOT move: 80 / 19 / 61, exactly as before. @@ -657,11 +664,26 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ // are counted in both readings; the four new occurrences are that slice's // seam, the same two spellings as slice 1's (the host literal's forwarder, // two; the host interface's member, two). No call site was added. + // [#23050 slice 3] 70 / 19 / 51 UNCHANGED, the same measurement again: + // `registerApprovalsEndpoints`' body (seven registration sites, all outside + // `registerMetadataEndpoints`, so all in the blind spot) moved to + // `approvals-routes.ts`. The population rule now reads the registrar LIST + // rather than naming each module, so the slice that moves the next body + // widens it by adding its module there; each module keeps its own control + // and the list's length is one too. `this.routeManager.register(` stays 63 + // over the four files (42 here + 9 + 5 + 7), `private register*Endpoints(` + // stays 16, and `enforceAuth` 68 -> 72 is RE-ANCHORED, not relaxed: the + // seven call sites moved with their routes and are counted in both + // readings; the four new occurrences are this slice's seam, the same two + // spellings (the host literal's forwarder, two; the host interface's + // member, two). No call site was added. controls: { 'private register*Endpoints(': 16, 'this.routeManager.register(': 63, + 'registrar list length': 4, 'data-action-routes.ts: this.routeManager.register(': 9, 'batch-routes.ts: this.routeManager.register(': 5, + 'approvals-routes.ts: this.routeManager.register(': 7, // Both halves of the new rule carry their own control, so neither can go // silently to zero: a helper deleted and its routes inlined back would // still read population 80, and only these two controls would notice the @@ -669,7 +691,7 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ 'registerPerItemRoute(': 8, 'const registerPerItemRoute =': 1, 'forwarder slice: this.routeManager.register(': 1, - enforceAuth: 68, + enforceAuth: 72, }, note: 'The single non-tripwire probe names ONE registrar of 16. The other 15 can never mint a key: ' + @@ -1045,13 +1067,22 @@ export function deriveProbeFileCensus(): { // no route. The moved module defines no registrar (`private register…(`) and // no `registerPerItemRoute`, so appending it moves neither the forwarder // slice nor the `reachable` slice; its own control below proves it is read. - const moved = read('packages/rest/src/data-action-routes.ts'); + // // [#23050 slice 2] The same lesson, again: `registerBatchEndpoints`' body // moved to `batch-routes.ts` with its five registration sites. Appended for // the same reason and on the same terms — it declares no registrar and no // `registerPerItemRoute` either — with its own control below. - const movedBatch = read('packages/rest/src/batch-routes.ts'); - const all = `${src}\n${moved}\n${movedBatch}`; + // + // [#23050 slice 3] …and `registerApprovalsEndpoints`' body to + // `approvals-routes.ts`, with its seven. The modules are no longer named + // here one by one: they are read off THE registrar list, in its order, so + // the slice that moves the next body adds its module there once. Every + // module on it is appended on the same terms and gets its own control + // below, and the list's length is a control too. + const movedModules = REST_SERVER_REGISTRAR_FILES + .filter((file) => file !== 'rest-server.ts') + .map((file) => [file, read(`${REST_SERVER_REGISTRAR_DIR}/${file}`)] as const); + const all = [src, ...movedModules.map(([, text]) => text)].join('\n'); const registrarRe = /^\s*private\s+register[A-Za-z]*Endpoints\s*\(/gm; const mountRe = /this\.routeManager\.register\(/g; // Matches the CALL sites only. The declaration reads @@ -1152,10 +1183,14 @@ export function deriveProbeFileCensus(): { controls: { 'private register*Endpoints(': occurrences(src, /private\s+register[A-Za-z]*Endpoints\s*\(/g), 'this.routeManager.register(': occurrences(all, /this\.routeManager\.register\(/g), + // [#23050 slice 3] The registrar list's length, so a module added to it + // makes this row re-read its numbers. + 'registrar list length': REST_SERVER_REGISTRAR_FILES.length, // [#23050 slice 1] The moved module's own share, so a reading that // silently stopped appending it cannot hide behind the total above. - 'data-action-routes.ts: this.routeManager.register(': occurrences(moved, /this\.routeManager\.register\(/g), - 'batch-routes.ts: this.routeManager.register(': occurrences(movedBatch, /this\.routeManager\.register\(/g), + // [#23050 slice 3] One such control per module on the list. + ...Object.fromEntries(movedModules.map(([file, text]) => + [`${file}: this.routeManager.register(`, occurrences(text, /this\.routeManager\.register\(/g)])), 'registerPerItemRoute(': occurrences(src, /registerPerItemRoute\(/g), 'const registerPerItemRoute =': occurrences(src, /const\s+registerPerItemRoute\s*=/g), // ⭐ The one control here that is NOT a whole-file count, deliberately: diff --git a/packages/rest/src/approvals-routes.ts b/packages/rest/src/approvals-routes.ts new file mode 100644 index 00000000000..2c82cc76025 --- /dev/null +++ b/packages/rest/src/approvals-routes.ts @@ -0,0 +1,548 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23050 slice 3] The approval routes of `RestServer` — the request list and + * read, the two decisions (`approve`, `reject`), `recall`, the two flow moves + * (`revise`, `resubmit`), the four thread interactions (`reassign`, `remind`, + * `request-info`, `comment`) and the audit trail — moved out of + * `rest-server.ts` whole. + * + * A MOVE, not a redesign. The function below is the body of + * `RestServer#registerApprovalsEndpoints` as it stood, re-indented one level + * and otherwise unedited: the same routes, registered in the same order, by + * the same handlers, answering the same bodies. `handleApprovalError`'s prefix + * map and the three route factories (`decisionRoute`, `flowMoveRoute`, + * `threadRoute`) moved with it, unchanged. + * + * ## How the moved body reaches the class + * + * The shape slice 1 set (`data-action-routes.ts`' header carries the full + * reasoning), with slice 2's refinement. The class keeps its private + * `registerApprovalsEndpoints` — the published `dist/*.d.ts` lists its name, + * docblock and position — and its body is one {@link ApprovalsRouteHost} + * literal plus `registerApprovalsEndpoints.call(host, basePath)`. A method is + * handed over as a closure that calls it on the server at call time, so a test + * double or spy set on the server is still the one the body reaches. + * + * Each field was checked on its own, not inherited from an earlier slice: + * + * - `routeManager` goes BY VALUE. The body reads it only while registration + * runs — every `register` call, the three route factories' included, since + * each factory is invoked during registration — and the class's one + * `routeManager` reassignment wraps `registerMetadataEndpointsInner`, never + * this registrar. + * - `approvalsServiceProvider` goes as a GETTER. `resolveService` reads it at + * REQUEST time, and the field is not `readonly`, so a value copied at + * registration could part from it. The getter reads the field the handler + * read, when it read it. + * + * Module-level helpers stay where they were and are imported: + * `APPROVAL_REQUEST_LIST_PARAMS`, `UNDECLARED_WHOLE_NUMBER_PARAM` and + * `readDeclaredQueryNumber` from `rest-server.ts`, which already exported all + * three; the rest from the modules `rest-server.ts` imports them from. No + * export was added anywhere, so the published surface does not move. + */ + +// [#13807] The recogniser half of the stranded-decision carrier — see +// `handleApprovalError` below. Constructor and reader share one module in +// `@objectstack/types` because the producer is a PLUGIN and rest cannot +// import one. +import { strandedDecisionDetails, sendError as sendEnvelopeError } from '@objectstack/types'; +import { + classifiedRefusalAnswer, + nestedRefusalEnvelope, + sandboxFaultAnswer, + handleRouteError, +} from './error-response.js'; +import { logError } from './log.js'; +import { refuseRepeatedQueryParams } from './query-multiplicity.js'; +import { refuseUnknownQueryParams } from './query-allowlist.js'; +import { + APPROVAL_REQUEST_LIST_PARAMS, + UNDECLARED_WHOLE_NUMBER_PARAM, + readDeclaredQueryNumber, + type RestServer, +} from './rest-server.js'; + +/** + * The `RestServer` members {@link registerApprovalsEndpoints} reads — every one + * of them, and no other. Each is typed AS the class member (an indexed access + * on the class), so a change to a member's signature on the class is a change + * here too, and naming a member the class does not have is a compile error. + * See the module header for why `approvalsServiceProvider` is a getter. + */ +export interface ApprovalsRouteHost { + readonly routeManager: RestServer['routeManager']; + readonly approvalsServiceProvider: RestServer['approvalsServiceProvider']; + resolveExecCtx: RestServer['resolveExecCtx']; + enforceAuth: RestServer['enforceAuth']; +} + +/** + * Register approval endpoints — `RestServer#registerApprovalsEndpoints`' body, + * called by that method with the server as `this`. The method's own docblock, + * which the published declaration carries, names the routes. + */ +export function registerApprovalsEndpoints(this: ApprovalsRouteHost, basePath: string): void { + // Approval routes live at the top of the API surface (e.g. + // `/api/v1/approvals/requests/:id/approve`). Approvals are a + // cross-cutting capability — a request is not a record on a single + // CRUD object, so anchoring it on `basePath` (instead of + // `${basePath}/data`) keeps the URL semantics honest. + const dataPath = basePath; + const isScoped = basePath.includes('/environments/:environmentId'); + + const resolveService = async (environmentId?: string) => { + if (!this.approvalsServiceProvider) return undefined; + try { return await this.approvalsServiceProvider(environmentId); } + catch { return undefined; } + }; + const respond501 = (res: any) => res.status(501).json({ + code: 'NOT_IMPLEMENTED', + message: 'Approvals service is not configured on this deployment', + }); + const handleApprovalError = (res: any, err: any): boolean => { + const msg = String(err?.message ?? err ?? ''); + const mapping: Array<[RegExp, number, string]> = [ + [/^VALIDATION_FAILED/, 400, 'VALIDATION_FAILED'], + [/^DUPLICATE_REQUEST/, 409, 'DUPLICATE_REQUEST'], + [/^INVALID_STATE/, 409, 'INVALID_STATE'], + [/^THROTTLED/, 429, 'THROTTLED'], + [/^FORBIDDEN/, 403, 'FORBIDDEN'], + [/^REQUEST_NOT_FOUND/, 404, 'REQUEST_NOT_FOUND'], + // #4420 — the request and its flow run disagree about whether + // the work can still proceed. A conflict, like INVALID_STATE: + // the row is fine, the run behind it is not. + [/^RESUME_TARGET_LOST/, 409, 'RESUME_TARGET_LOST'], + // The outcome IS recorded and its run is stranded — a genuine + // server-side inconsistency, but named, so the client can say + // which run needs an operator instead of showing a bare 500. + [/^RESUME_FAILED/, 500, 'RESUME_FAILED'], + // The write IS recorded and NOT rolled back, but the updated + // row is invisible inside the caller's organization scope, so + // the result envelope cannot be built. Same class as + // RESUME_FAILED — a genuine server-side inconsistency, named + // (commit 5b3ff63cc): read the request back with a system or + // matching-organization context. + [/^READ_BACK_FAILED/, 500, 'READ_BACK_FAILED'], + ]; + // [#13807, maintainer ruling 2026-09-04 batch #37] The + // machine-readable half of a stranded decision, when the service + // attached one. The status code does NOT move — a recorded + // decision whose run strands is still a failure and the ruling + // upholds that — but the body stops being prose only: `finalized` + // says the decision stands, `decision` / `runId` name what and + // where, and `repairable` carries the engine's own `'stranded'` + // discriminator through instead of dying at the door. + // + // Before this, a caller reading 500 had exactly one honest move — + // assume the rejection did not happen — and it was the wrong one: + // the row IS terminal and the record's mirrored status HAS moved. + // An operator had to regex the run id out of a sentence. + // + // ⛔ Presence-gated, never synthesised. `strandedDecisionDetails` + // returns `undefined` for any error that did not carry a complete + // envelope, and this then answers exactly the body it always did — + // a `RESUME_FAILED` from a caller with no decision to report must + // not be dressed up as one. + const stranded = strandedDecisionDetails(err); + for (const [re, status, code] of mapping) { + if (re.test(msg)) { + // [#13095] The strip is anchored to the CODE this row just + // answered — the same declared-code anchoring + // `withoutDeclaredCodePrefix` (error-response.ts) and + // `respondSharingError`'s prefix arm apply, converged here + // by the 2026-08-31 ruling. The blanket + // SCREAMING_SNAKE-colon regex that used to sit here + // (`/^[A-Z_]+:\s*/`) is exactly the shape #12975 rejected: + // it could eat a token the wire carries nowhere else (a + // message opening with a DIFFERENT capitalised word and a + // colon), where the anchored form can only ever remove a + // duplicate of the `code` already on the wire. + res.status(status).json({ + code, + error: msg.replace(new RegExp(`^${code}:\\s*`), ''), + // Anchored to the code the envelope describes, for the + // same reason the strip above is: these four facts are + // about a recorded-decision-with-stranded-run and + // about nothing else. + ...(code === 'RESUME_FAILED' && stranded ? stranded : {}), + }); + return true; + } + } + // [#22719] A refusal no prefix above matches is asked of the `/data` + // door's own classification before the caller's outer 500 relays + // `.message`. For a sandboxed hook's refusal `.message` is the + // QuickJS debug wrapper, so all nine write routes here answered + // `500 APPROVAL_*_FAILED` with `hook '' threw: Error: …` where + // every conforming write door answers the hook's own sentence; a + // refusal declaring `status`/`statusCode` + `code` is answered at + // its own status too. + // + // AFTER the mapping, deliberately: every row above answers a code + // this family publishes, and none can read a sandbox error (its + // `.message` opens with the wrapper), so asking here moves only what + // used to reach the 500. Nested envelope, for the reason the + // sharing-rule family's `SHARING_NOT_ENABLED` arm gives: a new arm + // does not copy the flat `{ code, error }` dialect. + const refusal = classifiedRefusalAnswer(err); + if (refusal) { + const nested = nestedRefusalEnvelope(refusal); + sendEnvelopeError(res, nested.status, nested.code, nested.message, nested.extra); + return true; + } + // [#22764] A sandboxed body the doors answer as a FAULT — one that + // crashed, or a refusal that declared a 5xx — answers as the doors + // do rather than reaching the caller's outer `500 + // APPROVAL_*_FAILED`, which relayed the QuickJS wrapper around the + // native error text. Logged here because answering it skips the + // outer arm's log line, and the operator is now the only reader. + const fault = sandboxFaultAnswer(err); + if (fault) { + logError('[REST] approval error:', err); + const nested = nestedRefusalEnvelope(fault); + sendEnvelopeError(res, nested.status, nested.code, nested.message, nested.extra); + return true; + } + return false; + }; + + // ── Requests ────────────────────────────────────────────── + this.routeManager.register({ + method: 'GET', + path: `${dataPath}/approvals/requests`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) { + // No approvals plugin loaded — return empty list rather than 501 + // so Console badge polls don't spam the error log on deployments + // that don't run an approvals workflow. + res.json({ data: [] }); + return; + } + // [#7527] The closed parameter set for this route, measured + // from what the handler below actually reads. A name outside + // it is REFUSED, never dropped: `?assignedToMe=true` used to + // answer 200 with the whole list, and an ignored filter is + // indistinguishable from one that matched everything. Paging + // (`limit`/`offset`) and the snake_case alias spellings are + // in the set because the handler honours them — a whitelist + // built from the filters alone would break paging. + if (refuseUnknownQueryParams(req, res, APPROVAL_REQUEST_LIST_PARAMS)) return; + // [#6877] `approverId` / `approver_id` are the model case + // for the multi-valued side and are therefore NOT listed: + // the block immediately below reads their array arm ON + // PURPOSE. Everything else here narrows the list to one + // value and is declared single-valued. + if (refuseRepeatedQueryParams(req, res, [ + 'object', 'recordId', 'record_id', 'status', + 'submitterId', 'submitter_id', 'q', 'limit', 'offset', + ])) return; + const q = req.query ?? {}; + // `approverId` accepts a single id, a comma-separated + // list, or the param repeated (→ array). Normalise all + // three to a string[] so the Console can resolve "my + // pending approvals" across every identity (user id / + // email / role:) in ONE request rather than looping. + const rawApprover = q.approverId ?? q.approver_id; + const approverIds = (Array.isArray(rawApprover) ? rawApprover : (rawApprover != null ? [rawApprover] : [])) + .flatMap((s: any) => String(s).split(',')) + .map((s: string) => s.trim()) + .filter(Boolean); + // [#20139] Read, not coerced. `?limit=abc` (or `Infinity`) was + // `NaN`, dropped by an `isFinite` guard, and answered with the + // UNPAGED 500-row window and no `total` — a caller asking for a + // page got a different shape of answer; `?offset=abc` was dropped + // to the first page. `?limit=` / `?offset=` became `Number('')` = + // 0: a ONE-row page, or the service's 50-row paged mode the caller + // never asked for — so empty is refused here, not read as absent. + // No request schema is declared for this door, so both read as + // whole numbers; the service's own `[1, 200]` clamp is unchanged. + // + // Caught HERE, not by the catch below: that one answers every + // throw `500 APPROVAL_REQUEST_LIST_FAILED`, and a query value the + // door cannot read is the caller's `400`. + let limit: number | undefined; + let offset: number | undefined; + try { + limit = readDeclaredQueryNumber(q, 'limit', + UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); + offset = readDeclaredQueryNumber(q, 'offset', + UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); + } catch (refusal: any) { + handleRouteError(res, refusal); + return; + } + const listFilter = { + object: q.object, + recordId: q.recordId ?? q.record_id, + status: q.status, + approverId: approverIds.length ? approverIds : undefined, + submitterId: q.submitterId ?? q.submitter_id, + q: typeof q.q === 'string' ? q.q : undefined, + limit, + offset, + }; + const rows = await svc.listRequests(listFilter, context ?? {}); + // `total` only when the caller pages — counting costs a + // second query and unpaged callers don't need it. + if (listFilter.limit != null && typeof svc.countRequests === 'function') { + const total = await svc.countRequests(listFilter, context ?? {}); + res.json({ data: rows, total }); + return; + } + res.json({ data: rows }); + } catch (error: any) { + logError('[REST] List approval requests error:', error); + res.status(500).json({ code: 'APPROVAL_REQUEST_LIST_FAILED', error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: 'List approval requests', tags: ['approvals'] }, + }); + + this.routeManager.register({ + method: 'GET', + path: `${dataPath}/approvals/requests/:id`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const row = await svc.getRequest(req.params.id, context ?? {}); + if (!row) { + res.status(404).json({ code: 'REQUEST_NOT_FOUND', error: `Approval request '${req.params.id}' not found` }); + return; + } + res.json(row); + } catch (error: any) { + logError('[REST] Get approval request error:', error); + res.status(500).json({ code: 'APPROVAL_REQUEST_GET_FAILED', error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: 'Get an approval request by id', tags: ['approvals'] }, + }); + + // Record a decision on a node-driven request. Both branches funnel + // through the contract's `decide()`, which finalizes the request and + // resumes the owning flow run down the matching `approve` / `reject` + // edge. + // + // On the `actorId` these routes forward (#3800): it is a HINT, not the + // acting identity. The service pins the actor to the authenticated + // caller and accepts a body value only when it can prove the caller + // holds that identity — a slot keyed by a `type:value` literal or by + // the caller's email, which the Console legitimately sends. It is + // forwarded rather than dropped for exactly those cases; naming anyone + // else is `FORBIDDEN` at the service, never here. + const decisionRoute = (decision: 'approve' | 'reject') => { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/approvals/requests/:id/${decision}`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const body = req.body ?? {}; + try { + const out = await svc.decide(req.params.id, { + decision, + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + attachments: body.attachments, + // #3447 P2: author-declared decision outputs — the + // service validates keys against the node's + // `decisionOutputs` whitelist before any write. + outputs: body.outputs, + }, context ?? {}); + res.json(out); + } catch (err: any) { + if (handleApprovalError(res, err)) return; + throw err; + } + } catch (error: any) { + logError(`[REST] ${decision} approval error:`, error); + res.status(500).json({ code: `APPROVAL_${decision.toUpperCase()}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: `${decision[0].toUpperCase()}${decision.slice(1)} an approval request`, tags: ['approvals'] }, + }); + }; + decisionRoute('approve'); + decisionRoute('reject'); + + // Recall — submitter withdraws a pending request. Mirrors the decision + // routes' error mapping; the service enforces submitter-only access. + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/approvals/requests/:id/recall`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc || typeof svc.recall !== 'function') return respond501(res); + const body = req.body ?? {}; + try { + const out = await svc.recall(req.params.id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context ?? {}); + res.json(out); + } catch (err: any) { + if (handleApprovalError(res, err)) return; + throw err; + } + } catch (error: any) { + logError('[REST] recall approval error:', error); + res.status(500).json({ code: 'APPROVAL_RECALL_FAILED', error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: 'Recall (withdraw) an approval request', tags: ['approvals'] }, + }); + + // Send back for revision / resubmit (ADR-0044). Both move the flow (the + // request finalizes `returned` and the run parks at a wait point; a + // resubmit re-enters the approval node), so — like recall — they are + // dedicated routes rather than thread interactions. The service enforces + // access (send-back = a pending approver; resubmit = the submitter) and + // returns the flow outcome (`autoRejected` / `resumed`) verbatim. + const flowMoveRoute = ( + action: 'revise' | 'resubmit', + invoke: (svc: any, id: string, body: any, context: any) => Promise, + ) => { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/approvals/requests/:id/${action}`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const body = req.body ?? {}; + try { + const out = await invoke(svc, req.params.id, body, context ?? {}); + res.json(out); + } catch (err: any) { + if (handleApprovalError(res, err)) return; + throw err; + } + } catch (error: any) { + logError(`[REST] ${action} approval error:`, error); + res.status(500).json({ code: `APPROVAL_${action.toUpperCase()}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: `${action} an approval request`, tags: ['approvals'] }, + }); + }; + flowMoveRoute('revise', (svc, id, body, context) => { + if (typeof svc.sendBack !== 'function') throw new Error('VALIDATION_FAILED: revise is not supported'); + return svc.sendBack(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + flowMoveRoute('resubmit', (svc, id, body, context) => { + if (typeof svc.resubmit !== 'function') throw new Error('VALIDATION_FAILED: resubmit is not supported'); + return svc.resubmit(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + + // Thread interactions — reassign / remind / request-info / comment. + // None of these move the flow; they update approver slots or the + // audit thread. Registered generically: the service method enforces + // the per-action permission (slot holder / submitter / participant). + const threadRoute = ( + action: 'reassign' | 'remind' | 'request-info' | 'comment', + invoke: (svc: any, id: string, body: any, context: any) => Promise, + ) => { + this.routeManager.register({ + method: 'POST', + path: `${dataPath}/approvals/requests/:id/${action}`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const body = req.body ?? {}; + try { + const out = await invoke(svc, req.params.id, body, context ?? {}); + res.json(out); + } catch (err: any) { + if (handleApprovalError(res, err)) return; + throw err; + } + } catch (error: any) { + logError(`[REST] ${action} approval error:`, error); + res.status(500).json({ code: `APPROVAL_${action.toUpperCase().replace('-', '_')}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: `${action} on an approval request`, tags: ['approvals'] }, + }); + }; + threadRoute('reassign', (svc, id, body, context) => { + if (typeof svc.reassign !== 'function') throw new Error('VALIDATION_FAILED: reassign is not supported'); + return svc.reassign(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + to: body.to, from: body.from, comment: body.comment, + }, context); + }); + threadRoute('remind', (svc, id, body, context) => { + if (typeof svc.remind !== 'function') throw new Error('VALIDATION_FAILED: remind is not supported'); + return svc.remind(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + threadRoute('request-info', (svc, id, body, context) => { + if (typeof svc.requestInfo !== 'function') throw new Error('VALIDATION_FAILED: request-info is not supported'); + return svc.requestInfo(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + }, context); + }); + threadRoute('comment', (svc, id, body, context) => { + if (typeof svc.comment !== 'function') throw new Error('VALIDATION_FAILED: comment is not supported'); + return svc.comment(id, { + actorId: body.actorId ?? body.actor_id ?? context?.userId, + comment: body.comment, + attachments: body.attachments, + }, context); + }); + + this.routeManager.register({ + method: 'GET', + path: `${dataPath}/approvals/requests/:id/actions`, + handler: async (req: any, res: any) => { + try { + const environmentId = isScoped ? req.params?.environmentId : undefined; + const context = await this.resolveExecCtx(environmentId, req); + if (this.enforceAuth(req, res, context)) return; + const svc = await resolveService(environmentId); + if (!svc) return respond501(res); + const rows = await svc.listActions(req.params.id, context ?? {}); + res.json({ data: rows }); + } catch (error: any) { + logError('[REST] List approval actions error:', error); + res.status(500).json({ code: 'APPROVAL_ACTIONS_FAILED', error: String(error?.message ?? error).slice(0, 500) }); + } + }, + metadata: { summary: 'List actions (audit trail) for an approval request', tags: ['approvals'] }, + }); +} diff --git a/packages/rest/src/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 1ab21854cda..9377e256848 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -66,6 +66,7 @@ import { // choice against the registry rather than trusting a literal. import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; import { RestServer } from './rest-server.js'; +import { REST_SERVER_REGISTRAR_FILES } from './rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); @@ -79,8 +80,14 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); * * [#23050 slice 2] `RestServer#registerBatchEndpoints`' body followed, unedited, to * `batch-routes.ts`, and its five sites with it — on the same terms. + * + * [#23050 slice 3] …and `RestServer#registerApprovalsEndpoints`' body to + * `approvals-routes.ts`, with its seven. The files are no longer spelt here: the + * census reads every file on THE list of the files `RestServer`'s registrars + * live in (`rest-server-registrar-files.ts`), so the slice that moves the next + * body widens it by adding its module there. §2 pins the list's length. */ -const SITE_FILES = ['rest-server.ts', 'data-action-routes.ts', 'batch-routes.ts'] as const; +const SITE_FILES = REST_SERVER_REGISTRAR_FILES; const SOURCES = new Map(SITE_FILES.map((file) => [ file, file === 'rest-server.ts' ? SOURCE : readFileSync(resolve(HERE, file), 'utf8'), ])); @@ -88,7 +95,8 @@ const siteKey = (file: string, line: number) => `${file}:${line}`; /** * The frames in `rest-server.ts` that are NOT consumers: the host literal each - * moved registrar (`registerDataActionEndpoints`, `registerBatchEndpoints`) hands + * moved registrar (`registerDataActionEndpoints`, `registerBatchEndpoints`, + * `registerApprovalsEndpoints`) hands * its moved body forwards each member to the server with * `(...a) => this.(...a)`, so a moved site's stack shows that forwarder * above the site itself. The site is the first censused frame that is not one. @@ -100,10 +108,13 @@ const FORWARDER_LINES = SOURCE.split('\n').flatMap((l, i) => l.includes('resolveExecCtx: (...a) => this.resolveExecCtx(...a),') ? [i + 1] : []); const FORWARDERS = new Set(FORWARDER_LINES.map((line) => siteKey('rest-server.ts', line))); +/** A stack frame in any censused file, as `::`. */ +const SITE_FRAME = new RegExp(`(${SITE_FILES.map((file) => file.replace(/\./g, '\\.')).join('|')}):(\\d+):\\d+`, 'g'); + /** The census site a `resolveExecCtx` call came from, read off its stack. */ function siteOfStack(stack: string): string | undefined { - for (const m of stack.matchAll(/(rest-server|data-action-routes|batch-routes)\.ts:(\d+):\d+/g)) { - const key = siteKey(`${m[1]}.ts`, Number(m[2])); + for (const m of stack.matchAll(SITE_FRAME)) { + const key = siteKey(m[1], Number(m[2])); if (!FORWARDERS.has(key)) return key; } return undefined; @@ -547,8 +558,16 @@ describe('[#13160] §2 the consumer surface, counted from the tree', () => { // (its host literal's forwarder, two; its host interface's member, two). // There are now two forwarders, one per moved module, and the control // pins that pairing. + // + // [#23050 slice 3] 62 sites UNCHANGED / 85 → 89 mentions — the same MOVE. + // `registerApprovalsEndpoints`' body left for `approvals-routes.ts` with + // its seven BARE sites: 41 + 9 + 5 + 7 = 62 sites, read off the registrar + // list, whose length is pinned here. The four new mentions are this + // slice's seam, the same two spellings; three forwarders now, one per + // moved module. + expect(SITE_FILES).toHaveLength(4); expect(SITES.length).toBe(62); - expect([...SOURCES.values()].reduce((n, text) => n + text.split('resolveExecCtx').length - 1, 0)).toBe(85); + expect([...SOURCES.values()].reduce((n, text) => n + text.split('resolveExecCtx').length - 1, 0)).toBe(89); expect(FORWARDER_LINES, 'one host-literal resolveExecCtx forwarder per moved module: one was respelt or removed, or a module moved without joining SITE_FILES') .toHaveLength(SITE_FILES.length - 1); }); diff --git a/packages/rest/src/meta-draft-read-door-census.test.ts b/packages/rest/src/meta-draft-read-door-census.test.ts index c837cb40e69..845a11d7bf7 100644 --- a/packages/rest/src/meta-draft-read-door-census.test.ts +++ b/packages/rest/src/meta-draft-read-door-census.test.ts @@ -42,6 +42,14 @@ * runtime dispatcher's `/meta` door asks too, after its write-capability gate. * Its behaviour is held by `meta-compound-save-mode-parity.test.ts` and by both * doors' rows in `packages/runtime/src/domains/meta-save-preconditions-parity.test.ts`. + * + * [#23050 slice 3] The walk covers every file on THE list of the files + * `RestServer`'s registrars live in (`rest-server-registrar-files.ts`), not + * `rest-server.ts` alone: the split moves registrar bodies into modules of + * their own, and a door that reads a draft is no less a door there. A site's + * key names its route or function, never its file, so the ledger is the same + * ledger. The node helpers below read each node's OWN source file for that + * reason. The census's last case pins the list's length and its sum over it. */ import { describe, it, expect } from 'vitest'; @@ -49,19 +57,22 @@ import { readFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import ts from 'typescript'; +import { REST_SERVER_REGISTRAR_FILES } from './rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const SOURCE_FILE = 'rest-server.ts'; const sf = ts.createSourceFile( SOURCE_FILE, readFileSync(resolve(HERE, SOURCE_FILE), 'utf8'), ts.ScriptTarget.Latest, true, ts.ScriptKind.TS, ); +const SOURCE_FILES: readonly ts.SourceFile[] = REST_SERVER_REGISTRAR_FILES.map((file) => (file === SOURCE_FILE ? sf + : ts.createSourceFile(file, readFileSync(resolve(HERE, file), 'utf8'), ts.ScriptTarget.Latest, true, ts.ScriptKind.TS))); const collapse = (text: string): string => text.replace(/\s+/g, ' ').trim(); const unquote = (text: string): string => text.replace(/^[`'"]|[`'"]$/g, ''); function propertyInitializer(literal: ts.ObjectLiteralExpression, name: string): ts.Expression | undefined { for (const property of literal.properties) { - if (ts.isPropertyAssignment(property) && property.name.getText(sf) === name) return property.initializer; + if (ts.isPropertyAssignment(property) && property.name.getText() === name) return property.initializer; } return undefined; } @@ -73,11 +84,11 @@ function anchorOf(node: ts.Node): string { const method = propertyInitializer(at, 'method'); const path = propertyInitializer(at, 'path'); if (method && path && propertyInitializer(at, 'handler')) { - return `${unquote(method.getText(sf))} ${unquote(path.getText(sf))}`; + return `${unquote(method.getText())} ${unquote(path.getText())}`; } } if ((ts.isFunctionDeclaration(at) || ts.isMethodDeclaration(at)) && at.name) { - return `${ts.isFunctionDeclaration(at) ? 'function' : 'method'} ${at.name.getText(sf)}`; + return `${ts.isFunctionDeclaration(at) ? 'function' : 'method'} ${at.name.getText()}`; } } return 'module scope'; @@ -90,7 +101,7 @@ const isDraftSite = (n: ts.Node): boolean => || (ts.isPropertyAccessExpression(n) && ['previewDrafts', 'listDrafts'].includes(n.name.text)); const isAdmission = (n: ts.Node): boolean => - ts.isCallExpression(n) && n.expression.getText(sf) === 'mayReadPendingDrafts'; + ts.isCallExpression(n) && n.expression.getText() === 'mayReadPendingDrafts'; function contains(root: ts.Node, pred: (n: ts.Node) => boolean): boolean { let hit = false; @@ -116,7 +127,7 @@ function admittedInDeclaration(site: ts.Node): boolean { function behindRefusal(site: ts.Node): boolean { for (let child: ts.Node = site, at = site.parent; at; child = at, at = at.parent) { if (!ts.isBlock(at)) continue; - const before = at.statements.filter((s) => s.end <= child.getStart(sf)); + const before = at.statements.filter((s) => s.end <= child.getStart()); if (before.some((s) => ts.isIfStatement(s) && ts.isPrefixUnaryExpression(s.expression) && s.expression.operator === ts.SyntaxKind.ExclamationToken && isAdmission(s.expression.operand))) return true; @@ -124,19 +135,22 @@ function behindRefusal(site: ts.Node): boolean { return false; } -interface Site { readonly key: string; readonly node: ts.Node; readonly line: number } +interface Site { readonly key: string; readonly node: ts.Node; readonly file: string; readonly line: number } const SITES: Site[] = []; -const visit = (n: ts.Node): void => { - if (isDraftSite(n)) { - SITES.push({ - key: `${anchorOf(n)} » ${collapse(n.getText(sf))}`, - node: n, - line: sf.getLineAndCharacterOfPosition(n.getStart(sf)).line + 1, - }); - } - ts.forEachChild(n, visit); -}; -visit(sf); +for (const file of SOURCE_FILES) { + const visit = (n: ts.Node): void => { + if (isDraftSite(n)) { + SITES.push({ + key: `${anchorOf(n)} » ${collapse(n.getText(file))}`, + node: n, + file: file.fileName, + line: file.getLineAndCharacterOfPosition(n.getStart(file)).line + 1, + }); + } + ts.forEachChild(n, visit); + }; + visit(file); +} // ───────────────────────────────────────────────────────────────────────────── // The ledger — every draft switch in `rest-server.ts`, classified @@ -202,14 +216,14 @@ describe('[#20338] census: every draft switch in rest-server.ts is ledgered', () for (const site of SITES) { const row = LEDGER.find((r) => r.site === site.key); if (row?.disposition !== 'read') continue; - expect(admittedInDeclaration(site.node), `${SOURCE_FILE}:${site.line} ${row.door}`).toBe(true); + expect(admittedInDeclaration(site.node), `${site.file}:${site.line} ${row.door}`).toBe(true); } }); it('the listing sits behind its refusal', () => { const listing = SITES.filter((s) => LEDGER.find((r) => r.site === s.key)?.disposition === 'listing'); expect(listing.length).toBeGreaterThan(0); - for (const site of listing) expect(behindRefusal(site.node), `${SOURCE_FILE}:${site.line}`).toBe(true); + for (const site of listing) expect(behindRefusal(site.node), `${site.file}:${site.line}`).toBe(true); }); it('mayReadPendingDrafts is a bare delegation to isObjectSchemaMaskExempt — the one predicate, never a second rule', () => { @@ -221,4 +235,14 @@ describe('[#20338] census: every draft switch in rest-server.ts is ledgered', () expect(collapse(fns[0].body?.statements.map((s) => s.getText(sf)).join(' ') ?? '')) .toBe(`return isObjectSchemaMaskExempt(${param.name.getText(sf)});`); }); + + it('[#23050 slice 3] walks every file on the registrar list — its length and this sum are pinned', () => { + // A module added to the list makes this re-read its number rather than + // widening unnoticed. 8 switches, all in `rest-server.ts` (the `/meta` + // and analytics doors); the data-action, batch and approval bodies read + // no draft. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); + expect(SOURCE_FILES.map((file) => file.fileName)).toEqual([...REST_SERVER_REGISTRAR_FILES]); + expect(SITES).toHaveLength(8); + }); }); diff --git a/packages/rest/src/rest-approvals-wire-codes.test.ts b/packages/rest/src/rest-approvals-wire-codes.test.ts index b43a6d350a1..784551b1371 100644 --- a/packages/rest/src/rest-approvals-wire-codes.test.ts +++ b/packages/rest/src/rest-approvals-wire-codes.test.ts @@ -9,7 +9,8 @@ * family and all with the same cause: the approvals route factories spell the * terminal 500 catch's code as a TEMPLATE * (`` `APPROVAL_${action.toUpperCase()}_FAILED` `` — `decisionRoute`, - * `flowMoveRoute`, `threadRoute` in `rest-server.ts`), so the literal-grep + * `flowMoveRoute`, `threadRoute`, then in `rest-server.ts` and since #23050 + * slice 3 in `approvals-routes.ts`), so the literal-grep * pass that registered their literal-spelled siblings * (`APPROVAL_RECALL_FAILED`, `APPROVAL_ACTIONS_FAILED`, …) never saw them; the * ninth, `THROTTLED`, is a single-word code the multi-token sweep shape diff --git a/packages/rest/src/rest-server-canonical-query-ast.test.ts b/packages/rest/src/rest-server-canonical-query-ast.test.ts index 11dbe155dd8..b87ba836d33 100644 --- a/packages/rest/src/rest-server-canonical-query-ast.test.ts +++ b/packages/rest/src/rest-server-canonical-query-ast.test.ts @@ -84,17 +84,21 @@ import { dirname, resolve } from 'node:path'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import type { FindDataRequest } from '@objectstack/spec/api'; import { RestServer } from './rest-server.js'; +import { REST_SERVER_REGISTRAR_FILES, type RestServerRegistrarFile } from './rest-server-registrar-files.js'; import type { ImportProtocolLike } from '@objectstack/core'; const HERE = dirname(fileURLToPath(import.meta.url)); const sourceOf = (file: string) => readFileSync(resolve(HERE, file), 'utf8'); -const REST_SERVER = sourceOf('rest-server.ts'); +// [#23050 slice 3] Every file on THE list of the files `RestServer`'s +// registrars live in, imported rather than spelt here, so a registrar body the +// split moves next stays inside this census. +const REGISTRAR_SOURCES = new Map(REST_SERVER_REGISTRAR_FILES.map((file) => [file, sourceOf(file)])); // [#23050 slice 1] `RestServer#registerDataActionEndpoints`' body — the // import-job read, the import-job listing and the export chunk loop — moved to // its own module. Its literals are the same server-built literals, so the // census follows them there rather than losing them. -const DATA_ACTION_ROUTES = sourceOf('data-action-routes.ts'); +const DATA_ACTION_ROUTES = REGISTRAR_SOURCES.get('data-action-routes.ts')!; // [#20919] The runner moved to `@objectstack/core`, beside `bulkWrite`. const IMPORT_RUNNER = readFileSync(resolve(HERE, '../../core/src/utils/import-runner.ts'), 'utf8'); @@ -116,6 +120,20 @@ interface CensusEntry { noDoor: boolean; } +/** + * [#23050 slice 3] Each registrar file's floor, its own measured `querySlots` + * count: a file the split adds to the list needs a floor here before this + * census reads it. The §1 controls pin the list's length and the census's sum + * over it (4 = 1 + 3 + 0 + 0; the batch and approval bodies build no `query:` + * literal). + */ +const REGISTRAR_QUERY_SLOT_FLOORS: Readonly> = { + 'rest-server.ts': 1, + 'data-action-routes.ts': 3, + 'batch-routes.ts': 0, + 'approvals-routes.ts': 0, +}; + const CENSUS: CensusEntry[] = [ // [#21180] 5 -> 4: the anonymous lookup picker's `pickerRequest` literal // left with its route. Re-derived with `querySlots` itself: 5 slots on the @@ -129,8 +147,12 @@ const CENSUS: CensusEntry[] = [ // parses `filter` / `orderby` off the caller's querystring too, so it is a // door). Re-derived with `querySlots`: 1 slot left here (`req.query`), 3 // there, the same 4 in all. Each floor equals its file's measured count. - { file: 'rest-server.ts', source: REST_SERVER, minQuerySlots: 1, noDoor: false }, - { file: 'data-action-routes.ts', source: DATA_ACTION_ROUTES, minQuerySlots: 3, noDoor: false }, + // + // [#23050 slice 3] The registrar files are read off the list, each with its + // floor from `REGISTRAR_QUERY_SLOT_FLOORS` above. + ...REST_SERVER_REGISTRAR_FILES.map((file): CensusEntry => ({ + file, source: REGISTRAR_SOURCES.get(file)!, minQuerySlots: REGISTRAR_QUERY_SLOT_FLOORS[file], noDoor: false, + })), { file: 'import-runner.ts', source: IMPORT_RUNNER, minQuerySlots: 3, noDoor: true }, ]; @@ -295,6 +317,16 @@ describe('[#16638] §1 CONTROLS on the census instrument itself', () => { expect(wireKeysAnywhere('const a = { where: filter, limit: 2 };')).toEqual([]); }); + it('[#23050 slice 3] censuses every file on the registrar list — its length and the slot sum are pinned', () => { + // A module added to the list makes this re-read its number rather than + // widening unnoticed. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); + expect(Object.keys(REGISTRAR_QUERY_SLOT_FLOORS).sort()).toEqual([...REST_SERVER_REGISTRAR_FILES].sort()); + expect(CENSUS.filter(({ file }) => REGISTRAR_SOURCES.has(file)).map(({ file }) => file)) + .toEqual([...REST_SERVER_REGISTRAR_FILES]); + expect([...REGISTRAR_SOURCES.values()].reduce((n, source) => n + querySlots(source).length, 0)).toBe(4); + }); + it('dropping comment lines leaves the code that is being censused', () => { // The stripper is the one step that could silently empty the input. const stripped = withoutCommentLines(IMPORT_RUNNER); diff --git a/packages/rest/src/rest-server-docblock-position.test.ts b/packages/rest/src/rest-server-docblock-position.test.ts index e0cffa4c4c4..2793b4e1975 100644 --- a/packages/rest/src/rest-server-docblock-position.test.ts +++ b/packages/rest/src/rest-server-docblock-position.test.ts @@ -55,6 +55,7 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; +import { REST_SERVER_REGISTRAR_FILES, type RestServerRegistrarFile } from './rest-server-registrar-files.js'; const FILE = 'rest-server.ts'; const SOURCE = readFileSync(new URL('./rest-server.ts', import.meta.url), 'utf8'); @@ -133,10 +134,23 @@ describe('rest-server.ts docblock position', () => { * docblock), so it is read under the same rule rather than leaving that block * outside every pin. Its floor is its own measured count: the module header, * the moved alias's block, the host interface's and the function's. + * + * [#23050 slice 3] Every module on THE list of the files `RestServer`'s + * registrars live in is read under the same rule, imported rather than spelt + * here, so a registrar body the split moves next stays inside this pin. Each + * floor is that module's own measured count — the batch and approval modules + * carry three apiece (the module header, the host interface's block and the + * function's) — and a module added to the list needs one here before it reads. + * The case below the loop pins the list's length and the sum of the floors. */ -const MOVED_FILES: ReadonlyArray<{ file: string; minBlocks: number }> = [ - { file: 'data-action-routes.ts', minBlocks: 4 }, -]; +const MOVED_FILE_FLOORS: Readonly, number>> = { + 'data-action-routes.ts': 4, + 'batch-routes.ts': 3, + 'approvals-routes.ts': 3, +}; +const MOVED_FILES: ReadonlyArray<{ file: string; minBlocks: number }> = REST_SERVER_REGISTRAR_FILES + .filter((file): file is Exclude => file !== FILE) + .map((file) => ({ file, minBlocks: MOVED_FILE_FLOORS[file] })); for (const { file, minBlocks } of MOVED_FILES) { describe(`${file} docblock position (moved out of ${FILE})`, () => { @@ -155,3 +169,14 @@ for (const { file, minBlocks } of MOVED_FILES) { }); }); } + +describe('[#23050 slice 3] the registrar list this pin reads', () => { + it('reads every module on it, each against its own floor — the length and the floors\' sum are pinned', () => { + // A module added to the list makes this re-read its numbers rather than + // widening unnoticed: 4 + 3 + 3 = 10 blocks over the three modules. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); + expect(Object.keys(MOVED_FILE_FLOORS).sort()) + .toEqual(REST_SERVER_REGISTRAR_FILES.filter((file) => file !== FILE).sort()); + expect(MOVED_FILES.reduce((n, { minBlocks }) => n + minBlocks, 0)).toBe(10); + }); +}); diff --git a/packages/rest/src/rest-server-query-number-census.test.ts b/packages/rest/src/rest-server-query-number-census.test.ts index 09ac5dbb27e..e6e770ca939 100644 --- a/packages/rest/src/rest-server-query-number-census.test.ts +++ b/packages/rest/src/rest-server-query-number-census.test.ts @@ -60,6 +60,7 @@ import { fileURLToPath } from 'node:url'; import ts from 'typescript'; // `.js` on purpose — NodeNext resolution requires the extension. import { RestServer } from './rest-server.js'; +import { REST_SERVER_REGISTRAR_FILES } from './rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const SOURCE_FILE = 'rest-server.ts'; @@ -68,11 +69,12 @@ const SOURCE = readFileSync(resolve(HERE, SOURCE_FILE), 'utf8'); // own module, carrying the export door and its `page` exemption with it. The // census reads both files as one population — a site's key is its route or // function, never its file, so the ledger below did not change a row. -const MOVED_FILES = ['data-action-routes.ts'] as const; -const SOURCES: ReadonlyArray = [ - [SOURCE_FILE, SOURCE], - ...MOVED_FILES.map((file) => [file, readFileSync(resolve(HERE, file), 'utf8')] as const), -]; +// [#23050 slice 3] …and now every file on THE list of the files `RestServer`'s +// registrars live in, imported rather than spelt here, so a registrar body the +// split moves next stays inside this census. §2's last case pins the list's +// length and the census's sum over it. +const SOURCES: ReadonlyArray = REST_SERVER_REGISTRAR_FILES.map((file) => + [file, file === SOURCE_FILE ? SOURCE : readFileSync(resolve(HERE, file), 'utf8')] as const); // ───────────────────────────────────────────────────────────────────────────── // The walker @@ -310,6 +312,16 @@ describe('#20139 census — every numeric coercion in rest-server.ts is classifi expect(keys.has(row.site), row.site).toBe(true); } }); + + it('[#23050 slice 3] reads every file on the registrar list — its length and this sum are pinned', () => { + // A module added to the list makes this re-read its number rather than + // widening unnoticed. 9 sites = 8 in `rest-server.ts` + 1 in + // `data-action-routes.ts` (the export door's `page`); the batch and + // approval bodies carry none. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); + expect(SOURCES.map(([file]) => file)).toEqual([...REST_SERVER_REGISTRAR_FILES]); + expect(sites).toHaveLength(9); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/rest/src/rest-server-registrar-files.ts b/packages/rest/src/rest-server-registrar-files.ts new file mode 100644 index 00000000000..47e9f68380b --- /dev/null +++ b/packages/rest/src/rest-server-registrar-files.ts @@ -0,0 +1,47 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23050 slice 3] THE list of the files `RestServer`'s registrars live in: + * `rest-server.ts`, plus each module a registrar's body moved to when the split + * of that file took it out. + * + * Test layer only — nothing in `src/index.ts` reaches it, so tsup (entry: + * `src/index.ts`) never emits it into `dist` and it is not published. Same + * placement, and for the same reason, as `src/http-request-test-builder.ts`. + * + * ## Why one list + * + * A whole-file reader of `rest-server.ts` (a census that counts a spelling + * over the file, a structural pin, a tripwire) reads the code `RestServer`'s + * registrars run. Each slice of the split moves one registrar's body to its + * own module, and a reader that never learns of that module stops seeing + * whatever is written there later: still green, and reading less than it says. + * Those readers import THIS list and read every file on it, so the slice that + * moves the next body adds its module here once, and every reader widens with + * it. + * + * Each reader keeps its own sum over the list and pins the list's `length`, so + * a module added here makes every one of them re-read its number instead of + * widening unnoticed. ⛔ Not a gate: nothing runs this list on its own, and a + * reader that imports it stays the authority on what it counts. + * + * Names are relative to this directory, which every file on the list lives in. + */ +export const REST_SERVER_REGISTRAR_FILES = [ + 'rest-server.ts', + // [#23050 slice 1] `registerDataActionEndpoints`' body. + 'data-action-routes.ts', + // [#23050 slice 2] `registerBatchEndpoints`' body. + 'batch-routes.ts', + // [#23050 slice 3] `registerApprovalsEndpoints`' body. + 'approvals-routes.ts', +] as const; + +/** One file on {@link REST_SERVER_REGISTRAR_FILES}. */ +export type RestServerRegistrarFile = (typeof REST_SERVER_REGISTRAR_FILES)[number]; + +/** + * The repo-relative directory of {@link REST_SERVER_REGISTRAR_FILES}, for a + * reader outside this package (the dogfood suite reads them by repo path). + */ +export const REST_SERVER_REGISTRAR_DIR = 'packages/rest/src'; diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index bee3d9b7bf5..da10d3841b4 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -34,11 +34,8 @@ import { looksLikeInternalErrorLeak, declaresServerFault, INTERNAL_ERROR_MESSAGE, - // [#13807] The recogniser half of the stranded-decision carrier — see - // `handleApprovalError` below. Constructor and reader share one module in - // `@objectstack/types` because the producer is a PLUGIN and rest cannot - // import one. - strandedDecisionDetails, + // `strandedDecisionDetails`, the stranded-decision carrier's recogniser + // half, moved with `handleApprovalError` to `approvals-routes.ts` (#23050 slice 3). // [#20061] The thrown `VALIDATION_FAILED` + `fields[]` shape every catch in // this file already maps to `400` — see `readDeclaredQueryNumber` below. validationFailure, @@ -319,6 +316,8 @@ import { import { registerDataActionEndpoints, type DataActionRouteHost, type ServerScopedDataRequest } from './data-action-routes.js'; // [#23050 slice 2] The batch routes' body moved to its own module — see its header. import { registerBatchEndpoints, type BatchRouteHost } from './batch-routes.js'; +// [#23050 slice 3] The approval routes' body moved to its own module — see its header. +import { registerApprovalsEndpoints, type ApprovalsRouteHost } from './approvals-routes.js'; // [#17551] The `POST …/analytics/dataset/query` door parse — the half of the // analytics family this route never had. See the module header for the // measurement that decides its shape. @@ -11884,467 +11883,18 @@ export class RestServer { * without `@objectstack/plugin-approvals` fail cleanly. */ private registerApprovalsEndpoints(basePath: string): void { - // Approval routes live at the top of the API surface (e.g. - // `/api/v1/approvals/requests/:id/approve`). Approvals are a - // cross-cutting capability — a request is not a record on a single - // CRUD object, so anchoring it on `basePath` (instead of - // `${basePath}/data`) keeps the URL semantics honest. - const dataPath = basePath; - const isScoped = basePath.includes('/environments/:environmentId'); - - const resolveService = async (environmentId?: string) => { - if (!this.approvalsServiceProvider) return undefined; - try { return await this.approvalsServiceProvider(environmentId); } - catch { return undefined; } - }; - const respond501 = (res: any) => res.status(501).json({ - code: 'NOT_IMPLEMENTED', - message: 'Approvals service is not configured on this deployment', - }); - const handleApprovalError = (res: any, err: any): boolean => { - const msg = String(err?.message ?? err ?? ''); - const mapping: Array<[RegExp, number, string]> = [ - [/^VALIDATION_FAILED/, 400, 'VALIDATION_FAILED'], - [/^DUPLICATE_REQUEST/, 409, 'DUPLICATE_REQUEST'], - [/^INVALID_STATE/, 409, 'INVALID_STATE'], - [/^THROTTLED/, 429, 'THROTTLED'], - [/^FORBIDDEN/, 403, 'FORBIDDEN'], - [/^REQUEST_NOT_FOUND/, 404, 'REQUEST_NOT_FOUND'], - // #4420 — the request and its flow run disagree about whether - // the work can still proceed. A conflict, like INVALID_STATE: - // the row is fine, the run behind it is not. - [/^RESUME_TARGET_LOST/, 409, 'RESUME_TARGET_LOST'], - // The outcome IS recorded and its run is stranded — a genuine - // server-side inconsistency, but named, so the client can say - // which run needs an operator instead of showing a bare 500. - [/^RESUME_FAILED/, 500, 'RESUME_FAILED'], - // The write IS recorded and NOT rolled back, but the updated - // row is invisible inside the caller's organization scope, so - // the result envelope cannot be built. Same class as - // RESUME_FAILED — a genuine server-side inconsistency, named - // (commit 5b3ff63cc): read the request back with a system or - // matching-organization context. - [/^READ_BACK_FAILED/, 500, 'READ_BACK_FAILED'], - ]; - // [#13807, maintainer ruling 2026-09-04 batch #37] The - // machine-readable half of a stranded decision, when the service - // attached one. The status code does NOT move — a recorded - // decision whose run strands is still a failure and the ruling - // upholds that — but the body stops being prose only: `finalized` - // says the decision stands, `decision` / `runId` name what and - // where, and `repairable` carries the engine's own `'stranded'` - // discriminator through instead of dying at the door. - // - // Before this, a caller reading 500 had exactly one honest move — - // assume the rejection did not happen — and it was the wrong one: - // the row IS terminal and the record's mirrored status HAS moved. - // An operator had to regex the run id out of a sentence. - // - // ⛔ Presence-gated, never synthesised. `strandedDecisionDetails` - // returns `undefined` for any error that did not carry a complete - // envelope, and this then answers exactly the body it always did — - // a `RESUME_FAILED` from a caller with no decision to report must - // not be dressed up as one. - const stranded = strandedDecisionDetails(err); - for (const [re, status, code] of mapping) { - if (re.test(msg)) { - // [#13095] The strip is anchored to the CODE this row just - // answered — the same declared-code anchoring - // `withoutDeclaredCodePrefix` (error-response.ts) and - // `respondSharingError`'s prefix arm apply, converged here - // by the 2026-08-31 ruling. The blanket - // SCREAMING_SNAKE-colon regex that used to sit here - // (`/^[A-Z_]+:\s*/`) is exactly the shape #12975 rejected: - // it could eat a token the wire carries nowhere else (a - // message opening with a DIFFERENT capitalised word and a - // colon), where the anchored form can only ever remove a - // duplicate of the `code` already on the wire. - res.status(status).json({ - code, - error: msg.replace(new RegExp(`^${code}:\\s*`), ''), - // Anchored to the code the envelope describes, for the - // same reason the strip above is: these four facts are - // about a recorded-decision-with-stranded-run and - // about nothing else. - ...(code === 'RESUME_FAILED' && stranded ? stranded : {}), - }); - return true; - } - } - // [#22719] A refusal no prefix above matches is asked of the `/data` - // door's own classification before the caller's outer 500 relays - // `.message`. For a sandboxed hook's refusal `.message` is the - // QuickJS debug wrapper, so all nine write routes here answered - // `500 APPROVAL_*_FAILED` with `hook '' threw: Error: …` where - // every conforming write door answers the hook's own sentence; a - // refusal declaring `status`/`statusCode` + `code` is answered at - // its own status too. - // - // AFTER the mapping, deliberately: every row above answers a code - // this family publishes, and none can read a sandbox error (its - // `.message` opens with the wrapper), so asking here moves only what - // used to reach the 500. Nested envelope, for the reason the - // sharing-rule family's `SHARING_NOT_ENABLED` arm gives: a new arm - // does not copy the flat `{ code, error }` dialect. - const refusal = classifiedRefusalAnswer(err); - if (refusal) { - const nested = nestedRefusalEnvelope(refusal); - sendEnvelopeError(res, nested.status, nested.code, nested.message, nested.extra); - return true; - } - // [#22764] A sandboxed body the doors answer as a FAULT — one that - // crashed, or a refusal that declared a 5xx — answers as the doors - // do rather than reaching the caller's outer `500 - // APPROVAL_*_FAILED`, which relayed the QuickJS wrapper around the - // native error text. Logged here because answering it skips the - // outer arm's log line, and the operator is now the only reader. - const fault = sandboxFaultAnswer(err); - if (fault) { - logError('[REST] approval error:', err); - const nested = nestedRefusalEnvelope(fault); - sendEnvelopeError(res, nested.status, nested.code, nested.message, nested.extra); - return true; - } - return false; - }; - - // ── Requests ────────────────────────────────────────────── - this.routeManager.register({ - method: 'GET', - path: `${dataPath}/approvals/requests`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) { - // No approvals plugin loaded — return empty list rather than 501 - // so Console badge polls don't spam the error log on deployments - // that don't run an approvals workflow. - res.json({ data: [] }); - return; - } - // [#7527] The closed parameter set for this route, measured - // from what the handler below actually reads. A name outside - // it is REFUSED, never dropped: `?assignedToMe=true` used to - // answer 200 with the whole list, and an ignored filter is - // indistinguishable from one that matched everything. Paging - // (`limit`/`offset`) and the snake_case alias spellings are - // in the set because the handler honours them — a whitelist - // built from the filters alone would break paging. - if (refuseUnknownQueryParams(req, res, APPROVAL_REQUEST_LIST_PARAMS)) return; - // [#6877] `approverId` / `approver_id` are the model case - // for the multi-valued side and are therefore NOT listed: - // the block immediately below reads their array arm ON - // PURPOSE. Everything else here narrows the list to one - // value and is declared single-valued. - if (refuseRepeatedQueryParams(req, res, [ - 'object', 'recordId', 'record_id', 'status', - 'submitterId', 'submitter_id', 'q', 'limit', 'offset', - ])) return; - const q = req.query ?? {}; - // `approverId` accepts a single id, a comma-separated - // list, or the param repeated (→ array). Normalise all - // three to a string[] so the Console can resolve "my - // pending approvals" across every identity (user id / - // email / role:) in ONE request rather than looping. - const rawApprover = q.approverId ?? q.approver_id; - const approverIds = (Array.isArray(rawApprover) ? rawApprover : (rawApprover != null ? [rawApprover] : [])) - .flatMap((s: any) => String(s).split(',')) - .map((s: string) => s.trim()) - .filter(Boolean); - // [#20139] Read, not coerced. `?limit=abc` (or `Infinity`) was - // `NaN`, dropped by an `isFinite` guard, and answered with the - // UNPAGED 500-row window and no `total` — a caller asking for a - // page got a different shape of answer; `?offset=abc` was dropped - // to the first page. `?limit=` / `?offset=` became `Number('')` = - // 0: a ONE-row page, or the service's 50-row paged mode the caller - // never asked for — so empty is refused here, not read as absent. - // No request schema is declared for this door, so both read as - // whole numbers; the service's own `[1, 200]` clamp is unchanged. - // - // Caught HERE, not by the catch below: that one answers every - // throw `500 APPROVAL_REQUEST_LIST_FAILED`, and a query value the - // door cannot read is the caller's `400`. - let limit: number | undefined; - let offset: number | undefined; - try { - limit = readDeclaredQueryNumber(q, 'limit', - UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); - offset = readDeclaredQueryNumber(q, 'offset', - UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); - } catch (refusal: any) { - handleRouteError(res, refusal); - return; - } - const listFilter = { - object: q.object, - recordId: q.recordId ?? q.record_id, - status: q.status, - approverId: approverIds.length ? approverIds : undefined, - submitterId: q.submitterId ?? q.submitter_id, - q: typeof q.q === 'string' ? q.q : undefined, - limit, - offset, - }; - const rows = await svc.listRequests(listFilter, context ?? {}); - // `total` only when the caller pages — counting costs a - // second query and unpaged callers don't need it. - if (listFilter.limit != null && typeof svc.countRequests === 'function') { - const total = await svc.countRequests(listFilter, context ?? {}); - res.json({ data: rows, total }); - return; - } - res.json({ data: rows }); - } catch (error: any) { - logError('[REST] List approval requests error:', error); - res.status(500).json({ code: 'APPROVAL_REQUEST_LIST_FAILED', error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: 'List approval requests', tags: ['approvals'] }, - }); - - this.routeManager.register({ - method: 'GET', - path: `${dataPath}/approvals/requests/:id`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) return respond501(res); - const row = await svc.getRequest(req.params.id, context ?? {}); - if (!row) { - res.status(404).json({ code: 'REQUEST_NOT_FOUND', error: `Approval request '${req.params.id}' not found` }); - return; - } - res.json(row); - } catch (error: any) { - logError('[REST] Get approval request error:', error); - res.status(500).json({ code: 'APPROVAL_REQUEST_GET_FAILED', error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: 'Get an approval request by id', tags: ['approvals'] }, - }); - - // Record a decision on a node-driven request. Both branches funnel - // through the contract's `decide()`, which finalizes the request and - // resumes the owning flow run down the matching `approve` / `reject` - // edge. - // - // On the `actorId` these routes forward (#3800): it is a HINT, not the - // acting identity. The service pins the actor to the authenticated - // caller and accepts a body value only when it can prove the caller - // holds that identity — a slot keyed by a `type:value` literal or by - // the caller's email, which the Console legitimately sends. It is - // forwarded rather than dropped for exactly those cases; naming anyone - // else is `FORBIDDEN` at the service, never here. - const decisionRoute = (decision: 'approve' | 'reject') => { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/approvals/requests/:id/${decision}`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) return respond501(res); - const body = req.body ?? {}; - try { - const out = await svc.decide(req.params.id, { - decision, - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - attachments: body.attachments, - // #3447 P2: author-declared decision outputs — the - // service validates keys against the node's - // `decisionOutputs` whitelist before any write. - outputs: body.outputs, - }, context ?? {}); - res.json(out); - } catch (err: any) { - if (handleApprovalError(res, err)) return; - throw err; - } - } catch (error: any) { - logError(`[REST] ${decision} approval error:`, error); - res.status(500).json({ code: `APPROVAL_${decision.toUpperCase()}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: `${decision[0].toUpperCase()}${decision.slice(1)} an approval request`, tags: ['approvals'] }, - }); - }; - decisionRoute('approve'); - decisionRoute('reject'); - - // Recall — submitter withdraws a pending request. Mirrors the decision - // routes' error mapping; the service enforces submitter-only access. - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/approvals/requests/:id/recall`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc || typeof svc.recall !== 'function') return respond501(res); - const body = req.body ?? {}; - try { - const out = await svc.recall(req.params.id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - }, context ?? {}); - res.json(out); - } catch (err: any) { - if (handleApprovalError(res, err)) return; - throw err; - } - } catch (error: any) { - logError('[REST] recall approval error:', error); - res.status(500).json({ code: 'APPROVAL_RECALL_FAILED', error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: 'Recall (withdraw) an approval request', tags: ['approvals'] }, - }); - - // Send back for revision / resubmit (ADR-0044). Both move the flow (the - // request finalizes `returned` and the run parks at a wait point; a - // resubmit re-enters the approval node), so — like recall — they are - // dedicated routes rather than thread interactions. The service enforces - // access (send-back = a pending approver; resubmit = the submitter) and - // returns the flow outcome (`autoRejected` / `resumed`) verbatim. - const flowMoveRoute = ( - action: 'revise' | 'resubmit', - invoke: (svc: any, id: string, body: any, context: any) => Promise, - ) => { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/approvals/requests/:id/${action}`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) return respond501(res); - const body = req.body ?? {}; - try { - const out = await invoke(svc, req.params.id, body, context ?? {}); - res.json(out); - } catch (err: any) { - if (handleApprovalError(res, err)) return; - throw err; - } - } catch (error: any) { - logError(`[REST] ${action} approval error:`, error); - res.status(500).json({ code: `APPROVAL_${action.toUpperCase()}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: `${action} an approval request`, tags: ['approvals'] }, - }); - }; - flowMoveRoute('revise', (svc, id, body, context) => { - if (typeof svc.sendBack !== 'function') throw new Error('VALIDATION_FAILED: revise is not supported'); - return svc.sendBack(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - }, context); - }); - flowMoveRoute('resubmit', (svc, id, body, context) => { - if (typeof svc.resubmit !== 'function') throw new Error('VALIDATION_FAILED: resubmit is not supported'); - return svc.resubmit(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - }, context); - }); - - // Thread interactions — reassign / remind / request-info / comment. - // None of these move the flow; they update approver slots or the - // audit thread. Registered generically: the service method enforces - // the per-action permission (slot holder / submitter / participant). - const threadRoute = ( - action: 'reassign' | 'remind' | 'request-info' | 'comment', - invoke: (svc: any, id: string, body: any, context: any) => Promise, - ) => { - this.routeManager.register({ - method: 'POST', - path: `${dataPath}/approvals/requests/:id/${action}`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) return respond501(res); - const body = req.body ?? {}; - try { - const out = await invoke(svc, req.params.id, body, context ?? {}); - res.json(out); - } catch (err: any) { - if (handleApprovalError(res, err)) return; - throw err; - } - } catch (error: any) { - logError(`[REST] ${action} approval error:`, error); - res.status(500).json({ code: `APPROVAL_${action.toUpperCase().replace('-', '_')}_FAILED`, error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: `${action} on an approval request`, tags: ['approvals'] }, - }); + // [#23050 slice 3] The body moved to `approvals-routes.ts`. This literal + // hands it every member it reads — the list IS its dependency on this + // class (see `ApprovalsRouteHost`, and that module's header for why + // `approvalsServiceProvider` is a getter). + const readApprovalsServiceProvider = () => this.approvalsServiceProvider; + const host: ApprovalsRouteHost = { + routeManager: this.routeManager, + get approvalsServiceProvider() { return readApprovalsServiceProvider(); }, + resolveExecCtx: (...a) => this.resolveExecCtx(...a), + enforceAuth: (...a) => this.enforceAuth(...a), }; - threadRoute('reassign', (svc, id, body, context) => { - if (typeof svc.reassign !== 'function') throw new Error('VALIDATION_FAILED: reassign is not supported'); - return svc.reassign(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - to: body.to, from: body.from, comment: body.comment, - }, context); - }); - threadRoute('remind', (svc, id, body, context) => { - if (typeof svc.remind !== 'function') throw new Error('VALIDATION_FAILED: remind is not supported'); - return svc.remind(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - }, context); - }); - threadRoute('request-info', (svc, id, body, context) => { - if (typeof svc.requestInfo !== 'function') throw new Error('VALIDATION_FAILED: request-info is not supported'); - return svc.requestInfo(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - }, context); - }); - threadRoute('comment', (svc, id, body, context) => { - if (typeof svc.comment !== 'function') throw new Error('VALIDATION_FAILED: comment is not supported'); - return svc.comment(id, { - actorId: body.actorId ?? body.actor_id ?? context?.userId, - comment: body.comment, - attachments: body.attachments, - }, context); - }); - - this.routeManager.register({ - method: 'GET', - path: `${dataPath}/approvals/requests/:id/actions`, - handler: async (req: any, res: any) => { - try { - const environmentId = isScoped ? req.params?.environmentId : undefined; - const context = await this.resolveExecCtx(environmentId, req); - if (this.enforceAuth(req, res, context)) return; - const svc = await resolveService(environmentId); - if (!svc) return respond501(res); - const rows = await svc.listActions(req.params.id, context ?? {}); - res.json({ data: rows }); - } catch (error: any) { - logError('[REST] List approval actions error:', error); - res.status(500).json({ code: 'APPROVAL_ACTIONS_FAILED', error: String(error?.message ?? error).slice(0, 500) }); - } - }, - metadata: { summary: 'List actions (audit trail) for an approval request', tags: ['approvals'] }, - }); + registerApprovalsEndpoints.call(host, basePath); } /** diff --git a/packages/rest/src/ui-view-route-identity.measurement.test.ts b/packages/rest/src/ui-view-route-identity.measurement.test.ts index bbd8c37f620..c00c85c43c8 100644 --- a/packages/rest/src/ui-view-route-identity.measurement.test.ts +++ b/packages/rest/src/ui-view-route-identity.measurement.test.ts @@ -73,6 +73,7 @@ import { } from '@objectstack/core'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { RestServer } from './rest-server.js'; +import { REST_SERVER_REGISTRAR_FILES } from './rest-server-registrar-files.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); @@ -80,9 +81,13 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); // files RestServer's registrars live in, which since that slice include the // module `registerDataActionEndpoints`' body moved to. Sliced readings of ONE // registrar still read `SOURCE` alone. [#23050 slice 2] …and the module -// `registerBatchEndpoints`' body moved to. -const REGISTRAR_SOURCES = [SOURCE, ...['data-action-routes.ts', 'batch-routes.ts'] - .map((file) => readFileSync(resolve(HERE, file), 'utf8'))].join('\n'); +// `registerBatchEndpoints`' body moved to. [#23050 slice 3] …and every module +// on THE list of the files RestServer's registrars live in, read off that list +// (`rest-server-registrar-files.ts`) rather than named here, so the slice that +// moves the next body widens these checks by adding its module there. The +// reverse checks pin the list's length. +const REGISTRAR_SOURCES = REST_SERVER_REGISTRAR_FILES + .map((file) => (file === 'rest-server.ts' ? SOURCE : readFileSync(resolve(HERE, file), 'utf8'))).join('\n'); const BASE = '/api/v1'; const UI_ROUTE = `GET ${BASE}/ui/view/:object/:type`; @@ -337,6 +342,7 @@ describe('[#13214] §1 the REST seam — absent context vs entitled context', () // ⛔ Reverse-checked: a `true` from a substring test is only worth // something if the term is spelled the way the rest of the file spells // it — 52 sibling sites say it is. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); expect(REGISTRAR_SOURCES.split('this.enforceAuth(').length - 1).toBeGreaterThan(40); expect(REGISTRAR_SOURCES.split('this.resolveExecCtx(').length - 1).toBeGreaterThan(40); }); diff --git a/packages/rest/src/ui-view-route-tenancy.measurement.test.ts b/packages/rest/src/ui-view-route-tenancy.measurement.test.ts index 5909090a990..d0857a030dc 100644 --- a/packages/rest/src/ui-view-route-tenancy.measurement.test.ts +++ b/packages/rest/src/ui-view-route-tenancy.measurement.test.ts @@ -93,6 +93,7 @@ import { dirname, resolve } from 'node:path'; import { ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS } from '@objectstack/core'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { RestServer } from './rest-server.js'; +import { REST_SERVER_REGISTRAR_FILES } from './rest-server-registrar-files.js'; import type { RestEnvRegistry, RestKernelManager, RestRequestEnvResolver } from './rest-server.js'; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -101,9 +102,13 @@ const SOURCE = readFileSync(resolve(HERE, 'rest-server.ts'), 'utf8'); // files RestServer's registrars live in, which since that slice include the // module `registerDataActionEndpoints`' body moved to. Sliced readings of ONE // registrar still read `SOURCE` alone. [#23050 slice 2] …and the module -// `registerBatchEndpoints`' body moved to. -const REGISTRAR_SOURCES = [SOURCE, ...['data-action-routes.ts', 'batch-routes.ts'] - .map((file) => readFileSync(resolve(HERE, file), 'utf8'))].join('\n'); +// `registerBatchEndpoints`' body moved to. [#23050 slice 3] …and every module +// on THE list of the files RestServer's registrars live in, read off that list +// (`rest-server-registrar-files.ts`) rather than named here, so the slice that +// moves the next body widens these checks by adding its module there. The +// reverse checks pin the list's length. +const REGISTRAR_SOURCES = REST_SERVER_REGISTRAR_FILES + .map((file) => (file === 'rest-server.ts' ? SOURCE : readFileSync(resolve(HERE, file), 'utf8'))).join('\n'); const BASE = '/api/v1'; const UI_ROUTE = `GET ${BASE}/ui/view/:object/:type`; @@ -1103,6 +1108,7 @@ describe('[#13214] §4 preconditions', () => { // ⛔ Reverse-checked: the same terms counted over the WHOLE file, so // these readings are about this registrar and not about a misspelling. + expect(REST_SERVER_REGISTRAR_FILES).toHaveLength(4); expect(REGISTRAR_SOURCES.split('this.enforceAuth(').length - 1).toBeGreaterThan(40); expect(REGISTRAR_SOURCES.split('this.resolveExecCtx(').length - 1).toBeGreaterThan(40); // ...and the ownership guard is NOT sprayed across the file — it is a diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index c0249f19b91..53daa3ec744 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -338,7 +338,9 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ // ── runtime-pinned: an interpolated family, checked where it can be ───── { code: 'APPROVAL_*_FAILED', - file: 'packages/rest/src/rest-server.ts', + // [#23050 slice 3] Re-pointed, not re-judged: the three factories moved, + // unedited, with `RestServer#registerApprovalsEndpoints`' body. + file: 'packages/rest/src/approvals-routes.ts', shape: 'objlittemplate', door: 'rest', verdict: 'runtime-pinned', diff --git a/scripts/check-route-envelope.mjs b/scripts/check-route-envelope.mjs index 971bf6530af..7332a6b8b8f 100644 --- a/scripts/check-route-envelope.mjs +++ b/scripts/check-route-envelope.mjs @@ -334,7 +334,12 @@ const MODULES = { // string `error` bodies of `registerBatchEndpoints`' handlers left this file // with that body for `batch-routes.ts`, declared below at the count they had // here; the sum is unchanged (18 + 13 + 8 = 39). - stringError: 18, + // + // 18 → 17 (#23050 slice 3): NOT progress — the same MOVE. The one literal + // string `error` body of `registerApprovalsEndpoints`' handlers left this + // file with that body for `approvals-routes.ts`, declared below at the count + // it had here; the sum is unchanged (17 + 13 + 8 + 1 = 39). + stringError: 17, // 77 → 75 (#7981): registerSecurityEndpoints' two `handleError` arms moved // off the `{ code, error }` sibling-code literal onto the shared // `respondError` helper, banking that progress per the ratchet's own rule. @@ -380,7 +385,12 @@ const MODULES = { // 39 → 31 (#23050 slice 2): NOT progress — the same MOVE as `stringError` // above. Eight `{ code, error }` bodies left with the batch body for // `batch-routes.ts`, declared below; the sum is unchanged (31 + 15 + 8 = 54). - siblingCode: 31, + // + // 31 → 22 (#23050 slice 3): NOT progress — the same MOVE as `stringError` + // above. Nine `{ code, error }` bodies left with the approvals body for + // `approvals-routes.ts`, declared below; the sum is unchanged + // (22 + 15 + 8 + 9 = 54). + siblingCode: 22, }, // [#8850] The ADR-0112 error/fault-classification prologue, extracted from @@ -453,6 +463,24 @@ const MODULES = { stringError: 8, siblingCode: 8, }, + + // [#23050 slice 3] `RestServer#registerApprovalsEndpoints`' body — the + // approval request list and read, the decisions, recall, the flow moves, the + // thread interactions and the audit trail — moved out of `rest-server.ts` + // whole, on exactly slice 1's terms above: a MOVE, no wire answer changed, so + // it inherits `dialectOnly` and the two dialect facts, and no write-site + // count. Both counts were measured on the moved file and equal what left + // `rest-server.ts` in the same PR (stringError 18 → 17, siblingCode 31 → 22), + // so each sum is unchanged. Both only ever tick down. Discovered by the + // `*-routes.ts` convention and declared in the same PR that created the + // module, so the gap never exists. + 'packages/rest/src/approvals-routes.ts': { + dialectOnly: + 'moved out of `rest-server.ts` by #23050 slice 3 and held to exactly the guarantee it had there — the two dialect facts, never a write-site count', + ratchet: '#9559 (option 1: convert onto the shared sendOk/sendError)', + stringError: 1, + siblingCode: 9, + }, }; /** Identifiers whose `.json()` READS a request rather than writing a response. */