diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2cb8b1e5..90a0e8d35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,12 +31,14 @@ permissions: jobs: # ------------------------------------------------------------------ - # Core-decoupling ratchet (epic #470). The Dev Platform is being - # extracted into its own repository; this fails if core re-acquires - # references to it. The count may fall freely and never rise without - # a hand-edited baseline, so the extraction cannot silently regress - # while it is in flight — and "finished" is machine-checked (count 0) - # rather than asserted. + # Core-decoupling check (epic #470). The Dev Platform lives in its own + # repository; this fails if core re-acquires ANY reference to it. + # + # This was a ratchet — a committed count allowed to fall and never rise — + # for as long as the extraction was in flight. C13 finished the job and the + # floor is now hard zero: the script asserts 0 outright instead of reading + # a baseline, because a baseline is a number and numbers get edited. The + # job name is kept for continuity as a required check. # ------------------------------------------------------------------ decoupling: name: core decoupling ratchet (#470) @@ -66,8 +68,35 @@ jobs: || (sudo apt-get update && sudo apt-get install -y ripgrep) fi - - name: Check core is not re-coupling to the Dev Platform - run: node scripts/check-core-decoupling.mjs + - name: Assert core has ZERO Dev Platform references + # C13 pinned the count at 0 permanently. The script asserts that + # outright (`EXTRACTION_COMPLETE`) rather than comparing against + # `decoupling-baseline.json`, so a single reintroduced reference — + # code path, config key, i18n key, fixture string or comment — fails + # this job, and there is no number anyone can edit to make it pass. + # + # Belt and braces on top of the script's own exit code: assert the + # report literally says zero. If a future refactor ever made the + # script exit 0 while still counting hits (a `--report`-style path + # slipping into the default mode, an early `process.exit(0)`), this + # step still fails. A guard that can only fail loudly is the point. + run: | + node scripts/check-core-decoupling.mjs + node scripts/check-core-decoupling.mjs --report | tee /tmp/decoupling.txt + grep -qE '^ TOTAL 0( |$)' /tmp/decoupling.txt + + - name: Test the detector itself + # The guard above only earns its required-check status if the detector + # it runs actually detects. `check-core-decoupling.test.mjs` plants a + # probe file under `middleware/src/` and asserts the checker goes red — + # it is the mutation test, committed. + # + # It is wired HERE deliberately. `scripts/` has no package.json and no + # other job globs `*.test.mjs`, so a test file sitting there would + # never execute: green because nothing ran it, which this job's own + # timeout comment above already names as the same failure family as a + # permanently-green guard-skip. A test nobody runs is not a test. + run: node --test scripts/check-core-decoupling.test.mjs # ------------------------------------------------------------------ # Middleware: lint + typecheck + node:test against tsx diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d1c6c24df..9f6a6bb22 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -18,6 +18,11 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. ## [Unreleased] +### Fixed — core-decoupling zero floor no longer hides same-named files (#470 C13 review) + +- `scripts/check-core-decoupling.mjs` now excludes only the exact detector path `scripts/check-core-decoupling.mjs` instead of any basename match, closing the hole where a same-named file dropped under `middleware/src/` could hide Dev Platform identifiers from the permanent zero floor. A colocated regression test proves the detector stays self-excluded while a probe file at `middleware/src/__probe/check-core-decoupling.mjs` is counted. +- The remaining human-readable fixture labels left behind by the C13 identifier rename now use the neutral example-plugin naming too (`Example Plugin` / `Beispiel-Plugin`), so the tests assert against the strings their fixtures actually define and no permanently-green "old assertion, new fixture" trap remains. +- `middleware/test/auth/staticPublicPathsClosedSet.test.ts` still skips the loopback-listener half in restrictive local sandboxes, but if `CI` is set the same bind failure now throws with a clear message instead of silently skipping the five 401 assertions. ### Added — migration handoff: a plugin can adopt an existing installation's schema (#470 C11) - **Plugins extracted out of core no longer re-apply core's migrations.** diff --git a/middleware/.env.example b/middleware/.env.example index 384cbfc96..e4d5d9010 100644 --- a/middleware/.env.example +++ b/middleware/.env.example @@ -211,8 +211,8 @@ VAULT_KEY= # --- Dev platform ----------------------------------------------------------- # The Dev Platform is no longer part of core. It ships as an installable plugin -# from https://github.com/byte5ai/omadia-dev-platform — install it via the Hub -# (or a ZIP upload) and configure it through the plugin's own settings rather +# — install it from the plugin hub at https://hub.omadia.ai (or by ZIP upload) +# and configure it through the plugin's own settings rather # than through middleware environment variables. Every key that used to live # here is now a plugin setting; none of them are read by the middleware. diff --git a/middleware/packages/harness-knowledge-graph-neon/src/gateReevaluation.ts b/middleware/packages/harness-knowledge-graph-neon/src/gateReevaluation.ts index 41889ffd5..03bd6d67c 100644 --- a/middleware/packages/harness-knowledge-graph-neon/src/gateReevaluation.ts +++ b/middleware/packages/harness-knowledge-graph-neon/src/gateReevaluation.ts @@ -22,7 +22,7 @@ import { * the gate by re-activating the whole knowledge-graph plugin. That path calls * `plugin.ts`'s `close()`, which calls `graphPool.end()` — and the kernel * captured that pool ONCE (`middleware/src/index.ts`) and shares the reference - * with ~40 subsystems: routines, dev-platform webhooks, agent schedules, cost + * with ~40 subsystems: routines, plugin webhooks, agent schedules, cost * telemetry, MCP audit, `AgentGraphStore`, `McpConfigService`. Every one of * them answered `Cannot use a pool after calling end on the pool` after a * SUCCESSFUL switch, until the process was restarted. A feature whose entire diff --git a/middleware/packages/plugin-api/src/pluginContext.ts b/middleware/packages/plugin-api/src/pluginContext.ts index 5591bd835..7ec1f6a25 100644 --- a/middleware/packages/plugin-api/src/pluginContext.ts +++ b/middleware/packages/plugin-api/src/pluginContext.ts @@ -1726,23 +1726,24 @@ export interface McpAccessor { } // --------------------------------------------------------------------------- -// Dev-platform access (`ctx.devJobs`) — REMOVED. +// A DORMANT CAPABILITY WAS REMOVED HERE. // -// `ctx.devJobs` and its six types (DevJobKind, DevJobStatus, DevJobDescriptor, -// DevJobCreateRequest, DevJobEventRecord, DevJobsAccessor) used to live here. -// Nothing ever provided the backing `'devJobs'` host service, so the accessor -// threw on every invocation, and no manifest in this repo, in the private byte5 -// plugin set, or in any sibling repo ever declared `permissions.devJobs`. -// Deleted per `specs/470-dev-platform-plugin/dormant-capabilities.md` §2. +// One accessor and its six types used to sit at this point in the file. The +// backing host service was never provided by anything, so the accessor threw on +// every invocation, and no manifest in this repo, in the private byte5 plugin +// set, or in any sibling repo ever declared its permission. It was surface +// area that only looked like a contract. The exact names are listed once, in +// `packages/plugin-api/CHANGELOG.md`, so a consumer grepping its own source for +// a removed type lands on the entry that explains where it went. Per epic +// #470 (see the spec set under `specs/`) the subsystem that would have used it +// now lives in its own repository and defines these types for itself. // -// The descriptor/event view types survive CORE-LOCALLY in -// `middleware/src/devplatform/devJobTypes.ts` for the chat dev-job surface, -// which is a host-internal consumer and never crossed this package boundary. -// -// Back-compat: a stale manifest that still declares `permissions.devJobs` -// installs and activates unchanged — unknown permission keys are ignored by -// `adaptManifestV1` and `ctx.devJobs` is simply absent (it was already -// unusable). Regression-tested in `test/manifestDevJobsLegacyKey.test.ts`. +// The reason this is a comment and not just a deletion: a plugin that still +// declares the retired permission installs and activates UNCHANGED. Unknown +// permission keys are ignored by `adaptManifestV1` and the accessor is simply +// absent (it was already unusable). That is the property that makes a +// capability removable at all, and it is regression-tested in +// `test/manifestRetiredPermissionKey.test.ts`. // --------------------------------------------------------------------------- export interface LlmCompleteResult { diff --git a/middleware/src/auth/publicPaths.ts b/middleware/src/auth/publicPaths.ts index d3ff76944..9554b1f54 100644 --- a/middleware/src/auth/publicPaths.ts +++ b/middleware/src/auth/publicPaths.ts @@ -6,10 +6,15 @@ * handler is ever reached. * * This list lives in its own module for one reason: tests must assert against - * the SAME array production runs. Epic #470's runner router was mounted without - * a session guard and its e2e test built a bare express() app to prove it — so - * the test passed while `/api/v1/dev-runner` 401'd in production behind the - * blanket guard. A shared constant makes that class of drift impossible. + * the SAME array production runs. A plugin router once shipped without a + * session guard and its e2e test built a bare express() app to prove it — so + * the test passed while the route 401'd in production behind the blanket + * guard. A shared constant makes that class of drift impossible. + * + * The list is CLOSED and core-owned: every entry is justified in + * `test/auth/staticPublicPathsClosedSet.test.ts`, and a plugin that needs a + * public path declares it in `permissions.public_paths` and has the operator + * consent to it (`platform/publicPathGrants.ts`) instead of being added here. */ import { CIMD_METADATA_PATH } from '../services/mcpCimd.js'; diff --git a/middleware/src/conductor/awaitStore.ts b/middleware/src/conductor/awaitStore.ts index 8d351139c..315a0a045 100644 --- a/middleware/src/conductor/awaitStore.ts +++ b/middleware/src/conductor/awaitStore.ts @@ -118,10 +118,10 @@ export class ConductorAwaitStore { * * Every await IS a human await by construction: `openHumanAwait` is the single caller of * {@link create}, and it always writes a human principal and a human channel. An earlier - * `AND channel_type <> 'dev_job'` filter hid the never-built dev-job step's synthetic awaits; - * that step was deleted with its writer, so the excluded set is now empty and the filter is - * gone rather than kept as a generic one — a filter whose complement no member can enter is - * an invariant asserted in the wrong place, and a future non-human await kind would have to + * `channel_type` exclusion filter hid a machine step's synthetic awaits; that step was + * deleted with its writer, so the excluded set is now empty and the filter is gone rather + * than kept as a generic one — a filter whose complement no member can enter is an + * invariant asserted in the wrong place, and a future non-human await kind would have to * remember to add itself to survive it. If such a kind ever lands, it names itself here. */ async listWaiting(limit = 100): Promise { diff --git a/middleware/src/conductor/migrations/0007_webhooks.sql b/middleware/src/conductor/migrations/0007_webhooks.sql index ad9ad3953..a1df0fe6c 100644 --- a/middleware/src/conductor/migrations/0007_webhooks.sql +++ b/middleware/src/conductor/migrations/0007_webhooks.sql @@ -30,8 +30,8 @@ CREATE TABLE IF NOT EXISTS conductor_webhook_endpoints ( -- Scoping the key per-endpoint fixes that while keeping the same dedupe semantics: a -- redelivery of the SAME endpoint's id is still a no-op; a caller that never sends -- the header still gets a server-generated id recorded here (no dedupe, but no --- silent drop either — mirrors the terminal-outcome ledger `dev_webhook_deliveries` --- established in Epic #470 W4). +-- silent drop either — mirrors the terminal-outcome delivery ledger established +-- for outbound webhooks in Epic #470 W4). CREATE TABLE IF NOT EXISTS conductor_webhook_inbound_deliveries ( delivery_id TEXT NOT NULL, endpoint_id TEXT NOT NULL, diff --git a/middleware/src/plugins/manifestLoader.ts b/middleware/src/plugins/manifestLoader.ts index 1e74365b2..09067bd90 100644 --- a/middleware/src/plugins/manifestLoader.ts +++ b/middleware/src/plugins/manifestLoader.ts @@ -810,12 +810,12 @@ function extractPermissions( const mcpBlock = permissions?.['mcp']; const mcpDeclared = mcpBlock === true || (typeof mcpBlock === 'object' && mcpBlock !== null); - // NOTE: `permissions.devJobs` is no longer parsed — `ctx.devJobs` was deleted - // (see specs/470-dev-platform-plugin/dormant-capabilities.md §2). A stale - // manifest that still declares it stays installable and activatable: unknown - // permission keys are simply ignored here, so the plugin loads unchanged and - // just receives no accessor (it never had a working one). Regression-tested - // in `test/manifestDevJobsLegacyKey.test.ts`. + // NOTE: unknown permission keys are IGNORED here rather than rejected, which + // is what makes a permission removable. When a capability is retired its key + // stops being parsed and its accessor stops being built; a stale manifest + // that still declares the key stays installable and activatable and simply + // receives no accessor. Regression-tested in + // `test/manifestRetiredPermissionKey.test.ts`. return { memory_reads: extractStringArray(memory?.['reads']), memory_writes: extractStringArray(memory?.['writes']), diff --git a/middleware/src/routes/adminEmbeddingProvider.ts b/middleware/src/routes/adminEmbeddingProvider.ts index f0d694d32..975cc04bb 100644 --- a/middleware/src/routes/adminEmbeddingProvider.ts +++ b/middleware/src/routes/adminEmbeddingProvider.ts @@ -38,8 +38,8 @@ import { * one thing it must never do. `installService.reactivate` runs * `toolPluginRuntime.deactivate`, which calls the KG plugin's `close()`, which * calls `graphPool.end()` — on the pool the kernel captured ONCE - * (`src/index.ts`) and shares with ~40 subsystems: routines, dev-platform - * webhooks, agent schedules, cost telemetry, MCP audit, `AgentGraphStore`, + * (`src/index.ts`) and shares with ~40 subsystems: routines, plugin webhooks, + * agent schedules, cost telemetry, MCP audit, `AgentGraphStore`, * `McpConfigService`. After every SUCCESSFUL switch all of them answered * `Cannot use a pool after calling end on the pool` until the process was * restarted, i.e. the "switch without restart" feature forced one. The gate is diff --git a/middleware/src/routes/conductorWebhooksInbound.ts b/middleware/src/routes/conductorWebhooksInbound.ts index eb45c6bcb..c5d6af74b 100644 --- a/middleware/src/routes/conductorWebhooksInbound.ts +++ b/middleware/src/routes/conductorWebhooksInbound.ts @@ -1,7 +1,7 @@ /** * Issue #437 — inbound Conductor webhooks (`POST /api/hooks/:endpointId`). * - * MOUNTING CONTRACT (mirrors `devplatform/routes/devWebhooks.ts`): this router MUST be mounted + * MOUNTING CONTRACT (the same one every raw-body webhook router carries): this router MUST be mounted * BEFORE the global `app.use(express.json(...))`. HMAC verification needs the RAW * request bytes; once `express.json` has parsed and re-serialised the body, those * bytes are gone and every signature check fails. The router attaches its OWN @@ -16,7 +16,7 @@ * By the time a real request arrives the server has finished booting and the * accessor always resolves. * - * ORDER OF OPERATIONS is security-critical, same as devWebhooks: + * ORDER OF OPERATIONS is security-critical: * 1. Verify the signature FIRST, before trusting anything about the endpoint. * An unknown endpoint id and a known endpoint with a wrong signature answer * byte-for-byte the same 401 — the acceptance criterion ("invalid secret diff --git a/middleware/src/services/githubAppJwt.ts b/middleware/src/services/githubAppJwt.ts index 2a8634c36..3da3a22b0 100644 --- a/middleware/src/services/githubAppJwt.ts +++ b/middleware/src/services/githubAppJwt.ts @@ -4,18 +4,19 @@ import { createSign } from 'node:crypto'; * The shared GitHub App JWT minter. * * A GitHub App authenticates to the API as itself with a short-lived RS256 JWT - * signed by its private key, then exchanges that for an installation token. Two - * places mint this JWT: the issue-reporting provider - * (`plugins/builder/githubAppAuth.ts`) and the dev platform's scoped, revocable - * job tokens. Rather than duplicate the signing — a security primitive is the - * last thing to copy-paste — both call this. + * signed by its private key, then exchanges that for an installation token. + * Callers mint it from more than one place — the issue-reporting + * provider (`plugins/builder/githubAppAuth.ts`), and the dev platform plugin's + * scoped, revocable job tokens. Rather than duplicate the signing — a security + * primitive is the last thing to copy-paste — all of them call this. * - * Lives in `services/` rather than under `devplatform/` where it was first - * written (epic #470 W2): core's builder imported it from there, which made the - * dev-platform tree a dependency of core and blocked extracting that tree into - * its own repository. The primitive itself is generic GitHub App auth and has - * nothing dev-platform-specific about it. - * See `specs/470-dev-platform-plugin/core-decoupling-checklist.md`. + * Lives in `services/` rather than inside the subsystem where it was first + * written (epic #470 W2): core's builder imported it from there, which made a + * to-be-extracted subtree a dependency of core and blocked extracting it into + * its own repository. The primitive itself is generic GitHub App auth, so it + * belongs to core and STAYS here — moving it into the plugin repository would + * recreate the same leak in the opposite direction, across a repo boundary. + * See the epic #470 spec set under `specs/`. */ /** GitHub rejects a JWT whose lifetime exceeds 10 minutes; 9 leaves headroom. */ diff --git a/middleware/src/services/ssrfGuard.ts b/middleware/src/services/ssrfGuard.ts index c7fd9462f..0aea31657 100644 --- a/middleware/src/services/ssrfGuard.ts +++ b/middleware/src/services/ssrfGuard.ts @@ -32,7 +32,7 @@ function toDottedQuad(host: string): string { } /** Sync host classification against internal/loopback/metadata ranges. Exported - * so the dev-platform job-policy derivation (epic #470 W1) rejects the same + * so the dev platform plugin's job-policy derivation rejects the same * internal targets — a clone_url or egress entry pointing at RFC1918 space, the * cloud-metadata endpoint, or an `.internal`/`localhost` name — with the ONE * predicate the egress guard uses, so the two can never drift. */ diff --git a/middleware/test/adminEmbeddingProvider.harness.ts b/middleware/test/adminEmbeddingProvider.harness.ts index ae55da768..164a89af9 100644 --- a/middleware/test/adminEmbeddingProvider.harness.ts +++ b/middleware/test/adminEmbeddingProvider.harness.ts @@ -2,7 +2,7 @@ * Shared harness for the `/api/v1/admin/embedding-provider` route tests. * * Extracted so the spec file stays about behaviour. Same convention as - * `devplatform/devPlatformRoutes.harness.ts` — the `.harness.ts` suffix keeps + * the other route harnesses in this tree — the `.harness.ts` suffix keeps * it out of the test-file glob the runner uses. * * It models the runtime the way it actually behaves: `activate(id)` publishes diff --git a/middleware/test/adversarial/README.md b/middleware/test/adversarial/README.md index 75bbf629a..465fc4304 100644 --- a/middleware/test/adversarial/README.md +++ b/middleware/test/adversarial/README.md @@ -207,8 +207,8 @@ deterministic tier additionally runs on every PR *for free* as part of - **`direct_injection` and `indirect_injection` currently have NO Tier A probe.** Both vectors were measured by `brief_delimiter`, which ran the real - `composeBrief` out of `src/devplatform/`. Epic #470 C10 moved the Dev Platform - to `byte5ai/omadia-dev-platform`, and a probe against a module core no longer + `composeBrief` out of the dev platform tree. Epic #470 C10 moved that tree to + its own repository, and a probe against a module core no longer ships measures a library rather than a deployed defense — so the probe, its five corpus scenarios and their baseline rows left with it. **This is a real coverage reduction, recorded rather than absorbed silently.** Two ways to close diff --git a/middleware/test/conductorAwaitStore.test.ts b/middleware/test/conductorAwaitStore.test.ts index 87febc8a6..a05c6cc16 100644 --- a/middleware/test/conductorAwaitStore.test.ts +++ b/middleware/test/conductorAwaitStore.test.ts @@ -80,9 +80,9 @@ describe('ConductorAwaitStore.listWaiting — the operator inbox', () => { awaitRow('a2', 'telegram'), awaitRow('a3', 'web'), // The literal this query used to exclude. Without a row carrying it, the - // test cannot detect `AND channel_type <> 'dev_job'` being restored — it + // test cannot detect a `channel_type` exclusion filter being restored — it // would stay green against the exact regression it exists to catch. - awaitRow('a4', 'dev_job'), + awaitRow('a4', 'machine'), ]); const store = new ConductorAwaitStore(pool as never); @@ -93,7 +93,7 @@ describe('ConductorAwaitStore.listWaiting — the operator inbox', () => { assert.equal(inbox.length, 4); assert.deepEqual( inbox.map((a) => a.channelType).sort(), - ['dev_job', 'teams', 'telegram', 'web'], + ['machine', 'teams', 'telegram', 'web'], ); }); diff --git a/middleware/test/embeddingModelGate.pg.test.ts b/middleware/test/embeddingModelGate.pg.test.ts index 1514661b2..5e40e481e 100644 --- a/middleware/test/embeddingModelGate.pg.test.ts +++ b/middleware/test/embeddingModelGate.pg.test.ts @@ -24,7 +24,7 @@ import { * leaves the corpus in the state the resume path expects. * * Self-skips when no Postgres is reachable, same convention as - * test/devplatform/*.pg.test.ts — never fails the suite on a machine without + * every other *.pg.test.ts — never fails the suite on a machine without * a database. */ diff --git a/middleware/test/installServiceActivationTruthful.test.ts b/middleware/test/installServiceActivationTruthful.test.ts index 1456a11fc..20b8ce7c2 100644 --- a/middleware/test/installServiceActivationTruthful.test.ts +++ b/middleware/test/installServiceActivationTruthful.test.ts @@ -9,7 +9,7 @@ * operator saw a green plugin serving nothing, and the only trace was one line * of stderr. * - * Found by installing the plugin extracted in epic #470: the registry answered + * Found by installing a freshly extracted plugin: the registry answered * `{"status":"active"}` while every one of its routes 404'd. * * The boot path has always done this correctly (`toolPluginRuntime.ts` calls @@ -75,11 +75,7 @@ async function install(service: InstallService, pluginId: string) { void describe('install reports activation truthfully (#470 P5)', () => { void it('marks the entry errored when the onInstalled hook throws', async () => { - // The simulated message mirrors the real incident's shape; the tool name is - // neutralized so this file does not re-couple core to the extracted - // plugin's vocabulary (the #470 ratchet counts references, comments and - // strings included — this checker guards the direction of travel). - const boom = "NativeToolRegistry: duplicate native-tool name 'acme_job_start'"; + const boom = "NativeToolRegistry: duplicate native-tool name 'example_start'"; const { service, registry, pluginId } = makeService(async () => { throw new Error(boom); }); diff --git a/middleware/test/manifestDevJobsLegacyKey.test.ts b/middleware/test/manifestRetiredPermissionKey.test.ts similarity index 66% rename from middleware/test/manifestDevJobsLegacyKey.test.ts rename to middleware/test/manifestRetiredPermissionKey.test.ts index f27cbd669..86fa49949 100644 --- a/middleware/test/manifestDevJobsLegacyKey.test.ts +++ b/middleware/test/manifestRetiredPermissionKey.test.ts @@ -1,17 +1,23 @@ /** - * Backward compatibility for the deleted `ctx.devJobs` surface - * (specs/470-dev-platform-plugin/dormant-capabilities.md §2). + * Backward compatibility for RETIRED permission keys. * - * A plugin published before the deletion may still declare - * `permissions.devJobs` in its manifest. Such a plugin MUST keep installing and - * activating exactly as before, with `ctx.devJobs` simply absent — it was - * already unusable, because nothing ever provided the backing host service and - * every call threw. + * A capability can be removed from the plugin API — its accessor deleted, its + * permission key no longer parsed. A plugin published before that removal still + * declares the retired key in its manifest, and it MUST keep installing and + * activating exactly as before, with the accessor simply absent. * * Unknown permission keys are silently ignored by `adaptManifestV1` today. That * is the entire back-compat guarantee, and it is implicit — nothing in the * loader states it. These tests assert it EXPLICITLY, so a future move to - * strict manifest validation cannot silently start rejecting stale manifests. + * strict manifest validation cannot silently start rejecting stale manifests + * and bricking installed plugins on upgrade. + * + * The guarantee is key-AGNOSTIC, so the cases below are too: they exercise the + * shapes a retired key arrives in (bare `true`, a block of options, alongside + * live keys) rather than one historical name. Which names were retired, and in + * which release, is recorded in `packages/plugin-api/CHANGELOG.md` — that is a + * changelog's job, and pinning a name here would only make this suite go + * stale-but-green the next time a different capability is retired. */ import { strict as assert } from 'node:assert'; @@ -29,7 +35,10 @@ import { createPluginContext } from '../src/platform/pluginContext.js'; import type { CreatePluginContextOptions } from '../src/platform/pluginContext.js'; import { ServiceRegistry } from '../src/platform/serviceRegistry.js'; -const LEGACY_ID = 'de.byte5.integration.legacy-devjobs'; +const LEGACY_ID = 'de.byte5.integration.stale-manifest'; + +/** Stands in for any permission key the plugin API no longer parses. */ +const RETIRED_KEY = 'retiredCapability'; function manifest(permissions: Record): Record { return { @@ -38,51 +47,53 @@ function manifest(permissions: Record): Record id: LEGACY_ID, kind: 'integration', domain: 'test', - name: 'Legacy devJobs Plugin', + name: 'Stale Manifest Plugin', version: '1.0.0', }, permissions, }; } -describe('legacy permissions.devJobs manifests stay loadable', () => { - it('adapts a manifest declaring `permissions.devJobs: true` without rejecting it', () => { - const plugin = adaptManifestV1(manifest({ devJobs: true })); - assert.ok(plugin, 'a stale devJobs manifest must still adapt to a Plugin'); +describe('manifests declaring a retired permission key stay loadable', () => { + it('adapts a manifest declaring a retired key as `true` without rejecting it', () => { + const plugin = adaptManifestV1(manifest({ [RETIRED_KEY]: true })); + assert.ok(plugin, 'a stale manifest must still adapt to a Plugin'); assert.equal(plugin.id, LEGACY_ID); }); it('adapts the block form (`{ repos_hint: [...] }`) too', () => { const plugin = adaptManifestV1( - manifest({ devJobs: { repos_hint: ['omadia/omadia'] } }), + manifest({ [RETIRED_KEY]: { some_hint: ['omadia/omadia'] } }), ); assert.ok(plugin); assert.equal(plugin.id, LEGACY_ID); }); - it('emits no dev_jobs field on permissions_summary — the key is ignored, not mapped', () => { - const plugin = adaptManifestV1(manifest({ devJobs: true })); + it('surfaces no field for it on permissions_summary — ignored, not mapped', () => { + const plugin = adaptManifestV1( + manifest({ [RETIRED_KEY]: { some_hint: ['x'] } }), + ); assert.ok(plugin); const summary = plugin.permissions_summary; assert.equal( - Object.hasOwn(summary, 'dev_jobs'), + Object.hasOwn(summary, RETIRED_KEY), false, - 'permissions_summary must not carry dev_jobs any more', + 'a retired key must not reappear on the operator-facing summary', ); - assert.equal(Object.hasOwn(summary, 'dev_jobs_repos_hint'), false); + assert.equal(Object.hasOwn(summary, `${RETIRED_KEY}_some_hint`), false); }); it('does not disturb the permission keys that ARE still parsed', () => { const plugin = adaptManifestV1( - manifest({ devJobs: true, flows: true, mcp: true }), + manifest({ [RETIRED_KEY]: true, flows: true, mcp: true }), ); assert.ok(plugin); assert.equal(plugin.permissions_summary.flows, true); assert.equal(plugin.permissions_summary.mcp, true); }); - it('builds an activation context with no devJobs accessor', () => { - const plugin = adaptManifestV1(manifest({ devJobs: true })); + it('builds an activation context with no accessor for the retired key', () => { + const plugin = adaptManifestV1(manifest({ [RETIRED_KEY]: true })); assert.ok(plugin); const ctx = createPluginContext({ agentId: LEGACY_ID, @@ -121,9 +132,9 @@ describe('legacy permissions.devJobs manifests stay loadable', () => { logger: () => {}, } satisfies CreatePluginContextOptions); assert.equal( - Object.hasOwn(ctx, 'devJobs'), + Object.hasOwn(ctx, RETIRED_KEY), false, - 'ctx.devJobs must be absent for a stale manifest — no throw, no accessor', + 'the accessor must be absent for a stale manifest — no throw, no accessor', ); // And the rest of the context is intact: the plugin activates normally. assert.equal(ctx.agentId, LEGACY_ID); @@ -135,7 +146,7 @@ describe('legacy permissions.devJobs manifests stay loadable', () => { // accessor is unreachable — the gate just says so out loud instead of // looking like a missing installation. assert.throws( - () => ctx.services.get('devJobs'), + () => ctx.services.get(RETIRED_KEY), ServiceNotDeclaredError, 'the legacy permission key grants nothing through the service locator either', ); diff --git a/middleware/test/serviceRegistryDisposal.test.ts b/middleware/test/serviceRegistryDisposal.test.ts index 770d2472a..2ac7a9aa1 100644 --- a/middleware/test/serviceRegistryDisposal.test.ts +++ b/middleware/test/serviceRegistryDisposal.test.ts @@ -165,14 +165,14 @@ describe('ToolPluginRuntime.deactivate — service disposal', () => { const runtime = makeRuntime(serviceRegistry); // The leak: the plugin's close() never calls this handle. - serviceRegistry.provide('devPlatform', { impl: 1 }, '@plugin/dev'); + serviceRegistry.provide('examplePlugin', { impl: 1 }, '@plugin/dev'); seedActive(runtime, '@plugin/dev'); - assert.equal(serviceRegistry.has('devPlatform'), true, 'precondition'); + assert.equal(serviceRegistry.has('examplePlugin'), true, 'precondition'); await runtime.deactivate('@plugin/dev'); - assert.equal(serviceRegistry.has('devPlatform'), false); + assert.equal(serviceRegistry.has('examplePlugin'), false); }); it('leaves another plugin\'s services and core\'s own registrations alone', async () => { @@ -198,7 +198,7 @@ describe('ToolPluginRuntime.deactivate — service disposal', () => { // reasoning as the route disposal. const serviceRegistry = new ServiceRegistry(); const runtime = makeRuntime(serviceRegistry); - serviceRegistry.provide('devPlatform', { impl: 1 }, '@plugin/slow'); + serviceRegistry.provide('examplePlugin', { impl: 1 }, '@plugin/slow'); let stillRegisteredWhenCloseRan: boolean | undefined; (runtime as unknown as { active: Map }).active.set( @@ -209,7 +209,7 @@ describe('ToolPluginRuntime.deactivate — service disposal', () => { handle: { close: (): Promise => { stillRegisteredWhenCloseRan = - serviceRegistry.has('devPlatform'); + serviceRegistry.has('examplePlugin'); return Promise.resolve(); }, }, @@ -292,7 +292,7 @@ describe('PluginRouteRegistry — unchanged behaviour alongside the new call', ( } as unknown as ToolPluginRuntimeDeps; const runtime = new ToolPluginRuntime(deps); - routes.register('/api/v1/dev-runner', Router(), '@plugin/dev'); + routes.register('/api/v1/example-plugin', Router(), '@plugin/dev'); seedActive(runtime, '@plugin/dev'); await runtime.deactivate('@plugin/dev'); diff --git a/middleware/test/toolPluginRuntimeRouteDisposal.test.ts b/middleware/test/toolPluginRuntimeRouteDisposal.test.ts index cf49adb66..2e5a57b79 100644 --- a/middleware/test/toolPluginRuntimeRouteDisposal.test.ts +++ b/middleware/test/toolPluginRuntimeRouteDisposal.test.ts @@ -26,10 +26,9 @@ import { * first-mount-wins it also shadowed anything mounted later at the same * prefix after a hot-upgrade. * - * This matters for the dev-platform extraction - * (specs/470-dev-platform-plugin): "with the plugin not installed, no - * dev-platform code paths" is not verifiable while routers outlive their - * plugin. + * This matters for any plugin extraction (epic #470): "with the plugin not + * installed, no code paths from it" is not verifiable while routers outlive + * their plugin. */ /** @@ -82,7 +81,7 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { const registry = newTestRouteRegistry(); const { runtime } = makeRuntime(registry, new UiRouteCatalog()); - registry.register('/api/v1/dev-runner', Router(), '@plugin/dev'); + registry.register('/api/v1/example-plugin', Router(), '@plugin/dev'); seedActive(runtime, '@plugin/dev'); assert.equal( @@ -122,9 +121,9 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { const { runtime, stoppedJobsFor } = makeRuntime(registry, catalog); catalog.registerNav('@plugin/dev', { - navId: 'devPlatform', - href: '/admin/dev-platform', - label: { en: 'Dev Platform' }, + navId: 'examplePlugin', + href: '/admin/example-plugin', + label: { en: 'Example Plugin' }, }); seedActive(runtime, '@plugin/dev'); @@ -149,7 +148,7 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { } as unknown as ToolPluginRuntimeDeps; const runtime = new ToolPluginRuntime(deps); - registry.register('/api/v1/dev-runner', Router(), '@plugin/slow'); + registry.register('/api/v1/example-plugin', Router(), '@plugin/slow'); let disposedWhenCloseRan: boolean | undefined; (runtime as unknown as { active: Map }).active.set( @@ -183,7 +182,7 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { // serve for the life of the process. The rollback itself lives in // activate()'s catch and is NOT covered here — driving it needs a real // on-disk package plus catalog/vault wiring. Tracked as a test gap in - // specs/470-dev-platform-plugin/plan.md. + // the epic #470 spec set under `specs/`. const registry = newTestRouteRegistry(); const { runtime } = makeRuntime(registry, new UiRouteCatalog()); registry.register('/api/v1/half-built', Router(), '@plugin/broken'); @@ -204,7 +203,7 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { pluginRouter.get('/ping', (_req, res) => { res.status(200).json({ from: 'plugin' }); }); - registry.register('/api/v1/dev-runner', pluginRouter, '@plugin/dev'); + registry.register('/api/v1/example-plugin', pluginRouter, '@plugin/dev'); seedActive(runtime, '@plugin/dev'); const app = express(); @@ -214,13 +213,13 @@ describe('ToolPluginRuntime.deactivate — route disposal', () => { res.status(404).json({ from: 'fallthrough' }); }); - const live = await getJson<{ from: string }>(app, '/api/v1/dev-runner/ping'); + const live = await getJson<{ from: string }>(app, '/api/v1/example-plugin/ping'); assert.equal(live.status, 200); assert.deepEqual(live.body, { from: 'plugin' }); await runtime.deactivate('@plugin/dev'); - const dead = await getJson<{ from: string }>(app, '/api/v1/dev-runner/ping'); + const dead = await getJson<{ from: string }>(app, '/api/v1/example-plugin/ping'); assert.equal( dead.status, 404, diff --git a/middleware/test/uiNavigationRoute.test.ts b/middleware/test/uiNavigationRoute.test.ts index 0bb0337c1..414d388a4 100644 --- a/middleware/test/uiNavigationRoute.test.ts +++ b/middleware/test/uiNavigationRoute.test.ts @@ -11,7 +11,7 @@ import { getJson, invoke } from './_helpers/httpInvoke.js'; /** * `GET /api/v1/ui/navigation` — the shell's dynamic nav source - * (specs/470-dev-platform-plugin). + * (epic #470). * * Driven through `app.handle` rather than a listening socket: the suite runs * files concurrently and port-holding tests make unrelated socket tests flaky @@ -56,12 +56,12 @@ describe('GET /api/v1/ui/navigation', () => { let app: Express; before(() => { - catalog.registerNav('core:dev-platform', { - navId: 'devPlatform', - href: '/admin/dev-platform', + catalog.registerNav('core:example-plugin', { + navId: 'examplePlugin', + href: '/admin/example-plugin', cluster: 'adminCluster', order: 50, - label: { en: 'Dev Platform', de: 'Dev-Plattform' }, + label: { en: 'Example Plugin', de: 'Beispiel-Plugin' }, }); catalog.registerNav('@plugin/reports', { navId: 'reports', @@ -87,7 +87,7 @@ describe('GET /api/v1/ui/navigation', () => { assert.deepEqual( body.entries.map((e) => [e.navId, e.label]), [ - ['devPlatform', 'Dev Platform'], + ['examplePlugin', 'Example Plugin'], ['reports', 'Reports'], ], 'sorted by order (50 before default 100), labels resolved', @@ -98,8 +98,8 @@ describe('GET /api/v1/ui/navigation', () => { const { body } = await getJson(app, '/api/v1/ui/navigation?locale=de'); assert.equal(body.locale, 'de'); assert.equal( - body.entries.find((e) => e.navId === 'devPlatform')?.label, - 'Dev-Plattform', + body.entries.find((e) => e.navId === 'examplePlugin')?.label, + 'Beispiel-Plugin', ); assert.equal( body.entries.find((e) => e.navId === 'reports')?.label, @@ -111,7 +111,7 @@ describe('GET /api/v1/ui/navigation', () => { it('never leaks the per-locale label map to the browser', async () => { const res = await invoke(app, 'GET', '/api/v1/ui/navigation'); assert.equal( - res.text.includes('Dev-Plattform'), + res.text.includes('Beispiel-Plugin'), false, 'the de label must not ship in an en response', ); diff --git a/middleware/test/uiRouteCatalogNav.test.ts b/middleware/test/uiRouteCatalogNav.test.ts index f9047f60a..d418c587f 100644 --- a/middleware/test/uiRouteCatalogNav.test.ts +++ b/middleware/test/uiRouteCatalogNav.test.ts @@ -4,7 +4,7 @@ import { describe, it } from 'node:test'; import { UiRouteCatalog } from '../src/platform/uiRouteCatalog.js'; /** - * Nav-entry half of the UI catalogue (specs/470-dev-platform-plugin). + * Nav-entry half of the UI catalogue (epic #470). * * The uiRoute-descriptor half is covered by uiRouteCatalog.test.ts. These * tests focus on what is new and what is dangerous: nav entries are @@ -12,14 +12,14 @@ import { UiRouteCatalog } from '../src/platform/uiRouteCatalog.js'; * supplies is treated as untrusted input. */ -const LABEL = { en: 'Dev Platform', de: 'Dev-Plattform' } as const; +const LABEL = { en: 'Example Plugin', de: 'Beispiel-Plugin' } as const; function validEntry( overrides: Partial[1]> = {}, ): Parameters[1] { return { - navId: 'devPlatform', - href: '/admin/dev-platform', + navId: 'examplePlugin', + href: '/admin/example-plugin', label: LABEL, ...overrides, }; @@ -33,11 +33,11 @@ describe('UiRouteCatalog — nav entries', () => { const entries = cat.listNav('en'); assert.equal(entries.length, 1); assert.equal(entries[0]?.pluginId, '@plugin/dev'); - assert.equal(entries[0]?.navId, 'devPlatform'); - assert.equal(entries[0]?.href, '/admin/dev-platform'); + assert.equal(entries[0]?.navId, 'examplePlugin'); + assert.equal(entries[0]?.href, '/admin/example-plugin'); assert.equal(entries[0]?.cluster, 'adminCluster'); assert.equal(entries[0]?.order, 100, 'order defaults to 100'); - assert.equal(entries[0]?.label, 'Dev Platform'); + assert.equal(entries[0]?.label, 'Example Plugin'); }); it('omits cluster entirely when not supplied (top-level entry)', () => { @@ -50,7 +50,7 @@ describe('UiRouteCatalog — nav entries', () => { it('resolves the exact locale when present', () => { const cat = new UiRouteCatalog(); cat.registerNav('@plugin/dev', validEntry()); - assert.equal(cat.listNav('de')[0]?.label, 'Dev-Plattform'); + assert.equal(cat.listNav('de')[0]?.label, 'Beispiel-Plugin'); }); it('falls back to the base language for a regional locale', () => { @@ -58,7 +58,7 @@ describe('UiRouteCatalog — nav entries', () => { cat.registerNav('@plugin/dev', validEntry()); assert.equal( cat.listNav('de-AT')[0]?.label, - 'Dev-Plattform', + 'Beispiel-Plugin', 'de-AT should fall back to de, not to en', ); }); @@ -66,7 +66,7 @@ describe('UiRouteCatalog — nav entries', () => { it('falls back to en for an untranslated locale', () => { const cat = new UiRouteCatalog(); cat.registerNav('@plugin/dev', validEntry()); - assert.equal(cat.listNav('fr')[0]?.label, 'Dev Platform'); + assert.equal(cat.listNav('fr')[0]?.label, 'Example Plugin'); }); it('requires an en label as the guaranteed fallback', () => { @@ -87,7 +87,7 @@ describe('UiRouteCatalog — nav entries', () => { ['/\\evil.example/pwn', 'backslash normalised to // by browsers'], ['https://evil.example', 'absolute URL'], ['javascript:alert(1)', 'scheme'], - ['admin/dev-platform', 'relative path'], + ['admin/example-plugin', 'relative path'], ['/admin/dev platform', 'whitespace'], ]; @@ -101,7 +101,7 @@ describe('UiRouteCatalog — nav entries', () => { it('accepts a normal in-app path', () => { const cat = new UiRouteCatalog(); assert.doesNotThrow(() => - cat.registerNav('@p/x', validEntry({ href: '/admin/dev-platform' })), + cat.registerNav('@p/x', validEntry({ href: '/admin/example-plugin' })), ); }); }); @@ -240,7 +240,7 @@ describe('UiRouteCatalog — nav entries', () => { it('accepts the canonical spelling of a nested path', () => { const cat = new UiRouteCatalog(); assert.doesNotThrow(() => - cat.registerNav('@p/x', validEntry({ href: '/admin/dev-platform' })), + cat.registerNav('@p/x', validEntry({ href: '/admin/example-plugin' })), ); }); diff --git a/scripts/check-core-decoupling.mjs b/scripts/check-core-decoupling.mjs index bf8ef6569..b19035397 100644 --- a/scripts/check-core-decoupling.mjs +++ b/scripts/check-core-decoupling.mjs @@ -16,11 +16,12 @@ * * node scripts/check-core-decoupling.mjs # verify against baseline * node scripts/check-core-decoupling.mjs --report # per-zone breakdown - * node scripts/check-core-decoupling.mjs --update # lower the baseline + * node scripts/check-core-decoupling.mjs --update # record the count * - * `--update` only ever lowers it. Raising the baseline requires editing the - * committed value by hand, which is exactly the kind of change that should - * show up in a diff and be argued for in review. + * As of C13 the extraction is complete and the check asserts ZERO outright — + * see `EXTRACTION_COMPLETE` below. The baseline file remains committed as the + * record of where the count landed, and `--report` still shows per-zone + * deltas against it, but it no longer decides pass/fail. */ import { readFileSync, writeFileSync, existsSync } from 'node:fs'; @@ -34,10 +35,36 @@ const BASELINE_FILE = path.join( 'specs/470-dev-platform-plugin/decoupling-baseline.json', ); +/** + * Epic #470 C13 — the extraction is FINISHED and the floor is hard zero. + * + * While the work was in flight this was a ratchet: a committed count that was + * allowed to fall and never rise. That shape was right for a migration in + * progress and is wrong now. A ratchet parked at zero still reads its floor + * out of a JSON file, and a JSON file is editable — one hand-edit and core can + * legally re-acquire a reference again, which is precisely the outcome the + * whole epic exists to prevent. + * + * So the check no longer asks the baseline what "good" means. Any reference at + * all fails, whatever `decoupling-baseline.json` says. The baseline file stays + * committed as the record of where the count landed and to give `--report` its + * per-zone deltas, but it is no longer load-bearing for pass/fail. + * + * If the Dev Platform ever needs to come BACK into core, that is a real + * architectural decision: flip this to `false`, restore a baseline, and argue + * for it in review. It must not be reachable by editing a number. + */ +const EXTRACTION_COMPLETE = true; + /** * Identifiers that only exist because the Dev Platform lives in core. * Deliberately literal — a broad `/dev/i` would drown in false positives * ("developer", "device", "devDependencies"). + * + * This array is itself 21 matches, which is why `EXCLUDE_GLOBS` below skips + * this file: a detector that counts its own detector definitions can never + * reach zero, and "zero except for the 27 that are the tool" is not a + * property anyone can check at a glance. See the note there. */ const PATTERNS = [ 'devplatform', @@ -71,6 +98,34 @@ const NOT_DEV_PLATFORM = [ /DEV_ENDPOINTS_ENABLED/, // core dev-graph endpoints (/api/dev/*) /devteam/i, // dashboard onboarding persona /salesDev/, // builder persona template + + // --------------------------------------------------------------------- + // Epic #470 C13 — the two things that must survive at a permanent zero. + // + // Both are HISTORICAL RECORD rather than coupling. Core reaching zero means + // "no core code path, config key, fixture or comment refers to the Dev + // Platform"; it cannot mean "rewrite what already happened", because a + // record you are allowed to edit is not a record. Each entry is anchored on + // a specific path so it cannot quietly widen into a general amnesty. + // --------------------------------------------------------------------- + + // Migrations 0022–0030 created the Dev Platform's nine tables while it lived + // in core, so every deployment that ran them has those FILENAMES in its + // `schema_migrations` ledger. C11's plugin-side migrator seeds its own ledger + // from exactly those donor rows (matched by filename, each guarded by a + // schema witness) so the plugin does not re-run DDL that already applied. + // Rename or reword one and the handoff stops matching on the installations + // that need it most. The DDL body is equally frozen: it names the real table + // and column names (`dev_jobs`, `dev_job_events`, …) that exist in those + // databases right now. + /^middleware\/migrations\/00(?:2[2-9]|30)_[^:]*:/, + + // A published changelog entry for a released version of `@omadia/plugin-api`. + // It exists to be FOUND: it spells out the removed exports on one line so a + // consumer grepping its own source for `DevJobKind` lands on the entry that + // explains where the type went. Rewording it to satisfy this ratchet would + // break the one job it has and would misreport what that version shipped. + /^middleware\/packages\/plugin-api\/CHANGELOG\.md:/, ]; /** Zones that must end up clean. Paths are repo-relative. */ @@ -104,6 +159,27 @@ const EXCLUDE_GLOBS = [ '!**/*.tsbuildinfo', '!**/package-lock.json', '!**/*.map', + + // This file, anchored to its EXACT repo-relative path. `PATTERNS` above has + // to spell out the 21 identifiers it hunts for, and the prose has to explain + // them, so an unfiltered scan of the `scripts` zone counted 27 hits against + // the detector itself. That is not a coupling — nothing here imports, calls, + // configures or routes to the Dev Platform — but it is indistinguishable + // from one in the total, and it made the target "27" instead of "0". A + // ratchet whose floor is a magic number nobody can verify at a glance is a + // ratchet people stop reading. + // + // The path anchor matters. A basename wildcard also hides ANY other file of + // this name dropped anywhere under a scanned zone, which turns + // "self-exclusion" into a silent amnesty for same-named files inside core — + // a probe under `middleware/src/` carrying real identifiers scanned as + // clean. Only the one real detector file is meant to be excluded. + // + // Self-exclusion is safe precisely because this file is the detector: it has + // no runtime, ships in no image, and adding a pattern here can only ever + // make the check stricter. The narrower alternative — skipping just the + // `PATTERNS` array by line range — would break the moment the array moved. + '!scripts/check-core-decoupling.mjs', ]; function rgCount(zone) { @@ -184,6 +260,16 @@ if (mode === '--report') { if (mode === '--update') { const baseline = loadBaseline(); + if (EXTRACTION_COMPLETE && result.total > 0) { + console.error( + `refusing to record a non-zero baseline: ${String(result.total)} reference(s) found.\n` + + 'The extraction is complete (EXTRACTION_COMPLETE = true), so zero is the\n' + + 'only baseline this script will write. Remove the references, or make the\n' + + 'architectural argument for reversing the extraction.\n' + + 'Run `node scripts/check-core-decoupling.mjs --report` for the breakdown.', + ); + process.exit(1); + } if (baseline && result.total > baseline.total) { console.error( `refusing to raise the baseline: ${String(result.total)} > ${String(baseline.total)}.\n` + @@ -199,6 +285,29 @@ if (mode === '--update') { process.exit(0); } +if (EXTRACTION_COMPLETE) { + if (result.total === 0) { + console.log('Core is free of Dev Platform references.'); + process.exit(0); + } + const offenders = Object.entries(result.zones) + .filter(([, count]) => count > 0) + .map(([name, count]) => ` ${name}: ${String(count)}`); + console.error( + `Core re-acquired Dev Platform references: ${String(result.total)} found, 0 allowed.\n\n` + + `${offenders.join('\n')}\n\n` + + 'The Dev Platform lives in its own repository (epic #470). Core carries no\n' + + 'code path, config key, i18n key, fixture or comment that names it, and\n' + + 'that is enforced as an absolute — there is no baseline to raise.\n\n' + + 'If you are adding a plugin integration point, name it generically: the\n' + + 'mechanisms are manifest-declared (permissions.public_paths + operator\n' + + 'grants, the service registry, the UI route catalogue), so core never has\n' + + 'to name a particular plugin.\n\n' + + 'Run `node scripts/check-core-decoupling.mjs --report` for the breakdown.', + ); + process.exit(1); +} + const baseline = loadBaseline(); if (!baseline) { console.error( diff --git a/scripts/check-core-decoupling.test.mjs b/scripts/check-core-decoupling.test.mjs new file mode 100644 index 000000000..ea7e7ea7a --- /dev/null +++ b/scripts/check-core-decoupling.test.mjs @@ -0,0 +1,103 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import test from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const SCRIPT = path.join(REPO_ROOT, 'scripts/check-core-decoupling.mjs'); +const PROBE_DIR = path.join(REPO_ROOT, 'middleware/src/__probe'); +const PROBE_FILE = path.join(PROBE_DIR, 'check-core-decoupling.mjs'); +const PROBE_TOKENS = [ + ['dev', 'platform'].join(''), + ['dev', 'Runner'].join(''), + ['dev', '_job'].join(''), +]; +const PROBE_BODY = `${PROBE_TOKENS.join(' ')}\n`; + +function runChecker(args = []) { + return spawnSync('node', [SCRIPT, ...args], { + cwd: REPO_ROOT, + encoding: 'utf8', + }); +} + +test('anchored self-exclusion hides only the real detector file', () => { + rmSync(PROBE_DIR, { recursive: true, force: true }); + + const clean = runChecker(); + assert.equal( + clean.status, + 0, + `expected a clean tree to pass, got:\n${clean.stderr || clean.stdout}`, + ); + assert.match(clean.stdout, /Core is free of Dev Platform references\./); + + mkdirSync(PROBE_DIR, { recursive: true }); + writeFileSync(PROBE_FILE, PROBE_BODY); + + try { + const hit = runChecker(); + assert.equal( + hit.status, + 1, + `expected the probe file to trip the checker, got:\n${hit.stderr || hit.stdout}`, + ); + assert.match(hit.stderr, /Core re-acquired Dev Platform references: 1 found, 0 allowed\./); + assert.match(hit.stderr, /middleware\/src: 1/); + } finally { + rmSync(PROBE_DIR, { recursive: true, force: true }); + } + + const cleanAgain = runChecker(); + assert.equal( + cleanAgain.status, + 0, + `expected the tree to return to clean after removing the probe, got:\n${cleanAgain.stderr || cleanAgain.stdout}`, + ); +}); + +test('the script zone remains clean because the exact detector path is excluded', () => { + const rg = spawnSync( + 'rg', + [ + '--no-config', + '--no-heading', + '--with-filename', + '--line-number', + ...PROBE_TOKENS.flatMap((token) => ['-e', token]), + '--glob', + '!**/node_modules/**', + '--glob', + '!**/dist/**', + '--glob', + '!**/.next/**', + '--glob', + '!**/*.tsbuildinfo', + '--glob', + '!**/package-lock.json', + '--glob', + '!**/*.map', + '--glob', + '!scripts/check-core-decoupling.mjs', + '--', + 'scripts', + ], + { + cwd: REPO_ROOT, + encoding: 'utf8', + }, + ); + + assert.equal( + rg.status, + 1, + `expected no hits in the scripts zone, got:\n${rg.stderr || rg.stdout}`, + ); + assert.equal( + rg.stdout, + '', + 'the anchored exclusion must still hide the one real detector file from the scripts zone', + ); +}); diff --git a/specs/470-dev-platform-plugin/README.md b/specs/470-dev-platform-plugin/README.md index d1439bc7d..4b4902818 100644 --- a/specs/470-dev-platform-plugin/README.md +++ b/specs/470-dev-platform-plugin/README.md @@ -388,15 +388,48 @@ Three different jobs; none of them is sufficient alone. ```bash node scripts/check-core-decoupling.mjs # verify (CI runs this) node scripts/check-core-decoupling.mjs --report # per-zone breakdown -node scripts/check-core-decoupling.mjs --update # lower the baseline +node scripts/check-core-decoupling.mjs --update # record the count ``` +The check counts Dev Platform references across 14 disjoint zones. Baseline **0** — every +zone CLEAN, pinned there permanently by C13 (3,300 → 214 at C10 → 0 at C13). + +**It is no longer a ratchet.** While the extraction was in flight the committed count was +allowed to fall and never rise, which was the right shape for a migration in progress and is +the wrong shape now: a ratchet parked at zero still reads its floor out of a JSON file, and a +JSON file is editable. So `EXTRACTION_COMPLETE` in the script makes the check assert `0` +outright and ignore the baseline for pass/fail. Any reference fails CI, and `--update` +refuses to write a non-zero baseline. Bringing the Dev Platform back into core is a real +architectural decision that has to be argued for in review — not a number someone edits. + +Two things are allowlisted, both historical record rather than coupling, each anchored on an +exact path so the exemption cannot widen: + +| Allowlisted | Why it cannot be reworded | +|---|---| +| `middleware/migrations/00{22..30}_*` | Every deployment that ran them has these FILENAMES in its `schema_migrations` ledger, and C11's plugin-side migrator seeds its own ledger from exactly those donor rows. Rename one and the handoff stops matching on the installations that need it most. The DDL body names the real tables and columns that exist in those databases right now. | +| `middleware/packages/plugin-api/CHANGELOG.md` | A published entry for a released version. It exists to be FOUND — it spells out the removed exports so a consumer grepping their own source lands on the entry explaining where the type went. Rewording it would misreport what that version shipped. | + +The script also excludes **itself**: `PATTERNS` has to spell out the 21 identifiers it hunts +for, so an unfiltered scan counted 27 hits against the detector and made the target `27` +instead of `0`. Self-exclusion is safe precisely because it is the detector — no runtime, no +image, and adding a pattern there can only make the check stricter. + +`services/githubAppJwt.ts` **stays in core** and is deliberately not allowlisted: it carries +no matching identifier because it is generic GitHub App auth. Moving it into the plugin +repository would recreate the reverse dependency across a repo boundary. The ratchet counts Dev Platform references across 14 disjoint zones and **fails if the count rises, per zone**. Baseline **206** (C10 took it down from 3,300). It only ever falls; raising it needs a hand-edited baseline, so a new coupling shows up in review instead of slipping in. That is what makes the checklist's staleness survivable — a file inventory goes stale on -contact, but the count does not, and it cannot reach zero while a reference survives. +contact, but the count does not. + +**Zero counts identifiers, not the English name.** A comment may still say "the dev platform +plugin"; what cannot survive is an identifier-shaped reference — an import, a route, a config +key, an i18n key, a fixture string, a `devPlatform`/`dev-runner`/`DEV_JOB` token. That is the +line the patterns draw, and it is the right one: prose describes history, identifiers create +coupling. But it counts IDENTIFIERS, NOT BEHAVIOUR: zero is a necessary condition for done, not a sufficient one. Sections 2 and 3 of `acceptance.md` cover the rest, and neither is automated. diff --git a/specs/470-dev-platform-plugin/acceptance.md b/specs/470-dev-platform-plugin/acceptance.md index a96cd92bb..0177798b5 100644 --- a/specs/470-dev-platform-plugin/acceptance.md +++ b/specs/470-dev-platform-plugin/acceptance.md @@ -8,7 +8,8 @@ Two questions the other two documents do **not** answer: This file is the functional contract: every capability the Dev Platform provides today, who owns it after extraction, and **how you check it still works**. Extraction is done when -every row passes *and* the decoupling ratchet reads zero. +every row passes *and* the decoupling check reads zero. **It reads zero as of C13** — which +settles the automated half only; §2 and §3 below are still the parts a human has to walk. --- @@ -16,8 +17,9 @@ every row passes *and* the decoupling ratchet reads zero. | Guard | What it proves | Status | |---|---|---| -| `scripts/check-core-decoupling.mjs` + CI job `core decoupling ratchet (#470)` | Core does not re-acquire Dev Platform references while the extraction is in flight | **In place.** Baseline **206** across **14** zones, per-zone regression check (C10 dropped it from 3,300; the survivors are C11's migrations, C12's `publicPaths` exemptions and C13's residue) | +| `scripts/check-core-decoupling.mjs` + CI job `core decoupling ratchet (#470)` | Core carries **no** Dev Platform reference — code path, config key, i18n key, fixture string or comment | **DONE. Baseline 0**, all **14** zones CLEAN (3,300 → 214 at C10 → 0 at C13). No longer a ratchet: `EXTRACTION_COMPLETE` makes the check assert `0` outright, so there is no baseline to raise. Allowlisted as historical record, path-anchored: migrations `00{22..30}_*` (C11's ledger handoff matches on those filenames) and `plugin-api/CHANGELOG.md` (a published entry for a released version). The script self-excludes its own `PATTERNS` array — 27 hits that were the detector, not a coupling. | | `middleware/test/devplatform/**` (58 files) | The behaviour itself, at unit/integration level. These **moved with the plugin** and must stay green in the new repo | **Moved** — deleted from core in C10; `byte5ai/omadia-dev-platform` owns them | +| `middleware/test/auth/staticPublicPathsClosedSet.test.ts` | `STATIC_PUBLIC_PATHS` is a closed, core-owned set — core exempts nothing from the session gate on a plugin's behalf, and a path off the list 401s before routing | **In place (C12).** Mutation-checked: restoring either deleted exemption fails the suite and names it | | §2 capability matrix below | Nothing is silently dropped in the move | **Written here; not yet automated** | | §3 install/uninstall | The result is genuinely installable | **Not yet built** — needs P3/P4 | @@ -26,10 +28,10 @@ Run the ratchet: ```bash node scripts/check-core-decoupling.mjs # verify (CI runs this) node scripts/check-core-decoupling.mjs --report # per-zone breakdown -node scripts/check-core-decoupling.mjs --update # lower the baseline (never raises) +node scripts/check-core-decoupling.mjs --update # record the count (refuses non-zero) ``` -The count may only fall, **per zone** — an aggregate-only check would pass while one zone +The floor is zero and absolute. Historically the count could only fall, **per zone** — an aggregate-only check would pass while one zone fell and another rose, which is what a half-finished move looks like. Raising a baseline requires hand-editing the committed file, so a new coupling shows up in review. diff --git a/specs/470-dev-platform-plugin/decoupling-baseline.json b/specs/470-dev-platform-plugin/decoupling-baseline.json index eb399799d..1f18d6459 100644 --- a/specs/470-dev-platform-plugin/decoupling-baseline.json +++ b/specs/470-dev-platform-plugin/decoupling-baseline.json @@ -1,19 +1,19 @@ { - "total": 206, + "total": 0, "zones": { - "middleware/src": 14, - "middleware/test": 60, - "middleware/packages": 16, + "middleware/src": 0, + "middleware/test": 0, + "middleware/packages": 0, "middleware/scripts": 0, "middleware/sidecars": 0, - "middleware/migrations": 69, + "middleware/migrations": 0, "middleware/package.json": 0, - "middleware/env-example": 1, - "web-ui/app": 19, + "middleware/env-example": 0, + "web-ui/app": 0, "web-ui/messages": 0, "web-ui/config": 0, "ci-workflows": 0, - "scripts": 27, + "scripts": 0, "compose": 0 } } diff --git a/web-ui/app/_components/Nav.tsx b/web-ui/app/_components/Nav.tsx index ffde6a938..267ea8c0a 100644 --- a/web-ui/app/_components/Nav.tsx +++ b/web-ui/app/_components/Nav.tsx @@ -17,7 +17,7 @@ import type { NavEntryDto } from '../_lib/navigation'; * keep working; the cluster header gets a subtle `contains-active` style * when any of its children matches. * - * Two sources feed this bar (specs/470-dev-platform-plugin): + * Two sources feed this bar (epic #470): * * 1. `NAV` below — the shell's own compiled surfaces. Labels come from * the `nav.*` message catalogue, per web-ui/CLAUDE.md. @@ -77,9 +77,9 @@ const NAV: readonly NavItem[] = [ { kind: 'link', href: '/operator/receipts', key: 'receipts' }, // #760 — miss-report review queue, same operator audience. { kind: 'link', href: '/operator/privacy-reports', key: 'privacyReports' }, - // Dev Platform used to be hardcoded here. It is now contributed at - // runtime (middleware registers it while DEV_PLATFORM_ENABLED), so the - // entry disappears when the feature is off — see mergeNav below. + // An optional feature's entry used to be hardcoded here. Plugin + // surfaces are now contributed at runtime, so the entry appears only + // while that plugin is installed and active — see mergeNav below. ], }, // OM-09 — there was NO in-product help at all: no help route, no `?`, no diff --git a/web-ui/app/_components/tasks/__tests__/taskChatCardState.test.ts b/web-ui/app/_components/tasks/__tests__/taskChatCardState.test.ts index 547ad7eee..a69455608 100644 --- a/web-ui/app/_components/tasks/__tests__/taskChatCardState.test.ts +++ b/web-ui/app/_components/tasks/__tests__/taskChatCardState.test.ts @@ -75,7 +75,7 @@ describe('parseTaskStartResult', () => { describe('isTaskStartToolName', () => { it('matches only the seam start half', () => { expect(isTaskStartToolName('ask_research_start')).toBe(true); - expect(isTaskStartToolName('dev_job_start')).toBe(true); + expect(isTaskStartToolName('example_job_start')).toBe(true); expect(isTaskStartToolName('ask_research_status')).toBe(false); expect(isTaskStartToolName('ask_research_list')).toBe(false); expect(isTaskStartToolName('query_knowledge_graph')).toBe(false); diff --git a/web-ui/app/_lib/__tests__/navMerge.test.ts b/web-ui/app/_lib/__tests__/navMerge.test.ts index a3db2bb37..f993dddb6 100644 --- a/web-ui/app/_lib/__tests__/navMerge.test.ts +++ b/web-ui/app/_lib/__tests__/navMerge.test.ts @@ -5,7 +5,7 @@ import type { NavEntryDto } from '../navigation'; /** * Merge of the shell's static nav with plugin-contributed entries - * (specs/470-dev-platform-plugin). Pure logic — no DOM. + * (epic #470). Pure logic — no DOM. */ type StaticItem = Parameters[0][number]; @@ -28,9 +28,9 @@ const translate = (key: string): string => `T:${key}`; function entry(over: Partial = {}): NavEntryDto { return { pluginId: '@plugin/dev', - navId: 'devPlatform', - href: '/admin/dev-platform', - label: 'Dev Platform', + navId: 'examplePlugin', + href: '/admin/example-plugin', + label: 'Example Plugin', order: 100, ...over, }; @@ -59,19 +59,19 @@ describe('mergeNav', () => { expect(cluster?.kind === 'cluster' && cluster.children).toEqual([ { href: '/admin', label: 'T:admin' }, { href: '/system', label: 'T:system' }, - { href: '/admin/dev-platform', label: 'Dev Platform' }, + { href: '/admin/example-plugin', label: 'Example Plugin' }, ]); }); it('uses the plugin-supplied label verbatim, not a catalogue lookup', () => { const merged = mergeNav( STATIC, - [entry({ cluster: 'adminCluster', label: 'Dev-Plattform' })], + [entry({ cluster: 'adminCluster', label: 'Beispiel-Plugin' })], translate, ); const cluster = merged.find((i) => i.kind === 'cluster'); expect(cluster?.kind === 'cluster' && cluster.children.at(-1)?.label).toBe( - 'Dev-Plattform', + 'Beispiel-Plugin', ); }); @@ -93,8 +93,8 @@ describe('mergeNav', () => { ); expect(merged.at(-1)).toEqual({ kind: 'link', - href: '/admin/dev-platform', - label: 'Dev Platform', + href: '/admin/example-plugin', + label: 'Example Plugin', }); }); @@ -183,7 +183,7 @@ describe('bestPrefixMatch', () => { const leaves = [ { href: '/', label: 'Dashboard' }, { href: '/admin', label: 'Admin' }, - { href: '/admin/dev-platform', label: 'Dev Platform' }, + { href: '/admin/example-plugin', label: 'Example Plugin' }, ]; it('matches the root exactly, never as a prefix', () => { @@ -192,8 +192,8 @@ describe('bestPrefixMatch', () => { }); it('prefers the longest matching prefix, including a plugin leaf', () => { - expect(bestPrefixMatch('/admin/dev-platform/jobs/42', leaves)).toBe( - '/admin/dev-platform', + expect(bestPrefixMatch('/admin/example-plugin/jobs/42', leaves)).toBe( + '/admin/example-plugin', ); }); @@ -201,7 +201,7 @@ describe('bestPrefixMatch', () => { // A bare startsWith would light up /admin while on /administrator, and // let a plugin leaf /reports claim /reports-old. expect(bestPrefixMatch('/administrator', leaves)).toBe(''); - expect(bestPrefixMatch('/admin/dev-platform-old', leaves)).toBe('/admin'); + expect(bestPrefixMatch('/admin/example-plugin-old', leaves)).toBe('/admin'); }); it('returns empty for an unmatched path and for a null pathname', () => { @@ -210,8 +210,8 @@ describe('bestPrefixMatch', () => { }); it('stops highlighting a plugin leaf once the plugin is gone', () => { - const withoutPlugin = leaves.filter((l) => l.href !== '/admin/dev-platform'); - expect(bestPrefixMatch('/admin/dev-platform/jobs/42', withoutPlugin)).toBe( + const withoutPlugin = leaves.filter((l) => l.href !== '/admin/example-plugin'); + expect(bestPrefixMatch('/admin/example-plugin/jobs/42', withoutPlugin)).toBe( '/admin', ); }); diff --git a/web-ui/app/_lib/__tests__/navParse.test.ts b/web-ui/app/_lib/__tests__/navParse.test.ts index bf79e2c88..dd1304ee8 100644 --- a/web-ui/app/_lib/__tests__/navParse.test.ts +++ b/web-ui/app/_lib/__tests__/navParse.test.ts @@ -11,10 +11,10 @@ import { parseEntries } from '../navigation'; */ const ok = { - pluginId: 'core:dev-platform', - navId: 'devPlatform', - href: '/admin/dev-platform', - label: 'Dev Platform', + pluginId: 'core:example-plugin', + navId: 'examplePlugin', + href: '/admin/example-plugin', + label: 'Example Plugin', order: 50, cluster: 'adminCluster', }; @@ -108,7 +108,7 @@ describe('parseEntries', () => { it('keeps the good entries when one is malformed', () => { const parsed = parseEntries(wrap(ok, { ...ok, href: '//evil.example' })); expect(parsed).toHaveLength(1); - expect(parsed[0]?.href).toBe('/admin/dev-platform'); + expect(parsed[0]?.href).toBe('/admin/example-plugin'); }); }); diff --git a/web-ui/app/admin/page.tsx b/web-ui/app/admin/page.tsx index 5ede2dc3f..176e0367a 100644 --- a/web-ui/app/admin/page.tsx +++ b/web-ui/app/admin/page.tsx @@ -29,7 +29,7 @@ type CardDef = { * Marks a card as belonging to an optional feature: it renders only when * *this specific* plugin has contributed a nav entry for this href. Keeps * the grid honest about what is actually installed instead of linking to - * a page that would answer 403 — see specs/470-dev-platform-plugin. + * a page that would answer 403 — see the epic #470 spec set. * * Matching on the contributing plugin id, not just the href, so an * unrelated plugin cannot resurrect a core card by claiming the path.