From fd470eec0877820c42c7076aea0f8f726dd993af Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Fri, 21 Aug 2026 08:21:24 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20activate=20against=20core=20main=20?= =?UTF-8?q?=E2=80=94=20pluginUi=20nav,=20optional=5Frequires=20(0.3.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verification pass against omadia core origin/main (9feb3ad3), the first core carrying the whole C9-C11 contract surface, with @omadia/plugin-api 1.5.0. Two real defects, both of which stopped the plugin working against that core. The nav entry hand-built a percent-encoded href. That is the one spelling core's HREF_SEGMENT refuses, and #798 deliberately kept the rule strict rather than widening it as the 2026-08-20 acceptance run's local patch assumed — so registerNav threw, the throw propagated out of activateInner, and the plugin did not activate at all. C9 closed this from the other side: declare `pluginUi: true` and the kernel renders the identical URL from the id it already holds. A recording double accepts any nav entry, which is why a green suite never saw it; the shape is now asserted directly. turnContext@1, githubAppJwt@1, usageTelemetry@1 and conductorRoles@1 move from `requires:` to `optional_requires:`. Core's capability resolver and install gate read `requires:` only, so listing a survivable capability there made this plugin uninstallable on stock core for four capabilities activate() is written to live without, each with a logged degradation. graphPool@1 stays the single hard requirement. Resolution moves to ctx.services.getOptional behind an optional-method guard, so a core predating plugin-api 1.4.0 still activates through the get() fallback; both paths are driven by tests. The nine C11 seedLedger witnesses needed no change: each is already a single SELECT returning one boolean row, built from to_regclass, information_schema.columns or a pg_constraint join, and they run clean under core's READ ONLY savepoint over the extended protocol. permissions.sql.ledger already satisfies the plg__ rule. The activate() double was lenient on two points that mattered here, and is not any more: services.get/getOptional now throw for a capability declared in neither manifest list, and the declared set is parsed from manifest.yaml rather than restated, so a forgotten entry fails in this suite instead of against a real core. Acceptance: 38 PASS / 33 FAIL / 2 BLOCKED against plain origin/main, where all 33 failures share one cause — the two dev-platform publicPaths exemptions C12 (#807) deletes, not yet merged. Re-run against a core carrying C12: 71 PASS / 0 FAIL / 2 BLOCKED, same artifact, no plugin change. Suite is 1,316 tests, 0 fail, 0 skipped with the Postgres suites running. Records a new core gap (G7) in docs/ACCEPTANCE-RUN-2026-08-21.md: core runs ctx.sql.runMigrations() itself before activate(), so the C11 handoff can only ever report alreadySeeded and its skippedNoWitness alarm never fires. --- CHANGELOG.md | 65 ++++++ docs/ACCEPTANCE-RUN-2026-08-21.md | 297 ++++++++++++++++++++++++ package-lock.json | 8 +- packages/plugin/manifest.yaml | 66 ++++-- packages/plugin/package.json | 2 +- packages/plugin/src/plugin.ts | 66 +++++- packages/plugin/test/activate.test.ts | 178 +++++++++++++- packages/plugin/test/manifest.test.ts | 27 ++- packages/runner-shim/package.json | 2 +- packages/ui/package.json | 2 +- sidecars/dev-runner-daemon/package.json | 2 +- 11 files changed, 666 insertions(+), 49 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 docs/ACCEPTANCE-RUN-2026-08-21.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..0ded2d3 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,65 @@ +# Changelog + +All notable changes to `@omadia/dev-platform`. The version that matters is +`packages/plugin/manifest.yaml` — the hub reads the manifest, and `npm run +package` aborts if it disagrees with `packages/plugin/package.json`. + +## 0.3.0 — 2026-08-21 + +Verification pass against omadia core `origin/main` (`9feb3ad3`), the first core +that carries the whole C9–C11 contract surface: `optional_requires`, +`ctx.services.getOptional`, `pluginUi` nav entries, core migrations at boot, and +`ctx.sql.seedLedger` with the read-only witness fence. `@omadia/plugin-api` is +**1.5.0**. + +### Fixed + +- **The plugin could not activate against core `main` at all.** The nav entry + hand-built a percent-encoded `href` (`/plugin-ui/%40omadia%2Fdev-platform`), + which is the one spelling core's `HREF_SEGMENT` refuses — and #798 + deliberately kept that rule strict rather than widening it. `registerNav` + threw, the throw propagated out of `activateInner`, and activation failed. + The entry now declares `pluginUi: true` and the kernel renders the identical + URL from the id it already holds (C9, closing gap G5 of the 2026-08-20 + acceptance run). `test/activate.test.ts` pins the shape: `pluginUi === true` + and no `href`. + +### Changed + +- **`turnContext@1`, `githubAppJwt@1`, `usageTelemetry@1` and + `conductorRoles@1` moved from `requires:` to `optional_requires:`.** Core's + capability resolver and install gate read `requires:` only, so listing a + survivable capability there made this plugin uninstallable on stock core for + four capabilities `activate()` is explicitly written to live without — gap G2 + of the acceptance run. Each still logs the degradation it takes. `graphPool@1` + stays the single hard requirement. +- **Optional capabilities resolve through `ctx.services.getOptional`** (added in + plugin-api 1.4.0) instead of `ctx.services.get` inside a `try`. The accessor + is called through an optional-method guard, so a core that predates it still + activates through the `get()` fallback; a test drives both paths. + +### Testing + +- The `activate()` double is now faithful on two points it was lenient about: + `services.get`/`getOptional` throw for a capability the manifest declares in + neither list, and the declared set is **parsed from `manifest.yaml`** rather + than restated in the test, so a forgotten manifest entry fails here instead of + against a real core. +- `test/manifest.test.ts` asserts the exact `requires:` / `optional_requires:` + sets, not just membership — the regression this guards against arrives one + promoted line at a time. + +### Unchanged, and verified so + +- The nine `seedLedger` witnesses in `src/ledgerHandoff.ts` already satisfy the + C11 fence: each is a single `SELECT` returning one boolean row, built from + `to_regclass`, `information_schema.columns` or a `pg_constraint` join — no + casts that throw on a missing relation, no multi-statement strings, no writes. +- `permissions.sql.ledger` is `plg_omadia_dev_platform_migrations`, inside the + `plg__` namespace the kernel enforces. + +## 0.2.0 — 2026-08-20 + +Extraction of the Dev Platform out of omadia core into this repository (epic +byte5ai/omadia#470, P1–P5 plus C11). See +`docs/ACCEPTANCE-RUN-2026-08-20.md` for the acceptance run that closed it. diff --git a/docs/ACCEPTANCE-RUN-2026-08-21.md b/docs/ACCEPTANCE-RUN-2026-08-21.md new file mode 100644 index 0000000..fb935d7 --- /dev/null +++ b/docs/ACCEPTANCE-RUN-2026-08-21.md @@ -0,0 +1,297 @@ +# Acceptance run — 2026-08-21 + +Artifact under test: `@omadia/dev-platform` **0.3.0** +(`omadia-dev-platform-0.3.0.zip`, 525,131 bytes, sha256 `832f659cff39…`). +No hub publish, no staging Fly. + +Successor to `ACCEPTANCE-RUN-2026-08-20.md`. That run needed a core built from +`main` + an unmerged C6 branch + four cherry-picked C7 commits + **five local +patches**. This one needs none of them: every gap it recorded except one has +landed on core `main`, and the last one is a merged-but-unreleased branch. + +--- + +## 1. Headline + +**The plugin was broken against core `main` and is now fixed.** Not "would have +degraded" — `registerNav` threw, the throw propagated out of `activateInner`, +and the plugin did not activate. See §4. + +Two runs, same artifact, same script, two cores: + +| Core | PASS | FAIL | BLOCKED | Rows | +|---|---|---|---|---| +| `origin/main` @ `9feb3ad3` | 38 | **33** | 2 | 73 | +| `origin/main` + C12 (`feat/470-c13-residue` @ `322afd19`) | **71** | **0** | 2 | 73 | + +**All 33 failures on `main` have one cause**, and it is a core residue the epic +already scheduled: the two dev-platform `publicPaths` exemptions C12 deletes. +Removing them turns 33 FAIL into 33 PASS with no change to the artifact. The +verdict on the plugin is the second row. + +The two BLOCKED rows are the same two as 2026-08-20, both honest, neither new: +core exposes no tool-registry endpoint to probe chat-tool registration over +HTTP, and `acceptance.md` §3.15 has not decided what happens to grant rows on +uninstall. + +--- + +## 2. What was run against what + +| | | +|---|---| +| Plugin | `omadia-dev-platform` @ `chore/verify-against-core-1.5.0`, artifact `omadia-dev-platform-0.3.0.zip` | +| Core (primary) | `byte5ai/omadia` `origin/main` @ `9feb3ad3` — **no patches, no cherry-picks** | +| Core (C12 comparison) | `feat/470-c13-residue` @ `322afd19` (= main + C12 + C13) | +| `@omadia/plugin-api` | **1.5.0** | +| Node | v22.22.3 (nvm) | +| Postgres | `pgvector/pgvector:pg16`, ports 55441 (main) / 55442 (C12) | +| Core env | `PORT=4111` / `4112`, local password auth. **No `DEV_PLATFORM_ENABLED`** — the key no longer exists in core (C10) and core boots fine without it. | + +Suite: **1,316 tests, 0 fail, 0 skipped** across four workspaces +(`plugin` 786 / `runner-shim` 76 / `dev-runner-daemon` + `ui` 454), with the +Postgres suites actually running. The only remaining skips are two that need a +built `dev-runner` image. + +--- + +## 3. Plugin defects found and fixed + +### P3 — the nav entry could not be registered at all (**the plugin did not activate**) + +``` +UiRouteCatalog.registerNav(@omadia/dev-platform/devPlatform): href segment + '%40omadia%2Fdev-platform' has characters outside [A-Za-z0-9-._~] +``` + +This is gap **G5** of the 2026-08-20 run, and the fix did not go the way that +run assumed. G5's local patch widened `HREF_SEGMENT` to admit percent-octets. +Core went the other way: **#798 deliberately kept the rule strict**, because the +shell decides "core destinations win" by comparing hrefs for string equality and +percent-encoding breaks that comparison — widening it would weaken every literal +href to fix the one path core can spell for itself. + +So the patch this repo was implicitly waiting for was never going to land, and +the hand-built href stayed unregisterable. C9 closed it from the other side: +the plugin declares `pluginUi: true` and the **kernel** renders +`pluginUiHref(id)` from the id it already holds. Byte-identical URL, and no +plugin ever spells a percent-encoded href again. + +Fixed in `src/plugin.ts` §7. `test/activate.test.ts` now asserts the shape — +`pluginUi === true`, no `href` — because a recording double accepts anything, +which is exactly why this survived a green suite in the first place. + +### P4 — four survivable capabilities were declared as hard requirements + +Gap **G2** of the previous run, fixed on the plugin side rather than in core. +`turnContext@1`, `githubAppJwt@1`, `usageTelemetry@1` and `conductorRoles@1` +moved from `requires:` to `optional_requires:` (C9 / core #795). Core's +capability resolver and install gate read `requires:` **only**, so listing a +survivable capability there made this plugin uninstallable on stock core for +four capabilities `activate()` is explicitly written to live without — each with +a logged, documented degradation. `graphPool@1` stays the one hard requirement. + +Resolution moved to `ctx.services.getOptional` (plugin-api 1.4.0), guarded by an +optional-method check so a core that predates it still activates through the +`get()` fallback. Both paths are driven by tests. + +### Verified unchanged + +- **The nine `seedLedger` witnesses already satisfy the C11 fence.** Each is a + single `SELECT` returning one boolean row, built from `to_regclass`, + `information_schema.columns` or a `pg_constraint` join — no casts that throw + on a missing relation, no multi-statement strings, no writes. Nothing to fix: + they ran clean under core's READ ONLY savepoint over the extended protocol. +- **`permissions.sql.ledger`** is `plg_omadia_dev_platform_migrations`, inside + the `plg__` namespace the kernel enforces. + +--- + +## 4. Core gaps — 2026-08-20 list, re-measured + +| Gap | 2026-08-20 | Now | +|---|---|---| +| G1 — C7's SQL gate stops core booting | P0, local patch | **CLOSED** — core boots clean | +| G2 — `requires:` has no optionality | blocks install, local patch | **CLOSED** by C9 `optional_requires` (#795); plugin side fixed here | +| G3 — core ledger is a side effect of an LLM key | P0, migrations applied by hand | **CLOSED** — core ran its own 47 migrations at boot (C9) | +| G4 — failed activation reports `status: "active"` | silent | **OPEN**, see below | +| G5 — no scoped plugin can express a nav href | blocks nav, local patch | **CLOSED** by C9 `pluginUi: true` (#796) | +| G6 — C12 public-path residue | last hard blocker | **merged on the C12 branch, not yet on `main`** | + +### G4 is still open, and this run is the proof + +`§3.9 activation` reported **PASS — `state=active, no error`** on the `main` run, +in which the plugin had already been torn down: + +``` +[@omadia/dev-platform] [dev-platform] activated — worker running +[@omadia/dev-platform] [dev-platform] deactivating +[install] onInstalled hook failed for @omadia/dev-platform: … cannot activate — + public-path declaration '/api/v1/dev-runner' … is already a static core public path +``` + +`activate()` completed — migrations, tools, routers, worker, cron, nav — and +core then rejected the public-path declaration and unwound it. The plugin record +kept saying `active` while nothing was mounted. Every one of the 33 downstream +`404 — route not mounted` rows is that state seen from outside. A row that reads +PASS while the subsystem is dead is the worst-in-class failure the acceptance +document exists to prevent, and it survived into this run unchanged. + +### NEW — G7: core's pre-activate migration run pre-empts the C11 handoff (**C11 is a no-op for the plugin class it was built for**) + +Measured on both cores, against a database carrying core's donor rows **and** all +nine `dev_*` tables — precisely the upgrade C11 exists for. Expected: 9 seeded, +runner applies 0. Observed, in this order: + +``` +[tool-runtime] @omadia/dev-platform: applied 9 migration(s) to ledger + 'plg_omadia_dev_platform_migrations' in 33ms (0022…0030) ← core, BEFORE activate() +[@omadia/dev-platform] [sql] ledger handoff — 0 seeded, 9 already seeded, + 0 left for the migration runner ← the plugin's seedLedger +``` + +Inverted. `toolPluginRuntime.ts:374-394` calls `ctx.sql.runMigrations()` itself, +before `activate()`, whenever the manifest declares `permissions.sql.migrations` +— deliberately, so "the tables exist" is an invariant `activate()` can rely on +(C7/G4). But `seedLedger` is documented to be called **inside `activate()`, +before `runMigrations()`** (`plugin-api/src/pluginContext.ts:2071-2087`), and by +then core's runner has already written all nine ledger rows. The handoff can +only ever report `alreadySeeded`. + +Consequences, in order of severity: + +1. The **`skippedNoWitness` alarm never fires.** That is the one output C11 was + built to produce: donor rows present, schema objects absent — a restore, a + rolled-back deploy, a dropped table. The operator is not told. +2. `dryRun` is unreachable in the real flow, so the plan an operator was meant + to read before a production handoff cannot be produced. +3. The nine files are re-applied on every upgrade — the cost C11 removed. + +Not a data-loss bug: the files are idempotent and re-applying them against a +partially-present schema is itself the repair, so the outcome stays safe. What +is lost is the *detection*. A plugin cannot fix this from its side — core calls +the runner before `activate()` and the witnesses are knowledge only the plugin +has. Core needs to either run the handoff ahead of its own pre-activate runner +(which means the manifest naming a handoff module), or skip the pre-activate run +for a plugin that declares one. + +- Core: `middleware/src/plugins/toolPluginRuntime.ts:374-394` +- Contract: `middleware/packages/plugin-api/src/pluginContext.ts:2071-2087` +- Plugin call site (correct as written): `packages/plugin/src/plugin.ts` §3 + +--- + +## 5. Row-level results + +### Install / uninstall (acceptance.md §3) — C12 core + +| # | Criterion | Verdict | Evidence | +|---|---|---|---| +| 1 | Boots with no `DEV_*` config | PASS | `listening on [127.0.0.1]:4112`; `DEV_PLATFORM_ENABLED` no longer exists | +| 2 | dev-platform endpoints 404 before install | PASS | jobs/repos/gates/dev-runner/webhooks all 404 | +| 3 | No nav entry, no admin card | PASS | `/api/v1/ui/navigation` has no `devPlatform` | +| 4 | No `dev_*` table required to boot | PASS | core boots with its own 47 migrations | +| 6 | Setup fields render from manifest | PASS | 23 fields, `state=awaiting_config` | +| 7 | Own migrations, 9 tables | PASS | ledger 9 rows; 9 `dev_*` tables | +| 8 | Public-path grant consented (C4/H1) | PASS | granted 3 — **was FAIL on `main`** | +| 8b | SQL grant (C7) | PASS\* | `INSERT INTO plugin_sql_grants` — still no HTTP endpoint | +| 9 | Nav entry appears | PASS | `navId devPlatform` — **no local patch this time** | +| 10 | Every §2 row passes | PASS | 39 probes, 0 FAIL | +| 11 | Routers stop on uninstall | PASS | `GET /jobs` → 404 | +| 12 | Nav entry disappears | PASS | no `devPlatform` | +| 13 | Background loops stop | PASS | worker + gate-deadline disposed with the handle | +| 14 | Public-path grant revoked | PASS | 200 → 404 | +| 15 | Data lifecycle | PASS / **BLOCKED** | tables + rows correctly remain (D3); grant rows orphaned — §3.15 still undecided | +| 16 | Reinstall lossless | PASS | ledger 9 (0 re-applied), routes 200 | + +### C12 static exemption + +**PASS on the C12 branch, FAIL on `main`.** #807 was not merged when this ran +(`origin/main` @ `9feb3ad3`; `publicPaths.ts:41,:44` still carry both literals). +The row was therefore re-run against `feat/470-c13-residue` @ `322afd19`, which +contains `d55a468f feat(#470): delete the two dev-platform publicPaths +exemptions (C12)` — and it passes, along with the 32 rows that cascade from it. +**Re-run `scripts/acceptance-local.mjs` against `main` once #807 lands; no +plugin change is required and none is expected.** + +### SPA and UI (§2.7) — C12 core + +| Check | Result | +|---|---| +| SPA at `/p/%40omadia%2Fdev-platform/ui/` | PASS — 200 | +| unencoded id does NOT serve | PASS — 404, encoded is canonical | +| hashed UI asset | PASS — 200 | +| `plugin-ui.css` served by core | PASS — 200 | +| bundle links no stylesheet of its own | PASS | +| 4 UI screens reachable | PASS — all backing endpoints answer | + +### ZIP + +| Check | Result | +|---|---| +| `manifest.identity.version` == `package.json` | PASS — both `0.3.0` | +| `dist/plugin.js` | PASS | +| `migrations/*.js` == 9 + `checksums.json` | PASS | +| `ui/index.html` + hashed asset | PASS | +| no `.css`, no `node_modules`, no sourcemaps | PASS — 147 maps pruned (1,741,237 bytes) | +| upload accepted | PASS — 525,131 bytes | + +--- + +## 6. Reproducing + +```bash +# 1. Postgres +docker run -d --name acc-0821-pg \ + -e POSTGRES_DB=omadia -e POSTGRES_USER=omadia -e POSTGRES_PASSWORD=omadia-ci \ + -p 55441:5432 pgvector/pgvector:pg16 + +# 2. Core — plain origin/main, no patches and no cherry-picks +git -C ../odoo-bot worktree add /tmp/odoo-bot-final origin/main +(cd /tmp/odoo-bot-final/middleware && npm install && npm run build) + +# 3. Boot. Core applies its OWN migrations now (G3 closed) — no psql loop. +# There is no DEV_PLATFORM_ENABLED any more. +cd /tmp/odoo-bot-final/middleware +DATABASE_URL='postgres://omadia:omadia-ci@127.0.0.1:55441/omadia' PORT=4111 \ + HOST=127.0.0.1 PUBLIC_BASE_URL=http://127.0.0.1:4111 AUTH_PROVIDERS=local \ + ADMIN_BOOTSTRAP_EMAIL=admin@byte5.de ADMIN_BOOTSTRAP_PASSWORD=omadia-local-dev-1 \ + PLATFORM_DATA_DIR=/tmp/acc-0821-data node dist/index.js + +# 4. Plugin, built against that core +cd ~/sources/omadia-dev-platform +npm ci && OMADIA_CORE_DIR=/tmp/odoo-bot-final npm run link:core +npm run typecheck && npm run build && npm run package -w packages/plugin + +# 5. The run — idempotent, re-runnable, exit code == FAIL count +BASE_URL=http://127.0.0.1:4111 \ + DATABASE_URL='postgres://omadia:omadia-ci@127.0.0.1:55441/omadia' \ + node scripts/acceptance-local.mjs +``` + +Full suite including the Postgres suites (without a DB they skip loudly, #572): + +```bash +docker run -d --name omadia-devplatform-pgtest -p 55438:5432 \ + -e POSTGRES_USER=test -e POSTGRES_PASSWORD=test -e POSTGRES_DB=test \ + pgvector/pgvector:pg16 +GRAPH_PG_TEST_URL=postgres://test:test@127.0.0.1:55438/test \ +OMADIA_CORE_DIR=/tmp/odoo-bot-final npm test +``` + +--- + +## 7. Notes for whoever picks this up + +- **The plugin needs nothing further.** Both fixes are in `0.3.0` and both are + pinned by tests. The remaining work is core's. +- **Re-run the C12 row against `main` when #807 lands.** Expect 71/0/2 with no + plugin change. If any row still fails, it is a new regression, not this one. +- **G7 is the finding worth acting on.** C11 shipped three weeks of design into + a code path that cannot execute for the plugin class it was written for, and + nothing failed — the log line reads `0 seeded, 9 already seeded`, which is + indistinguishable from a healthy re-run. It needs a core issue. +- **G4 keeps making every other verdict less trustworthy.** It is why `§3.9 + activation` reported PASS above a plugin that had already been torn down. Any + future run that trusts `state` over the boot log will draw the same wrong + conclusion. diff --git a/package-lock.json b/package-lock.json index 7b8f3ef..a2fe8e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5436,7 +5436,7 @@ }, "packages/plugin": { "name": "@omadia/dev-platform", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "devDependencies": { "@omadia/dev-platform-plugin-api": "*", @@ -5469,7 +5469,7 @@ }, "packages/runner-shim": { "name": "@omadia/dev-runner-shim", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "engines": { "node": ">=20" @@ -5477,7 +5477,7 @@ }, "packages/ui": { "name": "@omadia/dev-platform-ui", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { "react": "^19.2.0", @@ -5501,7 +5501,7 @@ }, "sidecars/dev-runner-daemon": { "name": "@omadia/dev-runner-daemon", - "version": "0.2.0", + "version": "0.3.0", "license": "MIT", "dependencies": { "dockerode": "^4.0.2", diff --git a/packages/plugin/manifest.yaml b/packages/plugin/manifest.yaml index 304bf2e..eed609a 100644 --- a/packages/plugin/manifest.yaml +++ b/packages/plugin/manifest.yaml @@ -21,7 +21,7 @@ identity: # settled on (specs/470-dev-platform-plugin/plan.md §4.1). There is no # "platform" kind. kind: "extension" - version: "0.2.0" + version: "0.3.0" domain: "dev-platform" description: "The Omadia Dev Platform — dev jobs, runner orchestration, the job pipeline, its own HTTP routes, chat tools and database migrations. Extracted from omadia core into its own installable plugin (epic byte5ai/omadia#470)." authors: @@ -41,33 +41,53 @@ lifecycle: entry: "dist/plugin.js" # --------------------------------------------------------------------------- -# requires — capabilities this plugin RESOLVES at runtime. +# requires / optional_requires — capabilities this plugin RESOLVES at runtime. # # Since epic #470 C2b, `ctx.services.get(name)` throws `ServiceNotDeclaredError` -# for a name that appears in neither `requires` nor `provides`. So this list is -# not documentation: every entry below is a name `activate()` actually asks for, -# and an entry removed here becomes a throw, not a silent degradation. +# for a name that appears in NONE of `requires`, `optional_requires` or +# `provides`. So neither list is documentation: every entry below is a name +# `activate()` actually asks for, and an entry removed becomes a throw, not a +# silent degradation. # -# graphPool@1 REQUIRED. The Postgres pool the job spine, repo, gate and -# artifact tables live in. Additionally gated by -# `permissions.sql` + an operator grant (C7) — a `requires:` -# line is the author's own say-so, and that is the wrong bar -# for the pool core writes user data through. -# turnContext@1 Optional-by-presence. The chat dev-job tools authorize PER -# CALL against the human driving the turn; without it there is -# no envelope, so the tools are not registered at all rather -# than registered-and-always-refusing. -# githubAppJwt@1 Optional. Core does not publish it yet; the plugin falls -# back to a local RS256 signer. See SEAMS.md S2. -# usageTelemetry@1 Optional. The operator cost dashboard's ledger. Per-job -# budgets do NOT depend on it — they meter this plugin's own -# tables. See SEAMS.md S6. -# conductorRoles@1 Optional. Live holder resolution for repos whose approver is -# a ROLE. Absent means role gates are unapprovable -# (fail-closed). See SEAMS.md S7. -# --------------------------------------------------------------------------- +# The SPLIT is what changed in C9 (core issue #795). Two gates — the capability +# resolver and the install gate — read `requires:` and nothing else, so a name +# listed there is a HARD dependency: no installed provider, no install and no +# activation. Four of the five below are survivable when absent, and each has a +# written-down degradation. Listing them as `requires:` made this plugin +# uninstallable on stock core for capabilities it is designed to live without +# (gap G2 of the 2026-08-20 acceptance run). `optional_requires:` grants the +# same `get()`/`getOptional()` access without the mandate. +# +# requires — this plugin cannot function at all without it. +# +# graphPool@1 The Postgres pool the job spine, repo, gate and artifact +# tables live in. Additionally gated by `permissions.sql` + +# an operator grant (C7) — a `requires:` line is the author's +# own say-so, and that is the wrong bar for the pool core +# writes user data through. requires: - "graphPool@1" + +# optional_requires — declared, never mandated. `activate()` reaches for each +# through `ctx.services.getOptional` (plugin-api 1.4.0) and takes the documented +# degradation when it answers `undefined`. Every one of them logs what the +# operator loses; none of them is silent. +# +# turnContext@1 The chat dev-job tools authorize PER CALL against the human +# driving the turn; without it there is no envelope, so the +# tools are not registered at all rather than +# registered-and-always-refusing. See SEAMS.md S8. +# githubAppJwt@1 Core does not publish it yet; the plugin falls back to a +# local RS256 signer. See SEAMS.md S2. +# usageTelemetry@1 The operator cost dashboard's ledger. Per-job budgets do +# NOT depend on it — they meter this plugin's own tables. +# See SEAMS.md S6. +# conductorRoles@1 Live holder resolution for repos whose approver is a ROLE. +# Absent means role gates are unapprovable (fail-CLOSED, the +# safe direction, but still a regression the operator is +# told about). See SEAMS.md S7. +# --------------------------------------------------------------------------- +optional_requires: - "turnContext@1" - "githubAppJwt@1" - "usageTelemetry@1" diff --git a/packages/plugin/package.json b/packages/plugin/package.json index 187782d..e7d43ae 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@omadia/dev-platform", - "version": "0.2.0", + "version": "0.3.0", "private": true, "type": "module", "main": "dist/plugin.js", diff --git a/packages/plugin/src/plugin.ts b/packages/plugin/src/plugin.ts index d9c48da..46c81b6 100644 --- a/packages/plugin/src/plugin.ts +++ b/packages/plugin/src/plugin.ts @@ -125,7 +125,20 @@ export interface DevPlatformPluginContext { delete?(key: string): Promise; }; readonly config: { get(key: string): T | undefined }; - readonly services: { get(name: string): T | undefined; has(name: string): boolean }; + readonly services: { + get(name: string): T | undefined; + has(name: string): boolean; + /** + * OPTIONAL, and the optionality is the version guard — same pattern as + * {@link DevPlatformPluginContext.sql}'s `seedLedger`. + * + * Added in `@omadia/plugin-api` 1.4.0 (epic #470 C9, core issue #795) as the + * accessor an `optional_requires:` entry is consumed through. On a core that + * has it, an absent provider answers `undefined`; on one that does not, the + * fallback is `get()` inside a `try` — see {@link optionalCapability}. + */ + getOptional?(name: string): T | undefined; + }; readonly sql?: { readonly ledger: string; runMigrations(opts?: { dir?: string }): Promise<{ @@ -499,15 +512,27 @@ async function activateInner( // `/p//ui/index.html?theme=&palette=&locale=`, which is where the // `ui/` directory in this package's ZIP is served from. // - // `encodeURIComponent` is load-bearing, not defensive. This plugin's id is - // SCOPED — `@omadia/dev-platform`, per `manifest.yaml` and per the charset - // `manifestLoader.ts:182` blesses — so it contains a `/`. Interpolated raw - // it would emit `/plugin-ui/@omadia/dev-platform`: two path segments, which - // neither the Next dynamic segment nor Express's `:pluginId` can match. + // NO `href` — AND THAT IS THE FIX, not a simplification. This plugin's id + // is SCOPED (`@omadia/dev-platform`), so the only URL that resolves is the + // percent-encoded one: Express's `:pluginId` and the Next dynamic segment + // both split on a raw `/`, making `@omadia/dev-platform` two segments that + // match nothing (measured in the P5 run: encoded 200, raw 404). But core's + // `HREF_SEGMENT` is the RFC 3986 unreserved set and refuses `%`, and #798 + // deliberately KEPT it strict — the shell decides "core destinations win" + // by comparing hrefs for string equality, and admitting `%xx` would weaken + // every literal href to fix the one path core can spell for itself. + // + // So a hand-built href here is not merely inelegant, it is unregisterable: + // `registerNav` throws, the throw propagates out of `activateInner`, and + // the plugin does not activate at all. That was gap G5 of the 2026-08-20 + // acceptance run, and it was closed in C9 by giving the two rules a place + // to meet: the plugin declares `pluginUi: true` and the KERNEL renders + // `pluginUiHref(id)` from the id it already holds. Byte-identical URL, and + // no plugin ever spells a percent-encoded href again. disposers.push( ctx.uiRoutes.registerNav({ navId: 'devPlatform', - href: `/plugin-ui/${encodeURIComponent(DEV_PLATFORM_PLUGIN_ID)}`, + pluginUi: true, cluster: 'adminCluster', order: 50, label: { en: 'Dev Platform', de: 'Dev-Plattform' }, @@ -582,10 +607,24 @@ async function activateInner( /** * Resolve an OPTIONAL host capability. * - * `ctx.services.get` throws `ServiceNotDeclaredError` when the manifest does not - * declare the name — a manifest bug, not a missing provider — and the two must - * not be reported the same way. Catching here keeps a declaration mistake from - * reading as "this core is too old", while still surfacing it in the log. + * Four of the five capabilities this plugin reaches for are survivable when + * absent, and the manifest says so: they sit under `optional_requires:` rather + * than `requires:` (core issue #795, epic #470 C9). Two gates read those lists + * differently — the capability resolver and the install gate consider + * `requires:` ONLY — so an `optional_requires:` entry can never keep this + * plugin from installing or activating, which is the whole point of the split. + * + * `getOptional` is the accessor that pairs with it: declared-but-unprovided + * answers `undefined` instead of throwing. It arrived with the manifest key, in + * plugin-api 1.4.0, so it is called through an optional-method guard — on an + * older core the fallback is `get()` inside a `try`, which is what this function + * did before C9 and still means the same thing. + * + * The `catch` stays on BOTH paths and is not paranoia. `ctx.services.get` throws + * `ServiceNotDeclaredError` for a name in neither list — a manifest bug, not a + * missing provider — and the two must not be reported the same way. Catching + * here keeps a declaration mistake from reading as "this core is too old", while + * still surfacing it in the log. */ function optionalCapability( ctx: DevPlatformPluginContext, @@ -593,7 +632,10 @@ function optionalCapability( log: (msg: string) => void, ): T | undefined { try { - return ctx.services.get(name); + const services = ctx.services; + return services.getOptional + ? services.getOptional(name) + : services.get(name); } catch (err) { log(`[dev-platform] capability '${name}' not resolvable: ${errText(err)}`); return undefined; diff --git a/packages/plugin/test/activate.test.ts b/packages/plugin/test/activate.test.ts index 5024f4a..cbfeb9d 100644 --- a/packages/plugin/test/activate.test.ts +++ b/packages/plugin/test/activate.test.ts @@ -23,12 +23,62 @@ import { strict as assert } from 'node:assert'; import { describe, it } from 'node:test'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + import { activate, type DevPlatformPluginContext } from '../src/plugin.js'; import { SEED_LEDGER_ENTRIES } from '../src/ledgerHandoff.js'; +/** + * The capability names the MANIFEST declares — read from the manifest rather + * than restated here. + * + * A hand-written list would be a second source of truth, and the disagreement + * between the two is exactly the failure this has to catch: `ctx.services.get` + * throws for a name in neither `requires:` nor `optional_requires:` (C2b), so + * a capability `activate()` reaches for but the manifest forgot is an + * activation throw against a real core, and a hardcoded set here would hide it. + * Binding the double to the file the kernel actually reads makes it a failure + * in this suite instead. + */ +const DECLARED_CAPABILITIES: ReadonlySet = readDeclaredCapabilities(); + +function readDeclaredCapabilities(): ReadonlySet { + const manifest = readFileSync( + resolve(dirname(fileURLToPath(import.meta.url)), '..', 'manifest.yaml'), + 'utf8', + ); + const names = new Set(); + let inList = false; + for (const line of manifest.split('\n')) { + if (/^(?:optional_)?requires:\s*$/.test(line)) { + inList = true; + continue; + } + if (!inList) continue; + if (line.trim() === '' || line.trim().startsWith('#')) continue; + const item = /^\s*-\s*["']?([^"'\n@]+)@\d+["']?\s*$/.exec(line); + if (!item?.[1]) { + inList = false; + continue; + } + names.add(item[1]); + } + if (names.size === 0) { + throw new Error('manifest.yaml declared no capabilities — the parser above drifted'); + } + return names; +} + interface Recorded { routes: Array<{ prefix: string; auth?: string; body?: string }>; navs: number; + /** C9: the nav ENTRY, not just the count. A percent-encoded `href` is what + * `HREF_SEGMENT` refuses, so the shape is the whole assertion. */ + navEntries: Array>; + /** Which accessor each optional capability was resolved through. */ + optionalLookups: Array<{ name: string; via: 'get' | 'getOptional' }>; tools: number; jobs: string[]; status: Array<{ status: string; message?: string }>; @@ -50,10 +100,14 @@ function makeCtx(over: { noSeedLedger?: boolean; /** Files the seed declined because their witness was false. */ skippedNoWitness?: readonly string[]; + /** Simulate a core older than plugin-api 1.4.0: no `services.getOptional`. */ + noGetOptional?: boolean; }): { ctx: DevPlatformPluginContext; rec: Recorded } { const rec: Recorded = { routes: [], navs: 0, + navEntries: [], + optionalLookups: [], tools: 0, toolNames: [] as string[], toolSpecs: [] as { name: string; hasHandler: boolean; hasPromptDoc: boolean }[], @@ -80,8 +134,38 @@ function makeCtx(over: { }, config: { get: (k: string) => (over.answers ?? {})[k] as T | undefined }, services: { - get: (n: string) => caps[n] as T | undefined, + // Faithful to the kernel since C2b: a name declared in NEITHER + // `requires:` nor `optional_requires:` THROWS rather than answering + // undefined. The previous stub answered undefined for everything, so a + // manifest that forgot a capability stayed green here and threw against a + // real core — the same class of fidelity gap that let the P1 double + // registration through. + get: (n: string) => { + rec.optionalLookups.push({ name: n, via: 'get' }); + if (!DECLARED_CAPABILITIES.has(n)) { + throw new Error( + `ServiceNotDeclaredError: @omadia/dev-platform did not declare ${n}`, + ); + } + return caps[n] as T | undefined; + }, has: (n: string) => n in caps, + // #795 / plugin-api 1.4.0. `over.noGetOptional` simulates a core that + // predates it, which must still resolve every optional capability through + // the `get()` fallback rather than degrading. + ...(over.noGetOptional + ? {} + : { + getOptional: (n: string) => { + rec.optionalLookups.push({ name: n, via: 'getOptional' }); + if (!DECLARED_CAPABILITIES.has(n)) { + throw new Error( + `ServiceNotDeclaredError: @omadia/dev-platform did not declare ${n}`, + ); + } + return caps[n] as T | undefined; + }, + }), }, ...(over.noSql ? {} @@ -161,8 +245,9 @@ function makeCtx(over: { }, }, uiRoutes: { - registerNav: () => { + registerNav: (entry: unknown) => { rec.navs += 1; + rec.navEntries.push(entry as Record); return dispose; }, }, @@ -415,3 +500,92 @@ describe('#470 C11 — activate() adopts core\'s ledger before applying', () => assert.equal(rec.migrationsRun, 1, 'the apply loop is the repair'); }); }); + +/** + * C9 (core issue #795 / #796): the two contract shapes that a unit test can pin + * and a fake context alone cannot. + * + * Both were found by running against a real core, not by reading code, and both + * fail the same way — a throw out of `registerNav` or `services.get` aborts + * `activateInner`, so the plugin does not activate AT ALL. A recording double + * accepts anything, which is exactly why the shape has to be asserted here + * rather than merely exercised. + */ +void describe('C9 host contracts', () => { + void it('registers nav with pluginUi, never a hand-built href', async () => { + // `HREF_SEGMENT` is the RFC 3986 unreserved set and #798 kept it strict, so + // the percent-encoded href this plugin's SCOPED id needs is precisely the + // one core refuses — while the raw spelling 404s (two path segments). The + // kernel renders `pluginUiHref(id)` itself when the entry says + // `pluginUi: true`; that is the only registrable shape. + const { ctx, rec } = makeCtx({}); + const handle = await activate(ctx); + await handle.close(); + + assert.equal(rec.navEntries.length, 1); + const entry = rec.navEntries[0] as Record; + assert.equal(entry['pluginUi'], true, 'the kernel must render the href'); + assert.equal( + entry['href'], + undefined, + "an href here is unregisterable, not merely redundant — `supply either 'href' or 'pluginUi: true', not both`", + ); + assert.equal(entry['navId'], 'devPlatform'); + }); + + void it('resolves optional capabilities through getOptional when core has it', async () => { + const { ctx, rec } = makeCtx({}); + const handle = await activate(ctx); + await handle.close(); + + const optional = ['githubAppJwt', 'usageTelemetry', 'conductorRoles', 'turnContext']; + for (const name of optional) { + const lookups = rec.optionalLookups.filter((l) => l.name === name); + assert.ok(lookups.length > 0, `${name} was never resolved`); + assert.ok( + lookups.every((l) => l.via === 'getOptional'), + `${name} must go through getOptional — it is an optional_requires entry`, + ); + } + // graphPool is the one MANDATORY capability and stays on `get`: a missing + // pool is a refusal with a named fix, not a degradation. + assert.ok(rec.optionalLookups.some((l) => l.name === 'graphPool' && l.via === 'get')); + }); + + void it('falls back to get() on a core older than plugin-api 1.4.0', async () => { + // The optionality of `getOptional` IS the version guard. A core without it + // must still activate, resolving the same names through `get()` — which is + // granted by `optional_requires:` just as it is by `requires:`. + const { ctx, rec } = makeCtx({ noGetOptional: true }); + const handle = await activate(ctx); + await handle.close(); + + assert.ok( + rec.optionalLookups.every((l) => l.via === 'get'), + 'no getOptional call may be attempted when the accessor is absent', + ); + assert.ok(rec.optionalLookups.some((l) => l.name === 'conductorRoles')); + assert.equal(rec.navs, 1, 'activation completed'); + }); + + void it('an undeclared capability is reported as a manifest bug, not an old core', async () => { + // `ctx.services.get`/`getOptional` throw `ServiceNotDeclaredError` for a + // name in neither list. `optionalCapability` catches it so a declaration + // mistake does not read as "this core is too old" — but it must still reach + // the log, or the plugin degrades in silence. + const { ctx, rec } = makeCtx({}); + const handle = await activate(ctx); + await handle.close(); + + for (const name of ['githubAppJwt', 'usageTelemetry', 'conductorRoles', 'turnContext']) { + assert.ok( + DECLARED_CAPABILITIES.has(name), + `manifest.yaml declares neither requires: nor optional_requires: '${name}'`, + ); + } + assert.ok( + !rec.logs.some((l) => l.includes('not resolvable')), + 'a declared capability must never be reported as unresolvable', + ); + }); +}); diff --git a/packages/plugin/test/manifest.test.ts b/packages/plugin/test/manifest.test.ts index 1669e44..84caeb5 100644 --- a/packages/plugin/test/manifest.test.ts +++ b/packages/plugin/test/manifest.test.ts @@ -140,15 +140,34 @@ void describe('manifest', () => { } }); - void it('requires lists every capability activate() resolves', () => { - const required = new Set(list('requires').map((r) => r.split('@')[0])); + void it('every capability activate() resolves is declared in one of the two lists', () => { // `ctx.services.get` THROWS for an undeclared name since C2b, so an omission - // here is a runtime throw, not a graceful degradation. + // from BOTH lists is a runtime throw, not a graceful degradation. + const declared = new Set( + [...list('requires'), ...list('optional_requires')].map((r) => r.split('@')[0]), + ); for (const cap of ['graphPool', 'turnContext', 'githubAppJwt', 'usageTelemetry', 'conductorRoles']) { - assert.ok(required.has(cap), `manifest requires: is missing '${cap}@1'`); + assert.ok(declared.has(cap), `manifest declares neither requires: nor optional_requires: '${cap}@1'`); } }); + void it('only graphPool is MANDATORY — the other four are optional_requires', () => { + // The split is load-bearing, and in the direction that is easy to get + // wrong. `capabilityResolver.ts` and the install gate read `requires:` and + // nothing else (#795), so a name promoted back into `requires:` makes this + // plugin uninstallable on a core with no provider for it — for a capability + // `activate()` is written to survive the absence of. That was gap G2 of the + // 2026-08-20 acceptance run, and asserting the exact SETS (not just + // membership) is what stops it coming back one line at a time. + assert.deepEqual(list('requires'), ['graphPool@1']); + assert.deepEqual(list('optional_requires'), [ + 'turnContext@1', + 'githubAppJwt@1', + 'usageTelemetry@1', + 'conductorRoles@1', + ]); + }); + void it('the host-provided runtime deps stay PEERS, never bundled dependencies', () => { // `implementation.md` §5: core cannot drop express/pg/zod even after zero // dev-platform code paths, because they are this plugin's peers resolved diff --git a/packages/runner-shim/package.json b/packages/runner-shim/package.json index b6af787..387ba35 100644 --- a/packages/runner-shim/package.json +++ b/packages/runner-shim/package.json @@ -1,6 +1,6 @@ { "name": "@omadia/dev-runner-shim", - "version": "0.2.0", + "version": "0.3.0", "private": true, "description": "The dev-platform runner shim: clones a repo read-only, drives the headless Claude CLI, translates its stream-json to runner events, and uploads a diff. Node builtins only — never imports the host. Ships as the dev-runner image entrypoint, and as the child process the unsafe local backend spawns. Moved out of omadia core in epic byte5ai/omadia#470 P4.", "type": "module", diff --git a/packages/ui/package.json b/packages/ui/package.json index c8f6d03..3777d5d 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@omadia/dev-platform-ui", - "version": "0.2.0", + "version": "0.3.0", "private": true, "type": "module", "description": "The Dev Platform operator SPA. Built with Vite into `packages/plugin/ui/`, shipped inside the plugin ZIP and served by core at `/p//ui/` (epic byte5ai/omadia#470, P2 / C8).", diff --git a/sidecars/dev-runner-daemon/package.json b/sidecars/dev-runner-daemon/package.json index 90f03f4..9e54b34 100644 --- a/sidecars/dev-runner-daemon/package.json +++ b/sidecars/dev-runner-daemon/package.json @@ -1,6 +1,6 @@ { "name": "@omadia/dev-runner-daemon", - "version": "0.2.0", + "version": "0.3.0", "private": true, "description": "The dev-platform runner daemon: a bearer-authed control-plane HTTP API over dockerode that provisions and reaps hardened job containers inside a dedicated docker:dind engine, and the default-deny egress proxy that is the only path from a job container to the internet. STANDALONE — dockerode + zod + node builtins only, never imports the host. Moved out of omadia core in epic byte5ai/omadia#470 P4.", "type": "module",