fix: require a compiled artifact for entrypoints.jobs
Module discovery gated the compiled-ESM rule on an allowlist, `["schema", "server", "ui"]`, so `entrypoints.jobs` accepted `./src/jobs/index.ts`. Every other executable entrypoint required a built `.js` or `.mjs`; `jobs` did not, purely because it was never added to that list. It is the second entrypoint to slip through the same way — `ui` did too, until Phase 6A added it explicitly — which is the argument against the allowlist rather than against `jobs`. The rule is now expressed as "every entrypoint except a directory entrypoint", with `migrations` the only member of that set. A new manifest entrypoint therefore defaults to the stricter validation instead of silently escaping it. The same constant replaces the two existing `=== "migrations"` special cases for the file/directory kind check and its message, so the one thing that distinguishes `migrations` is stated once. The directory message is now templated and produces byte-identical text. Every other entrypoint guarantee is untouched: package-relative paths, no traversal, inside-package before and after symlink resolution, the artifact must exist, and it must be a file. No runtime was added. `entrypoints.jobs` still has no consumer — it is declared, validated, and never executed — exactly as it has been since ADR 0006, and as `ui` was before its host loader existed. The manifest schema is unchanged and this stays within v4: the field, its type, and its optionality are all the same, and only discovery's validation of the declared path tightened. Compatibility impact is nil in first-party code: no module, template, or fixture declares a `jobs` entrypoint at all. A third-party module declaring a source-TypeScript path would now fail discovery, which is the intended correction. Tests cover accepted `.js` and `.mjs`, rejected `.ts`/`.tsx`/`.cjs`, a missing artifact, a directory, a symlink escaping the package, and that discovery still never evaluates the artifact. The three rejection cases were confirmed to fail against the previous implementation. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ff159a04dc
commit
3f9b5e3cb0
@@ -11,6 +11,22 @@ import {
|
|||||||
|
|
||||||
export const MODULE_MANIFEST_FILENAME = "ekklesia.module.json";
|
export const MODULE_MANIFEST_FILENAME = "ekklesia.module.json";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The only manifest entrypoint that resolves to a directory rather than a compiled file.
|
||||||
|
*
|
||||||
|
* Every other entrypoint is an executable build artifact, so the compiled-ESM rule is expressed as
|
||||||
|
* "not a directory entrypoint" rather than as a list of the entrypoints it covers. An allowlist is
|
||||||
|
* what let `jobs` and, before it, `ui` be added to the manifest and silently escape this check;
|
||||||
|
* defaulting a new entrypoint to *stricter* validation is the safer failure mode.
|
||||||
|
*/
|
||||||
|
const DIRECTORY_ENTRYPOINTS: ReadonlySet<string> = new Set(["migrations"]);
|
||||||
|
|
||||||
|
/** Whether an entrypoint must reference compiled ESM JavaScript (ADR 0006's production loading
|
||||||
|
* boundary): true for every entrypoint that is a file rather than a directory. */
|
||||||
|
function isCompiledEntrypoint(entrypointName: string): boolean {
|
||||||
|
return !DIRECTORY_ENTRYPOINTS.has(entrypointName);
|
||||||
|
}
|
||||||
|
|
||||||
export type ModuleDiscoveryIssueCode =
|
export type ModuleDiscoveryIssueCode =
|
||||||
| "DISCOVERY_CONFIGURATION_INVALID"
|
| "DISCOVERY_CONFIGURATION_INVALID"
|
||||||
| "MODULE_CAPABILITY_INCOMPATIBLE"
|
| "MODULE_CAPABILITY_INCOMPATIBLE"
|
||||||
@@ -247,11 +263,8 @@ async function resolveEntrypoints(
|
|||||||
if (relativeEntrypoint === undefined) {
|
if (relativeEntrypoint === undefined) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// `ui` joins `schema`/`server` here: a presentation entrypoint is a build output like any
|
|
||||||
// other module artifact, so source TypeScript is rejected at discovery rather than relied upon
|
|
||||||
// to be compiled later (ADR 0006's compiled-artifact rule, extended by ADR 0021).
|
|
||||||
if (
|
if (
|
||||||
["schema", "server", "ui"].includes(entrypointName) &&
|
isCompiledEntrypoint(entrypointName) &&
|
||||||
![".js", ".mjs"].includes(path.extname(relativeEntrypoint).toLowerCase())
|
![".js", ".mjs"].includes(path.extname(relativeEntrypoint).toLowerCase())
|
||||||
) {
|
) {
|
||||||
issues.push({
|
issues.push({
|
||||||
@@ -288,14 +301,14 @@ async function resolveEntrypoints(
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
const entrypointStats = await stat(canonicalEntrypoint);
|
const entrypointStats = await stat(canonicalEntrypoint);
|
||||||
const hasExpectedKind =
|
const hasExpectedKind = DIRECTORY_ENTRYPOINTS.has(entrypointName)
|
||||||
entrypointName === "migrations" ? entrypointStats.isDirectory() : entrypointStats.isFile();
|
? entrypointStats.isDirectory()
|
||||||
|
: entrypointStats.isFile();
|
||||||
if (!hasExpectedKind) {
|
if (!hasExpectedKind) {
|
||||||
issues.push({
|
issues.push({
|
||||||
code: "MODULE_ENTRYPOINT_INVALID",
|
code: "MODULE_ENTRYPOINT_INVALID",
|
||||||
message:
|
message: DIRECTORY_ENTRYPOINTS.has(entrypointName)
|
||||||
entrypointName === "migrations"
|
? `${entrypointName} must resolve to a directory.`
|
||||||
? "migrations must resolve to a directory."
|
|
||||||
: `${entrypointName} must resolve to a file.`,
|
: `${entrypointName} must resolve to a file.`,
|
||||||
moduleDirectory: directoryName,
|
moduleDirectory: directoryName,
|
||||||
moduleId: manifest.id,
|
moduleId: manifest.id,
|
||||||
|
|||||||
@@ -595,6 +595,122 @@ describe("module discovery", () => {
|
|||||||
expect(firstError.issues).toEqual(secondError.issues);
|
expect(firstError.issues).toEqual(secondError.issues);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("compiled jobs entrypoint", () => {
|
||||||
|
function manifestWithJobs(id: string, jobs: string): Record<string, unknown> {
|
||||||
|
const manifest = manifestFor(id);
|
||||||
|
manifest.entrypoints = { server: "./dist/server/index.js", jobs };
|
||||||
|
return manifest;
|
||||||
|
}
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["./dist/jobs/index.js", "index.js"],
|
||||||
|
["./dist/jobs/index.mjs", "index.mjs"],
|
||||||
|
])("accepts the compiled jobs artifact %s", async (jobs, filename) => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
await createModule(modulesDirectory, "jobs-compiled", {
|
||||||
|
manifest: manifestWithJobs("jobs-compiled", jobs),
|
||||||
|
extraFiles: { [`dist/jobs/${filename}`]: "export default {};\n" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const [discovered] = await discoverModules({
|
||||||
|
moduleSdkVersion: "0.1.0",
|
||||||
|
modulesDirectory,
|
||||||
|
platformVersion: "0.1.0",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(discovered?.entrypoints.jobs).toBe(
|
||||||
|
path.join(discovered?.directory ?? "", "dist/jobs", filename),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
"./src/jobs/index.ts",
|
||||||
|
"./src/jobs/index.tsx",
|
||||||
|
"./dist/jobs/index.cjs",
|
||||||
|
])("rejects the uncompiled jobs entrypoint %s", async (jobs) => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
await createModule(modulesDirectory, "jobs-source", {
|
||||||
|
manifest: manifestWithJobs("jobs-source", jobs),
|
||||||
|
extraFiles: { [jobs.slice(2)]: "export default {};\n" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const error = await discoveryError(modulesDirectory);
|
||||||
|
expect(error.issues).toContainEqual(
|
||||||
|
expect.objectContaining({
|
||||||
|
code: "MODULE_ENTRYPOINT_INVALID",
|
||||||
|
message: "jobs must reference compiled ESM JavaScript with a .js or .mjs extension.",
|
||||||
|
path: "entrypoints.jobs",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a declared jobs artifact that does not exist", async () => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
await createModule(modulesDirectory, "jobs-missing", {
|
||||||
|
manifest: manifestWithJobs("jobs-missing", "./dist/jobs/index.js"),
|
||||||
|
});
|
||||||
|
|
||||||
|
const error = await discoveryError(modulesDirectory);
|
||||||
|
expect(issueCodes(error)).toContain("MODULE_ENTRYPOINT_MISSING");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a jobs entrypoint that resolves to a directory", async () => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
await createModule(modulesDirectory, "jobs-directory", {
|
||||||
|
manifest: manifestWithJobs("jobs-directory", "./dist/jobs.js"),
|
||||||
|
extraFiles: { "dist/jobs.js/placeholder.txt": "not a file entrypoint" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const error = await discoveryError(modulesDirectory);
|
||||||
|
expect(error.issues).toContainEqual(
|
||||||
|
expect.objectContaining({
|
||||||
|
code: "MODULE_ENTRYPOINT_INVALID",
|
||||||
|
message: "jobs must resolve to a file.",
|
||||||
|
path: "entrypoints.jobs",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a jobs entrypoint linked outside its own module package", async () => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
const externalDirectory = await createModulesDirectory();
|
||||||
|
const externalArtifact = path.join(externalDirectory, "foreign-jobs.js");
|
||||||
|
await writeFile(externalArtifact, "export default {};\n");
|
||||||
|
await createModule(modulesDirectory, "jobs-linked", {
|
||||||
|
manifest: manifestWithJobs("jobs-linked", "./dist/jobs/index.js"),
|
||||||
|
});
|
||||||
|
await mkdir(path.join(modulesDirectory, "jobs-linked/dist/jobs"), { recursive: true });
|
||||||
|
await symlink(
|
||||||
|
externalArtifact,
|
||||||
|
path.join(modulesDirectory, "jobs-linked/dist/jobs/index.js"),
|
||||||
|
);
|
||||||
|
|
||||||
|
const error = await discoveryError(modulesDirectory);
|
||||||
|
expect(issueCodes(error)).toContain("MODULE_ENTRYPOINT_INVALID");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("never imports a jobs artifact merely to discover it", async () => {
|
||||||
|
const modulesDirectory = await createModulesDirectory();
|
||||||
|
await createModule(modulesDirectory, "jobs-inert", {
|
||||||
|
manifest: manifestWithJobs("jobs-inert", "./dist/jobs/index.js"),
|
||||||
|
// Discovery is inert. If it ever imported a jobs entrypoint this would reject rather than
|
||||||
|
// discover successfully — `jobs` still has no runtime consumer.
|
||||||
|
extraFiles: {
|
||||||
|
"dist/jobs/index.js": "throw new Error('a jobs entrypoint must never be evaluated');",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const discovered = await discoverModules({
|
||||||
|
moduleSdkVersion: "0.1.0",
|
||||||
|
modulesDirectory,
|
||||||
|
platformVersion: "0.1.0",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(discovered).toHaveLength(1);
|
||||||
|
expect(discovered[0]?.entrypoints.jobs).toContain("dist/jobs/index.js");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("module-owned UI entrypoint (ADR 0021)", () => {
|
describe("module-owned UI entrypoint (ADR 0021)", () => {
|
||||||
function manifestWithUi(id: string, ui: string): Record<string, unknown> {
|
function manifestWithUi(id: string, ui: string): Record<string, unknown> {
|
||||||
const manifest = manifestFor(id);
|
const manifest = manifestFor(id);
|
||||||
|
|||||||
@@ -272,3 +272,9 @@ every real feature module. `entrypoints.jobs` still accepts source TypeScript wh
|
|||||||
entrypoints require compiled output — that inconsistency is unchanged and remains open, because this
|
entrypoints require compiled output — that inconsistency is unchanged and remains open, because this
|
||||||
pass did not centralize executable-entrypoint validation and fixing it here would have been unrelated
|
pass did not centralize executable-entrypoint validation and fixing it here would have been unrelated
|
||||||
scope.
|
scope.
|
||||||
|
|
||||||
|
> **Correction — this deferred item is closed.** `entrypoints.jobs` now requires a compiled `.js` or
|
||||||
|
> `.mjs` artifact, exactly like `server`, `schema`, and `ui`. Module discovery expresses the rule as
|
||||||
|
> "every entrypoint except the directory entrypoint (`migrations`)", so the omission that let `jobs`
|
||||||
|
> escape it cannot recur. No jobs runtime was added: the entrypoint is still declared, validated, and
|
||||||
|
> never executed. Every other item in this list remains deferred.
|
||||||
|
|||||||
@@ -198,3 +198,9 @@ loading; and every real feature module. `entrypoints.jobs` still accepts source
|
|||||||
other executable entrypoints require compiled output; this pass reused the *path* grammar from
|
other executable entrypoints require compiled output; this pass reused the *path* grammar from
|
||||||
`@ekklesia/contracts` but did not centralize the *extension* rule, so that inconsistency is
|
`@ekklesia/contracts` but did not centralize the *extension* rule, so that inconsistency is
|
||||||
unchanged and still open.
|
unchanged and still open.
|
||||||
|
|
||||||
|
> **Correction — this deferred item is closed.** `entrypoints.jobs` now requires a compiled `.js` or
|
||||||
|
> `.mjs` artifact, exactly like `server`, `schema`, and `ui`. Module discovery expresses the rule as
|
||||||
|
> "every entrypoint except the directory entrypoint (`migrations`)", so the omission that let `jobs`
|
||||||
|
> escape it cannot recur. No jobs runtime was added: the entrypoint is still declared, validated, and
|
||||||
|
> never executed. Every other item in this list remains deferred.
|
||||||
|
|||||||
@@ -197,3 +197,9 @@ is unchanged and still open. The permission-id typing gap (`permissions.has` tak
|
|||||||
a typo silently hides a control) is likewise unchanged: the browser-safe split did not produce a
|
a typo silently hides a control) is likewise unchanged: the browser-safe split did not produce a
|
||||||
clean answer for it, and inventing one would have meant shipping generated constants this phase does
|
clean answer for it, and inventing one would have meant shipping generated constants this phase does
|
||||||
not need.
|
not need.
|
||||||
|
|
||||||
|
> **Correction — this deferred item is closed.** `entrypoints.jobs` now requires a compiled `.js` or
|
||||||
|
> `.mjs` artifact, exactly like `server`, `schema`, and `ui`. Module discovery expresses the rule as
|
||||||
|
> "every entrypoint except the directory entrypoint (`migrations`)", so the omission that let `jobs`
|
||||||
|
> escape it cannot recur. No jobs runtime was added: the entrypoint is still declared, validated, and
|
||||||
|
> never executed. Every other item in this list remains deferred.
|
||||||
|
|||||||
@@ -153,11 +153,11 @@ module's own HTTP routes, and creating relationship-derived grants automatically
|
|||||||
own business logic, are both still later work; this only makes a declared permission *usable*
|
own business logic, are both still later work; this only makes a declared permission *usable*
|
||||||
through Core's existing, generic surface.
|
through Core's existing, generic surface.
|
||||||
|
|
||||||
Server and optional schema and UI entrypoints must be compiled ESM `.js` or `.mjs` files. The
|
Every entrypoint except `migrations` must be a compiled ESM `.js` or `.mjs` file — `server`,
|
||||||
optional migration entrypoint is a directory. All entrypoints begin with `./`, remain inside the
|
`schema`, `jobs`, and `ui` alike. `migrations` is the one directory entrypoint. All entrypoints begin
|
||||||
package, and exist. Production never imports source TypeScript directly. Modules with no tables omit
|
with `./`, remain inside the package, and exist. Production never imports source TypeScript directly.
|
||||||
`entrypoints.schema`; modules with no migration history omit `entrypoints.migrations`; modules with
|
Modules with no tables omit `entrypoints.schema`; modules with no migration history omit
|
||||||
no UI omit `entrypoints.ui`.
|
`entrypoints.migrations`; modules with no UI omit `entrypoints.ui`.
|
||||||
|
|
||||||
Unknown manifest properties are still rejected. `ui` is the only entrypoint version 4 adds — a
|
Unknown manifest properties are still rejected. `ui` is the only entrypoint version 4 adds — a
|
||||||
`dashboard`, `surface`, `client`, `web`, or `renderers` key fails validation — and the manifest
|
`dashboard`, `surface`, `client`, `web`, or `renderers` key fails validation — and the manifest
|
||||||
|
|||||||
Reference in New Issue
Block a user