epic #470 C12: delete the two dev-platform publicPaths exemptions - #807
Merged
Conversation
…(C10) Epic #470 C10 — the flip. The Dev Platform now lives in byte5ai/omadia-dev-platform and installs via Hub/ZIP. Removed: - middleware/src/devplatform/ (62 files) — stores, worker, backends, LLM proxy, pipeline/gates, GitHub App, triggers, routers, wireDevPlatform - middleware/test/devplatform/ (58 files) - middleware/scripts/dev-transcript.ts Adversarial eval (#498): the `brief_delimiter` Tier A probe ran the real `composeBrief` out of src/devplatform/. A probe against a module core no longer ships measures a library, not a deployed defense — so the probe, its five corpus scenarios (direct-injection.jsonl, indirect-injection.jsonl) and their baseline rows leave with it. This is a real coverage reduction and is recorded as such in test/adversarial/README.md, with both ways to close it. Deterministic corpus: 12 scenarios -> 7.
…y (C10) Removed: - middleware/packages/dev-runner-shim/ (23 files) — the in-container agent shim. Never built by `npm run build`, yet index.ts resolved its dist/ at runtime; the extraction removes that inconsistency with it. - middleware/sidecars/dev-runner/ (2), dev-runner-daemon/ (30, dockerode), dev-dind/ (2) - docker-compose.dev-platform.yaml All four now live in byte5ai/omadia-dev-platform, which owns their GHCR publishing, SBOM and signing pipeline.
index.ts (-238 lines): - 15 devplatform imports + the now-dead DeviceFlowStore / ConductorRoleStore - the GitHub-webhook block mounted before express.json - the whole assembly block: assembleDevPlatform / mountDevPlatform, the GitHub-App routers, the three chat orchestrator tools, worker start + SIGTERM/SIGINT hooks, the uiRouteCatalog nav registration, the retention cron and the no-graphPool warning Side effect worth calling out: the `/api/v1/dev-runner/llm/` carve-out sat BEFORE the conductor inbound webhook router, so an express.json ran ahead of that router's route-level express.raw() and short-circuited it (body-parser marks the request `_body`). Deleting the carve-out restores the order the surrounding comments already document: pluginRawBodyMount -> conductor raw router -> global express.json. config.ts (-305 lines): all 43 DEV_*/FLY_* dev-platform schema keys, the `devPlatform` namespace, `buildDevPlatformConfig`, `csvList`, and the CORE_DEV_PREFIXED_KEYS / DevPlatformEnvKey / isDevPlatformEnvKey machinery that existed only to hold those keys out of the top level — `Config` is now `ParsedConfig`. `devPlatformBootRefusals` goes too: the plugin owns both interlocks (pluginConfig.ts, verified), so they became activation refusals rather than vanishing. KEPT: devFlag() (two PUBLIC_MCP_* call sites), DEV_ENDPOINTS_ENABLED, DEV_ENDPOINTS_LOOPBACK_ONLY, FLY_APP_NAME. .env.example: the 15-key block replaced with a pointer to the plugin repo. middleware/package.json: the §5 note — express/pg/zod stay as plugin peerDependencies resolved through the host node_modules symlink.
… (C10) - web-ui/app/admin/dev-platform/ (29 files) — the operator SPA. It ships from the plugin now as a compiled Vite bundle served through the C8 static host at /p/<pluginId>/ui/. - web-ui/app/_components/devjobs/ (6 files) + app/_lib/useDevJobEvents.ts - app/admin/page.tsx — the grid card. The generic `requiresNavFrom` mechanism stays; this was its only user. - app/_lib/i18n-structural.test.ts — the GateInbox.tsx path entry. H3 resolved by omission, as the plugin's ACCEPTANCE-RUN records. chat/page.tsx no longer special-cases `tool.name === 'dev_job_start'`; a dev-job start from the installed plugin now falls through to the generic long-running-task card (`isTaskStartToolName` / TaskChatCard), which is the accepted degradation. i18n: 299 leaf keys per locale removed (adminDevPlatform.* 288, chat.devJob.* 9, admin.index.cards.devPlatform.* 2) = 598 across en+de, plus the four orphaned `i18n-identical-allowlist.json` entries the validator flagged. `npm run i18n:check` OK — 3560 keys; `i18n:literals` translate=0.
… (C10)
publish-images.yml:
- the `dev-runner` and `dev-runner-daemon` matrix entries
- the runner-image supply chain: cosign install, syft SPDX-JSON SBOM,
keyless sign + attest (all guarded on `matrix.name == 'dev-runner'`)
- `id-token: write`, which existed solely for that keyless signing
auto-release.yml / release.yml: the matching caller-side `id-token: write`
grants, which a reusable workflow cannot self-grant and nothing else needs.
The `if: false` npm-provenance job in release.yml keeps its own grant.
byte5ai/omadia-dev-platform owns runner GHCR publishing, SBOM and signing
now. Per implementation.md §2.4 the cosign certificate identity binds to
repo + workflow + ref, so the new signer needs the transition
`--certificate-identity-regexp` landed before it publishes — that is P4's
job in the plugin repo, not core's.
The missing-Dockerfile guard stays: it also computes the version `stamp`
every remaining image consumes; only its dev-runner-daemon justification
comment is gone.
scripts/wave-{implement,verify}.workflow.mjs: the generic wave prompts baked
in "terminal transitions go through finalizeDevJob" — a dev-platform domain
rule in a domain-agnostic prompt, naming a function core no longer ships.
Genericised to "the subsystem's single finalizer"; the rule survives, the
implementor name does not. Same for the `docs/dev-platform/` example path.
`tsc` caught these, which is exactly why C10 has to be one PR. Three LONG_RUNNING_* keys sat INTERLEAVED with the dev-platform keys in config.ts, between DEV_PLATFORM_RUNNER_BASE_URL and DEV_PLATFORM_CLI_BIN, and went out with the block: LONG_RUNNING_SUBAGENT_TOOLS — W2-2 / issue #543, the generic LONG_RUNNING_TASK_STALE_MS `<tool>_start/_status/_list` seam that any LONG_RUNNING_TASK_RETAIN_MS slow tool opts into. Not dev-platform. FLY_APP_NAME went too, despite CORE_DEV_PREFIXED_KEYS documenting in prose that it "describes the host, not the feature, so it must survive the extraction". All four are restored under an explicit heading that says why they are core, so the next person reading config.ts does not have to re-derive it. Also drops the `node:os` import, now unused (it backed the workspace-dir default), and rewords the three comments this PR itself introduced that named the extracted subsystem — C10 must not add coupling references while removing them. Verified: middleware build + typecheck + adversarial/golden tsconfigs green; 6925 pass / 0 fail; typecheck:test ratchet 406 -> 371 (baseline lowered); lint 0 problems. web-ui typecheck green, 717 pass / 0 fail, i18n 3560 keys OK.
…(C10)
Ratchet: **3,300 → 214**, updated consistently in all three places the README
says must agree — `decoupling-baseline.json`, the README "Baseline **214**"
line, and the `acceptance.md` guard row.
Nine of fourteen zones read CLEAN. Every survivor is scheduled, not stranded:
migrations 69 C11 — 0022-0030 stay; core still applies them
middleware/test 62 C13 — fixture strings + the legacy-key regression test
scripts 27 C13 — the ratchet's own pattern list (needs a
self-exclusion before the total can reach 0)
middleware/src 19 C12 — publicPaths (6); rest are comments
web-ui/app 19 C13 — nav test fixtures, two comments
packages 17 C13 — plugin-api CHANGELOG recording the removal
env-example 1 the plugin repo URL, which cannot be reworded
Also: `test-typecheck-baseline.json` 406 → 371, and `package-lock.json`
regenerated (npm left `packages/dev-runner-shim` as an `extraneous: true`
stanza rather than dropping it; `npm ci` verified clean afterwards).
Docs:
- README gains a "C10 — the flip" status section: what left, what was kept and
why, H3 resolved by omission, the adversarial coverage reduction, and the
express.json ordering bug the deletion fixed. The H3 "decision before code"
is struck through — it is answered now.
- plan.md §4.2 gains a C10 note (and flags that its `devJobStepEffect.ts` line
went stale in the other direction: C5 deleted that port as dead code). P4 row
marked shipped.
- docs/CHANGELOG.md: "Dev Platform moved to byte5ai/omadia-dev-platform
(install via Hub/ZIP)" — written for operators, so it leads with how to get
it back, states that no data is touched, and that in-flight jobs survive the
upgrade because C12 has not run yet.
…sha_on_release matrix ref (C10 review) Cross-family review of PR #804. Three defects, no scope change to the deletion itself. 1. `.github/workflows/publish-images.yml` referenced `matrix.sha_on_release`, which nothing defines any more: the only two matrix entries that set it (dev-runner, dev-runner-daemon) left with the Dev Platform. actionlint errors on this branch and is clean on main. Dropping the clause also restores the behaviour the adjacent comment already claims — sha tags on the edge build only. 2. `middleware/package.json` — the `_dependencyNote` added by this PR was indented four spaces and pushed `"dependencies"` to column 0. Valid JSON, but the rest of the file is 2-space and any tool that rewrites it would reflow the diff. Text unchanged. 3. Removing the LLM-proxy carve-out moved `createConductorWebhooksInboundRouter` from *after* the global `express.json` to *before* it. That is correct — and it silently fixes a live bug: on main the router HMAC'd `Buffer.alloc(0)`, because body-parser stamps `_body` and the route-level `express.raw()` then short-circuits, so every correctly-signed inbound webhook answered 401. Measured on a standalone probe: json-first yields rawLen 0, router-first yields the full 24 bytes. Nothing locked that order. `conductorWebhookInbound.test.ts` mounts the router alone on a bare app, which passes under BOTH orders — which is exactly how the broken ordering shipped green. Added two behavioural tests (same signed request, both mount orders: 202 vs 401) plus a source-order assertion in `778RouteMounts.wiring.test.ts` that fails loudly if either anchor goes missing rather than passing vacuously. All four assertions mutation-checked: reordering the mount fails the wiring test, renaming the anchor fails it for the anchor reason, and forcing/removing the parser in the harness kills exactly the intended behavioural tests. Also corrects the C10 file count in the spec README (213 -> 217 measured deletions). The adjacent "43 config keys" claim was verified correct and left alone. Ratchet stays at 214 — the one comment that would have raised it was reworded, not excused.
Core stops exempting two URLs on a plugin's behalf. Since C4 the plugin declares them itself in `permissions.public_paths`, and the operator consents per prefix (`platform/publicPathGrants.ts`). The deletion is the load-bearing half of that handover, not tidy-up. `validateDeclaredPublicPath` REJECTS a declaration that collides with a live static exemption — "is already a static core public path … remove the core exemption first, or drop the declaration" — because two mechanisms claiming one URL is ambiguous and only one of them terminates. So while these entries existed the plugin's own grant could never validate. With them gone it does. Guarded by a new suite that pins `STATIC_PUBLIC_PATHS` as a CLOSED set: every entry must be accounted for by a named, justified core-owned exemption. Restore either deleted entry and the suite goes red on the unaccounted entry, which is what makes this commit cleanly revertible. The suite deliberately does not name the two deleted paths. Hardcoding a plugin's wire paths into core is a reference to that plugin whichever direction it asserts in — the coupling this epic removes and the thing `scripts/check-core-decoupling.mjs` counts — and it goes stale-but-green the day the plugin renames them. The closed set is also the stronger guard: it catches any unauthenticated surface being added, not just these two. The 401 follows as a corollary, since `requireAuth` runs on the `/api` mount before routing, and is asserted end-to-end against the same chain `src/index.ts` assembles. Ratchet: 214 → 209. Nothing raised.
…oopback (C12 review) The suite self-skips its five unauthenticated 401 assertions when a sandbox refuses a loopback listener. Locally that is right — the closed-set assertion still runs. In CI it is not: a runner that denied bind(127.0.0.1:0) would delete the entire 401 half of the guard while the job stayed green, which is the same permanently-green failure family the decoupling job's own timeout comment warns about. Keep the local skip, but throw with a named reason when CI is set.
Weegy
added a commit
that referenced
this pull request
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Epic #470 C12. Stacked on
feat/470-c10-delete-dev-platform(C11 has no branch yet). Single revertible commit.What
Deletes the two
STATIC_PUBLIC_PATHSentries core held on the Dev Platform's behalf: the runner phone-home prefix (which also carries the LLM proxy under/llm/) and the GitHub-Appcallback/setupredirect targets.Why this is a handover, not tidy-up
Since C4 the plugin declares its own public prefixes in
permissions.public_paths, claimed exclusively at activation and consented per prefix by the operator (platform/publicPathGrants.ts).The C4 validator rejects a declaration colliding with a live static exemption:
Two mechanisms claiming one URL is ambiguous and only one of them terminates, so the validator refuses rather than letting the static entry silently win. While these two entries existed, the plugin's own grant could never validate. Verified against this branch:
Nothing goes dark: the grant path is what serves these now, and it fails closed — empty grant table, store down, plugin not activated, registry unwired all degrade to
requireAuth401, never to "less authentication".The guard
middleware/test/auth/staticPublicPathsClosedSet.test.tspinsSTATIC_PUBLIC_PATHSas a closed set: every entry must be accounted for by a named, justified core-owned exemption, and every documented exemption must still have an entry. Mutation-checked — restoring either deleted entry fails the suite and prints the restored entry:That is what makes this cleanly revertible: the revert flips one test, not a suite.
Why the suite does not name the two deleted paths
Deliberate, and worth arguing with if you disagree. Hardcoding a plugin's wire paths into a core test is a reference to that plugin whichever direction it asserts in — the exact coupling this epic removes, the thing
scripts/check-core-decoupling.mjscounts on its way to a permanent zero (C13), and a guard that goes stale-but-green the day the plugin renames a path.Stated as a closed set it is also strictly stronger: enumerating two paths catches those two coming back, pinning the whole array catches any unauthenticated surface being added — including the next one nobody has thought of yet.
The 401 then follows as a corollary rather than an enumeration, since
requireAuthruns on the/apimount before routing. Asserted end-to-end against the same chainsrc/index.tsassembles (express.json+cookieParser+ the OB-106app.use('/api', requireAuth, …)line, nothing mounted underneath), including401rather than404for an unmounted path — because a guard that decided after routing would leave the path open the moment anything mounts under it.Verification
staticPublicPathsClosedSet+ 8 related auth/grant/MCP suiteseslint,tsc --noEmittypecheck:testratchetcheck-core-decoupling.mjsBaseline intentionally left at 214 — C13 drives it to 0 and pins it there.
Do not merge yet; stacked.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Cross-family review (Forge)
Reviewed against code, not against the description. GPT-5.4 (reasoning=high) via
codex exec— a different model family from the author, deliberately.Verdict: MERGE. Every claim in this PR held up under mutation.
What was verified against the tree
git diffofpublicPaths.ts/^\/api\/v1\/dev-runner(?:\/|$|\?)/to the array and ran the suite# skipped 0), andrequireAuthis mounted on/apiatindex.ts:1610ahead of routing/api/v1/plugin-owned/…) so no plugin path is written back into coreplatform/publicPathGrants.ts:181-191— the collision branch returnsok: falseonly while a static entry matches; the comment states the handover explicitlyDeriving the removed paths from git rather than naming them was the right call, and stating the guard as a closed set is strictly stronger than enumerating two paths — it also catches the next unauthenticated surface nobody has thought of yet.
One defect fixed —
f7acf93fThe suite computed
sandboxDeniedListenandt.skip()d the five 401 tests when a loopback listener could not bind. Locally that is correct. In CI it is not: a runner that deniedbind(127.0.0.1:0)would have deleted the entire 401 half of the guard while the job stayed green — the same permanently-green failure family the decoupling job's own timeout comment warns about.The local skip stays; when
CIis set the same bind failure now throws with a named reason.# tests 8 / pass 8 / skipped 0before and after.Gates
staticPublicPathsClosedSet8/8 · middleware suite 6,948 tests, 6,936 pass, 0 fail, 12 skipped ·typecheck:testratchet 371, no regressions · middlewareeslintclean.