fix(manifests): use canonical key/help in setup fields so secrets render - #123
Merged
Conversation
The setup fields of five built-in plugin manifests declared each field
with `id:` + `description:`, but the manifest parser
(`manifestLoader.ts`) reads `f['key']` and `f['help']`:
const key = asString(f['key']);
if (!key || !type) continue; // ← field silently dropped
Every setup field of these five plugins was therefore dropped at parse
time → their catalog `required_secrets` was empty → the Admin-UI
secret-editor rendered no input. The operator-visible consequence: a
rotated `anthropic_api_key` / `database_url` could not be re-entered
through the UI at all.
Affected manifests (all switched `id:` → `key:`, `description:` → `help:`
in `setup.fields[]` only — identity `description` and the 2-space
`links` block are untouched):
- harness-orchestrator (anthropic_api_key + 3 model knobs)
- harness-verifier (anthropic_api_key + 7 knobs)
- harness-orchestrator-extras (anthropic_api_key + 16 knobs)
- harness-knowledge-graph-neon (database_url + 22 knobs)
- harness-knowledge-graph-inmemory (graph_tenant_id)
No behaviour change at runtime: the plugins read config/secrets via
`ctx.config` / `ctx.secrets` regardless of the manifest. The fix only
restores the UI rendering + `hasRequiredSecret` classification. The
seven already-correct manifests use `key:`/`help:` — this aligns the
five outliers.
Verified: all five manifests parse, every field now exposes `key`+`help`,
secret fields (`anthropic_api_key` ×3, `database_url`) detected.
2486/2486 middleware tests pass, typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
Weegy
added a commit
that referenced
this pull request
Jul 29, 2026
Marcel confirmed a Jira/Linear tracker IS on the roadmap and matters.
That turned "move the registry dormant and forget it" into an
architecture question: after extraction the registry lives in the
dev-platform PLUGIN repo, so a Jira tracker would be a plugin
registering into another plugin's registry.
The answer is to invert the direction:
Jira plugin = PROVIDER provides: ["devTracker.jira@1"]
dev-platform = CONSUMER services.get('devTracker.' + repo.trackerKind)
per repo, per sweep — no tracker `requires`
TrackerRegistry is then DELETED, not moved. Its plugin-map half becomes
the services.get lookup; its GitHub-fallback half folds into
dev-platform's own resolver.
The naive direction fails four ways: it hands a MUTABLE registry through
an ungated accessor; registerTracker(kind, factory) has no caller
attribution (identity is the key the caller picks); services.replace()
is an exposed MITM primitive; and the ABI is DevRepo-shaped — a ~40-field
internal type that moves to the plugin repo at P4, paired with a return
type from a core route file deleted at C10. Both sides of that signature
cease to exist where a third party can reach them.
Inverted, the object crossing the seam is a read-only stateless service —
the same risk class as graphPool@1, which this org already ships. And it
is what makes C2's per-caller factory pay off: the credential owner
decides who may use its credentials. Applied to a shared registry the
factory would gate who may REGISTER, which is the wrong question.
THREE VERIFIED FINDINGS, all with consequences beyond the tracker:
1. The hot-install path bypasses capability resolution entirely.
index.ts routes `case 'extension'` straight to
toolPluginRuntime.activate(agentId) — no resolveEligiblePlugins, no
topo-sort. So `requires`-based ordering applies only on the BOOT
path; for the normal case (operator installs from the hub at runtime)
it does nothing. Any design leaning on activation ordering is already
broken there — which weakens ordering arguments elsewhere in these
docs, including the ctx.devJobs inversion discussion.
2. findDependents checks depends_on only, never capability `requires`.
An operator can uninstall a provider with live consumers, no 409.
3. source_ref is `owner/name#N`.
FINDING 3 INVERTS THE SHIP ORDER I RECOMMENDED. A Jira PROJ-123 coerced
to 123 collides with GitHub issue #123, so widening the unique index
BEFORE namespacing source_ref ships a false-POSITIVE dedupe: a Jira
ticket silently suppressing an unrelated GitHub issue. Namespace first
(jira:PROJ-123), widen second.
And the dedupe fix is less load-bearing than I claimed. listPollableRepos
selects `... AND (tracker_kind IS NOT NULL OR credential_kind =
'github_app')` — that OR is what drags webhook-covered GitHub repos into
the poll set, the sole source of the double-job risk. Delete the built-in
GitHub fallback and no repo is ever both polled and webhooked for the
same ticket. The widened index drops to defence-in-depth (still worth
having: migration 0025's source='plugin' is a third potential writer).
Verdict changes:
- Tracker polling: DEFER → DEFER-AND-HARDEN. Behind a flag, contract
frozen, expiry kept. P3's exit condition becomes "cannot be switched
on without all six hardening fixes".
- TrackerRegistry: DEFER → DELETE. Nothing to move once inverted, and
that is what lets the ratchet reach 0 without an allowlist entry.
- Comment-back: no longer "moves with #3" — REWRITTEN at P3 against the
tracker contract; only the marker/idempotency logic survives.
The contract must be frozen BEFORE the poller is hardened: Ticket needs
ticketId (opaque string), displayKey, labels[] and labelAppliedAt, plus
updatedSince as a provider parameter. Without labelAppliedAt the "fires
on any update" bug is unfixable at the consumer. Home:
src/devplatform/trackerContract.ts in Phase A, travelling at P4 — the
treatment already agreed for devJobTypes.ts.
Weegy
added a commit
that referenced
this pull request
Jul 30, 2026
… implementation (#539) * docs(470): plugins use Tailwind, so they ship no CSS (G7 reduced) Marcel's point: web-ui is Tailwind, so require Tailwind in plugins and the missing `.css` in the ZIP allowlist stops mattering. Validated, and it is better than the workaround it replaces. The catch is the whole design: Tailwind v4 emits only classes it has SEEN. It detects them by scanning source at build time, and a plugin installed at runtime from another repository is never scanned. So this only works if core pre-generates a documented, finite vocabulary. v4 supports exactly that — `@source inline(...)` (the replacement for v3's `safelist`, brace-expandable) plus `@import "tailwindcss" source(none)` to disable scanning. Measured with the repo's own tailwindcss 4.3.3 + @tailwindcss/postcss, not estimated (probe kept as specs/470-dev-platform-plugin/ plugin-tailwind-subset.probe.css): 43,199 B raw → 7,704 B gzip → 5.7 KB brotli for layout/flex/grid/spacing/typography/borders/shadows, sm:/md:/lg: and hover:/focus:/disabled: variants, with colours restricted to the Lume tokens — .bg-accent, .text-fg-muted, .border-border, .text-danger all verified present in the output. Worth more than unblocking this extraction: - Plugins inherit the design system by construction. They get OUR colour names wired to the runtime CSS variables, so they follow the active palette and light/dark automatically and cannot hardcode a hex. - It retires a known drift hazard. middleware/src/admin-ui/ harness-admin-css.ts is 345 hand-maintained lines whose own header says "mirror web-ui/app/_lib/theme.css; keep the two roughly in sync when the design system changes". Generating both from the same tokens removes the sync obligation instead of restating it. - It is enforceable: reject `[` in class attributes at ingest. HARD CONSTRAINT now in the contract: no arbitrary values (`w-[137px]`, `bg-[#abc]`). That space is unbounded, cannot be pre-generated, and such a class renders unstyled with no diagnostic — the worst failure mode. Documentation alone is not enough; it needs the ingest check. Implementation note: the `@theme inline` bridge currently lives inside globals.css:16-48 and must be extracted to its own file that both it and the plugin stylesheet import — otherwise the two drift, which is the exact failure this is meant to end. G7 is downgraded from "hard blocker" to the JS-bundle question alone: `.js` and `.map` are already allowlisted, so a compiled SPA can ship today; what is still missing is a static-asset serving path from the plugin's router. Much smaller than a styling story. * feat(470): automated decoupling ratchet + functional acceptance matrix Answers a question the existing docs could not: how do we KNOW every function got extracted and that the result is installable? They could not, and this is the gap: - core-decoupling-checklist.md enumerates FILES. You can move all ~200 and still silently lose a feature — a file inventory cannot tell you a capability survived. - plan.md stated success criteria in prose. Prose is not a probe. - The only "completeness check" was a single `rg` in the P6 exit criterion, i.e. a one-shot grep nobody runs. Two additions. 1. scripts/check-core-decoupling.mjs — a ratchet, wired into CI as the `core decoupling ratchet (#470)` job. Counts Dev Platform references across 12 zones of core and FAILS if the count rises. Baseline is 3,171 (middleware/src 1621, test 925, web-ui/app 192, sidecars 180, packages 86, migrations 70, compose 39, scripts 30, ci 16, messages 4). `--update` only ever lowers it; raising it needs a hand-edit, so a new coupling shows up in review instead of slipping in. This is what makes the checklist's staleness survivable: even if the sweep missed a reference, the count still sees it, and the count cannot reach zero while it survives. It also stops core re-acquiring a dependency mid-extraction, which is the realistic failure mode for a multi-week epic touching ~200 files. And it turns "finished" into a machine-checked fact (count 0) rather than an assertion. Verified in both directions: passes at baseline, exits 1 with the offending zone named when a reference is added. 2. specs/470-dev-platform-plugin/acceptance.md — the functional contract, which is the actual answer to "all the functions". 34 HTTP endpoints (9 job admin, 9 repo/credential, 2 gates, 5 GitHub App, 7 runner phone-home, job-policy, webhook), 3 chat tools, ctx.devJobs, 4 background loops, 4 UI screens, the chat card, the dev-transcript CLI and the conductor `dev.job` step kind — each with an owner and a probe. Plus install/uninstall/upgrade acceptance, which does not exist yet and belongs in P4. Rows whose MECHANISM must exist in core first are marked: the seven runner endpoints and the App callback need H1 (public paths), the webhook needs G3 (raw body), the chat card needs H3, the conductor step needs H2, ctx.devJobs needs the G8 contract decision. Honest about what is still not covered, in acceptance.md §4: the capability matrix is a review checklist rather than a smoke suite, install/uninstall cannot be tested before P3/P4, and once the plugin leaves this repo nothing here verifies it still satisfies §2 — that becomes the plugin repo's CI against a published core contract. Also flagged: the boot-time safety refusals (SUBSCRIPTION_MODE without ACK, UNSAFE_LOCAL without LOCAL_UID) must become activation refusals, or misconfiguration silently activates instead of failing closed. Stacked on the Tailwind commit because both edit plan.md; the ratchet is independently reviewable and independently revertible. * docs(470): index the epic — one entry point for plan, checklist, acceptance Marcel wants the whole planning to live in ONE PR, because the implementation happens there too. This is the entry point that makes that real: what each of the four documents answers, what already merged via #536, what is in flight, and the two decisions that block code. Also writes down the working agreement for a long-lived epic PR: one commit per phase so ~49k LOC stays reviewable and revertible; wire paths frozen (deployed runners phone home to literal URLs); do not delete the publicPaths exemptions before H1 is proven; and the abandonment checkpoint after P3/P3b. * docs(470): implementation plan from six parallel design passes One design pass per hard problem (H1 public paths, H2 conductor step kinds, H3 chat card + plugin UI, G4 plugin SQL + migration handoff, G8 plugin-api contract, P4 repo split + supply chain). specs/470-dev- platform-plugin/implementation.md is the synthesis: what they changed, what they found, and the PR sequence. Five decisions in plan.md were wrong or under-specified: 1. publicPaths must NOT become a dynamic set. requireAuth runs before routing and structurally cannot know who will answer, so putting the grant there rebuilds the hole it is meant to close. Use a mount slot BEFORE requireAuth that terminates — fail-closed by construction, and publicPaths.ts stays a frozen literal. 2. The chat card is neither a generic schema nor a degradation. A generic node tree makes core a rendering engine for untrusted markup; degradation makes the human gate — the platform's principal safety mechanism — annoying, and annoying safety mechanisms get bypassed. A closed 7-node contract with liveness mediated by core. A plugin-supplied SSE URL would be an SSRF aimed at the operator's own session. 3. Do NOT add .css to the ZIP allowlist. The inability to ship CSS IS the enforcement for the Tailwind vocabulary. 4. The plugin-api break has no installed base — see below. 5. The vault re-key, not the migrations, is the most irreversible step. Migrations are idempotent and additive; a deleted GitHub App private key is gone. Six live bugs found, none caused by the extraction. Two verified here: B1 ctx.services.get is completely ungated (platform/pluginContext.ts :230-233 is a bare pass-through). Any installed plugin can call ctx.services.get('graphPool') and receive the superuser pg.Pool — full read/write on users, conductor_runs, everything. No manifest declaration, nothing in the install dialog. Biggest hole found in this epic and it is live today. B4 .sql is not in the ZIP extension allowlist, so a distributed plugin cannot ship migrations at all. Blocks G4 exactly as the missing .css blocked G7. Reported and not yet independently verified: ServiceRegistry is never disposed on deactivate (same class as the router bug fixed in #536, one layer down); all five core migrators race on multi-replica boot with no advisory lock; the conductor dev-job step is dead code in production so the reconciliation sweep has never run; dev_repo_plugin_grants is never cleaned on uninstall. And a trap in my own plan: removing ctx.devJobs without replacing its gate would have converted a permission-gated, kernel-attributed accessor into an ungated, self-attributed one — because B1 is the only remaining path. Two designs flagged it independently from different directions. Two risks moved in opposite directions. H2 got much cheaper: the conductor step never ran, so there are almost certainly no live dev_job awaits and backwards compatibility is nearly free. G8's SemVer risk evaporated: @omadia/plugin-api is private:true and its publish job is gated `if: false` — never published, no installed base, so take the break now and cut 1.0.0 clean. H3 and P4 got visibly more expensive. Also newly found: next/font and data-theme do not cross an iframe boundary (silent font and dark-mode regressions), and keyless cosign binds the certificate identity to repo+workflow+ref — publishing the same image from the new repo makes every daemon with a pinned identity refuse to launch jobs. The migration handoff needs per-file schema WITNESSES, not trust in the donor ledger. Donor rows present with tables absent — a restore, a skewed rollback, an incident — makes a naive seed activate green while every request 500s. Sequence: Phase A (C1-C8) ships five reusable platform capabilities and moves zero dev-platform code, ending at the abandonment checkpoint. Phase B (P0-P5, C10-C13) is copy → prove → delete, with the proof gate at P5 and the two publicPaths exemptions deleted last, alone, in a revertible commit. Six decisions block work; D1 (publish plugin-api to public npm) blocks everything after it. * docs(470): correct D1 — publishing plugin-api was never required Marcel pushed back on D1 ("warum ist das der Blocker? Wir brauchen das doch nicht public?!") and he was right. The evidence was two directories away and I did not look. - omadia-byte5-plugins already solves this in production for six private plugins: package.json declares "@omadia/plugin-api": "file:../odoo-bot/middleware/packages/plugin-api" with the sub-packages carrying "*" as a peer resolved by the workspace root. No registry, no publish, nothing public. - The boilerplate contract mandates the OPPOSITE of what I recommended. Point 1: "KEIN Cross-Import ... Die Interface-Definition wird bewusst in ./types.ts dupliziert ... Absicht nicht Bug." omadia-plugin-starter ships vendored types/omadia-plugin-api.d.ts for exactly this. - There is no runtime dependency at all: every @omadia/plugin-api import in the dev-platform tree is `import type` and vanishes from the emitted JS. Even a value import would resolve against the host's own node_modules, which the uploaded-package store symlinks in. Root cause of the error: the design pass recommended public npm on a PRODUCT argument — GitHub Packages needs auth even to read, which would hurt a third-party plugin ecosystem. Sound for a public ecosystem, irrelevant for a private byte5 plugin. I passed it through as a technical blocker without checking how this org actually builds private plugins. D1 drops from "blocks everything after it" to a P3-typecheck-only choice between three options, none of them public: file: sibling (proven, but the plugin repo's CI then needs a core checkout — friction already recorded in project memory), vendored .d.ts (CI-isolated, drifts silently), or a git dependency on a tag (CI-isolated, explicit version). Consequence for the sequence: the first real step is no longer C1 but the B-fix PR — the three live bugs that are wrong today independently of this epic. * fix(470): corrections from the codex deep-check Full verification pass against the code (GPT-5.6, reasoning=high) over all five planning documents. It found eleven errors. The three that would have cost the most: 1. THREE CAPABILITIES IN THE ACCEPTANCE MATRIX ARE DEAD IN PRODUCTION. Verified here: - the conductor dev.job step — conductor/index.ts builds the executor with no devJob dep, so the dispatch branch never fires - ctx.devJobs — `provide('devJobs', …)` exists NOWHERE in src/, so the accessor throws on every call - tracker polling — TrackerPoller is never constructed or started The matrix was written from source, and source presence is not production reality. It would have certified preservation of capabilities the operator never had. Marked in acceptance.md; each needs a delete-or-wire decision in P2b. 2. THE RATCHET HAD A ZONE GAP AND OVERLAPPING ZONES. middleware/.env.example (19 references) was covered by no zone, so the count could have read 0 while it still documented DEV_* keys — the exact false-negative that would make "0 means done" a lie. And a root-config zone rescanned the whole web-ui tree, double-counting web-ui/app. Fixed: 14 depth-bounded, disjoint zones; baseline 3,171 → 3,181; and the check is now PER ZONE, because an aggregate-only comparison passes while one zone falls and another rises — which is precisely what a half-finished move looks like. Verified the guard now catches a regression in the previously invisible zone. acceptance.md now states plainly what the ratchet does NOT prove: it counts identifiers, not behaviour. Necessary condition, not sufficient. The earlier "machine-checked definition of completion" claim was too strong. 3. A MISSING CAPABILITY, WHICH IS THE DANGEROUS DIRECTION. The endpoint count was 34; it is 35 business endpoints / 36 handlers. The miscount hid the omission: the LLM proxy has TWO handlers and only one was listed. GET /api/v1/dev-runner/llm/ is a liveness probe the CLI depends on and it was absent from the matrix entirely. Also corrected: - The wireDevPlatform ↔ routes "cycle" is NOT an import cycle. wireDevPlatform is imported only by index.ts and no route imports back. One-way layering inversion. C3 is boundary cleanup and must not be justified as fixing hoist-dependent behaviour. - `pgPool@1` invented a capability name; the established contract is `graphPool@1`, already provided by harness-knowledge-graph-neon. Second D1-class error — a recommendation contradicting house practice. Now: gate the EXISTING graphPool@1 behind permissions.sql. - C1 still said "publish to public npm" — residue of the corrected D1. plugin-api stays private:true; only the .d.ts golden snapshot lands. - B3: at least eight migrators race, not five. - B1: the hole is real, the "superuser" characterisation is unproven. - B6: the MCP grant bug is live; the dev-repo grant half is unwired. - Arbitrary Tailwind values CAN be pre-generated when named exactly (@source inline("w-[137px]") emits it). What cannot is the unbounded universe. The vocabulary argument holds; the absolute phrasing did not. And ingest sees compiled Vite JS, not JSX class attributes, so "reject [ in class attributes" is under-specified. - src/devplatform is 53 files / 14,457 LOC, not 54 / 14,498. - plan.md §4.1 and §4.2 contradicted each other on DevJob type ownership. §4.2 wins: the types move to the plugin repo. Flagged, not yet resolved: a Vite multi-file SPA is not supported by today's plugin contract (boilerplate mandates single-file HTML and a tsc-only build), so P2 is viable only after C8 ships static serving. And unknown manifest keys are silently IGNORED, not rejected — a plugin declaring permissions.public_paths against an unpatched core would activate with no grant and no error. * feat(470): decide the dormant capabilities — and there are five, not three Three design passes (one per capability) plus a codex verification round. The verdicts differ, which is the finding — "activate all three" would have been wrong. 1. Conductor dev.job step → ACTIVATE as its own PR (C5b), or delete 2. ctx.devJobs → DELETE 3. Tracker polling → DEFER, move dormant 4. TrackerRegistry → DEFER, moves with #3 (newly found) 5. Comment-back → DEFER, moves with #3 (newly found) #4 and #5 surfaced while designing #3. acceptance.md listed comment-back as live with the probe "result posted to the issue" — it is not wired, so a polled job's result never reaches the issue and the loop is half-open even if the poller ran. THE DECISIVE FINDING is on ctx.devJobs, and it inverts the intuition. Every access gate lives in the ACCESSOR, and every identity is a parameter the CALLER passes — listGrantedRepoIds(pluginId), cancelJob(jobId, requestedByPluginId), createdBy:{kind:'plugin',id}. Verified: the host service itself verifies nothing, and DevRepoPluginGrantStore is never constructed, so the grant table has no writer at all. Combined with the ungated ctx.services.get (B1), the moment ANYONE registers 'devJobs' — core today or the extracted plugin tomorrow — any installed plugin can fetch it with no manifest declaration and no operator consent, pass an arbitrary pluginId, and bypass the permission gate, the repo-grant scope, the creator check and the audit attribution, while framing another plugin. So the dead state is SAFER than the wired state. "It throws on every call" is currently load-bearing. That also sharpens implementation.md §2.2 by a notch: I had written that REMOVING ctx.devJobs without replacing its gate opens the hole. True — but ADDING the provider opens it too. `provide` is the dangerous operation. C2 bundling the gate fix with the removal is a correctness requirement, not a convenience. And a test-shape lesson worth more than this epic: pluginDevJobsAccessor.test.ts has a case titled "throws a clear error when the host service is unregistered" — it asserts the PRODUCTION BEHAVIOUR as the error path and stays green, against a two-line fake registry. No test in the repo boots a real ServiceRegistry and asks whether anything provided the service. A boot-level accessor/provider invariant should ship independent of #470. ONE THING SHIPS NOW: cross-source trigger dedupe. hasActiveTriggerJob filters on `source` and dev_jobs_webhook_one_active is scoped WHERE source='webhook', so a repo with both triggers would get two runners, two LLM budgets and two PRs for one issue. Latent only because no tracker job has ever been created. It is the only artifact here that is not thrown away by the extraction. CORRECTIONS FROM THE VERIFICATION ROUND — the first draft had errors, and two of them understated risk: - cold start costs $500, not $150 (default budget is $5, page limit 100). Understated by 3x. - "a dry run would spend real money" was overstated: the supported route is previewRun, which explicitly stubs action steps. - "nobody could ever author the step" is wrong — validation is bypassed on the raw POST / path. - "a poller with nothing to poll" is overstated — TrackerRegistry has a built-in GitHub fallback needing no registration. - hooking DevJobStore.finishTerminal contradicts our own contract: finalizeDevJob is the documented choke point, and the decoupling checklist names it. Fix the split finalizer wiring instead. - phase-engine terminals do NOT bypass boundFinalize; only the worker-driven ones do. - the widened index is not "mandatory today" — the existing webhook-only index already makes the live path replica-safe — and it is NOT a safe drop-in: it needs a duplicate preflight, and it should land after the migrator advisory-lock fix, not before. PROCESS FAILURE, named because it matters: the first draft proposed resolutions and did not propagate them, leaving acceptance.md and implementation.md still saying "three" and still requiring preservation of things this document deletes. A decision doc that contradicts its siblings leaves the spec set worse than before. Propagated here, along with the stale README ratchet numbers (12 zones/3,171 → 14 zones/3,181). * docs(470): record Marcel's answers — devJobs delete confirmed, tracker is a roadmap foundation Two open questions answered, and the second changes more than the first. NO customer-side or unreleased plugin declares permissions.devJobs. The DELETE verdict for ctx.devJobs is confirmed — the single fact that could have inverted it does not exist. G8 collapses almost entirely with it, and the plugin-api major bump becomes hygiene rather than a break with downstream cost. YES, a Jira/Linear tracker is on the roadmap and is considered important. The tracker verdict keeps its direction — defer, move dormant — but loses its meaning: 'and forget about it' was wrong. TrackerRegistry is not dead weight being tolerated, it is the extension point for a roadmap feature. Three consequences: - The blockers stop being hypothetical. Cold start ($500 ceiling), requireGate:false with no sender allowlist, firing on any ticket update rather than on label application, and the cross-source dedupe gap become must-fix before a Jira tracker runs. - A new architectural question, in no document until now: after extraction the registry lives in the dev-platform PLUGIN repo, so a Jira tracker would be a plugin registering into another plugin's registry. That seam constrains the extraction — it may argue for keeping a generic job-trigger-source extension point in CORE rather than moving the registry out. - It inherits B1. A tracker registry is a WRITE surface: registering a tracker influences which issues become code-execution jobs. With ctx.services.get ungated, any plugin could register one. The per-caller-factory fix in C2 becomes a prerequisite, not optional hardening. Design pass on the cross-plugin seam is in flight. * feat(470): invert the tracker seam — provider, not registry Marcel confirmed a Jira/Linear tracker IS on the roadmap and matters. That turned "move the registry dormant and forget it" into an architecture question: after extraction the registry lives in the dev-platform PLUGIN repo, so a Jira tracker would be a plugin registering into another plugin's registry. The answer is to invert the direction: Jira plugin = PROVIDER provides: ["devTracker.jira@1"] dev-platform = CONSUMER services.get('devTracker.' + repo.trackerKind) per repo, per sweep — no tracker `requires` TrackerRegistry is then DELETED, not moved. Its plugin-map half becomes the services.get lookup; its GitHub-fallback half folds into dev-platform's own resolver. The naive direction fails four ways: it hands a MUTABLE registry through an ungated accessor; registerTracker(kind, factory) has no caller attribution (identity is the key the caller picks); services.replace() is an exposed MITM primitive; and the ABI is DevRepo-shaped — a ~40-field internal type that moves to the plugin repo at P4, paired with a return type from a core route file deleted at C10. Both sides of that signature cease to exist where a third party can reach them. Inverted, the object crossing the seam is a read-only stateless service — the same risk class as graphPool@1, which this org already ships. And it is what makes C2's per-caller factory pay off: the credential owner decides who may use its credentials. Applied to a shared registry the factory would gate who may REGISTER, which is the wrong question. THREE VERIFIED FINDINGS, all with consequences beyond the tracker: 1. The hot-install path bypasses capability resolution entirely. index.ts routes `case 'extension'` straight to toolPluginRuntime.activate(agentId) — no resolveEligiblePlugins, no topo-sort. So `requires`-based ordering applies only on the BOOT path; for the normal case (operator installs from the hub at runtime) it does nothing. Any design leaning on activation ordering is already broken there — which weakens ordering arguments elsewhere in these docs, including the ctx.devJobs inversion discussion. 2. findDependents checks depends_on only, never capability `requires`. An operator can uninstall a provider with live consumers, no 409. 3. source_ref is `owner/name#N`. FINDING 3 INVERTS THE SHIP ORDER I RECOMMENDED. A Jira PROJ-123 coerced to 123 collides with GitHub issue #123, so widening the unique index BEFORE namespacing source_ref ships a false-POSITIVE dedupe: a Jira ticket silently suppressing an unrelated GitHub issue. Namespace first (jira:PROJ-123), widen second. And the dedupe fix is less load-bearing than I claimed. listPollableRepos selects `... AND (tracker_kind IS NOT NULL OR credential_kind = 'github_app')` — that OR is what drags webhook-covered GitHub repos into the poll set, the sole source of the double-job risk. Delete the built-in GitHub fallback and no repo is ever both polled and webhooked for the same ticket. The widened index drops to defence-in-depth (still worth having: migration 0025's source='plugin' is a third potential writer). Verdict changes: - Tracker polling: DEFER → DEFER-AND-HARDEN. Behind a flag, contract frozen, expiry kept. P3's exit condition becomes "cannot be switched on without all six hardening fixes". - TrackerRegistry: DEFER → DELETE. Nothing to move once inverted, and that is what lets the ratchet reach 0 without an allowlist entry. - Comment-back: no longer "moves with #3" — REWRITTEN at P3 against the tracker contract; only the marker/idempotency logic survives. The contract must be frozen BEFORE the poller is hardened: Ticket needs ticketId (opaque string), displayKey, labels[] and labelAppliedAt, plus updatedSince as a provider parameter. Without labelAppliedAt the "fires on any update" bug is unfixable at the consumer. Home: src/devplatform/trackerContract.ts in Phase A, travelling at P4 — the treatment already agreed for devJobTypes.ts. * docs(470): scope correction — conductor dev-job is delete, not genericise Marcel: 'Der Conductor ist eine neue Funktion. Was hat das mit der Dev Platform zu tun?' Correct, and it exposed a scope error. The Conductor is a real, live feature (31 files, 6202 LOC backend, 23 UI files, 7 migrations, its own spec). It is in this epic only because its code holds 73 dev-platform references that must leave for the ratchet to reach 0 — not because anything about Conductor itself is being changed. I turned that into 'build a generic step-kind registry (H2/C5) and activate the step (C5b)' — a new platform capability plus a new feature, neither of which anyone asked for, propagated through the plan as a hard blocker. Removing a dev-platform reference from core has exactly two paths: genericise, or delete. Genericising is only justified when something real needs the generic version. Nothing did. So G9 drops from hard blocker to a deletion, C5 shrinks to 'delete dead code', and C5b disappears along with the await_kind migration, the registry deactivation semantics and the cross-kind guard — all unbuilt. Deleting is not lost work: the existing code is dev-job-SHAPED, so a generic registry would replace it anyway. Only the design has value, and that survives in this document. * chore(470): resync with main (PR #529) and re-baseline the ratchet main brought PR #529 — a substantial dev-platform change: 59 files, +3,751 LOC. New web-ui surfaces (PhaseArtifactPanel, PrettyArtifact, ToolCallCard, lineDiff/prettyArtifact/toolCallLog libs + tests), LLM proxy test coverage, and 27 new i18n lines per locale. The ratchet caught it exactly as designed: 3,181 → 3,293 across six zones (src +14, test +32, packages +10, sidecars +15, web-ui/app +35, compose +6). This is the documented hand-edit case — main legitimately ADDED dev-platform code, so the count rises for a legitimate reason rather than core re-acquiring a dependency. Baseline raised deliberately and recorded here. Re-measured, since the checklist is a snapshot: src/devplatform 53 files, 14,457 → 14,520 LOC web-ui admin surface 20 files / 3,163 → 29 files / 4,344 LOC adminDevPlatform i18n 269 → 288 keys (3,205 total) Also propagated the verdicts into acceptance.md's warning box, which still carried the pre-correction versions: conductor step now DELETE (not "activate as C5b"), TrackerRegistry DELETE (the seam inverts), comment-back REWRITE at P3, polling DEFER-AND-HARDEN. Verification after merge: middleware build + typecheck clean, 5,044 pass; web-ui typecheck clean, 388 pass, i18n parity OK at 3,205 keys. One middleware failure did not reproduce on re-run — consistent with the pre-existing load-sensitive flakiness already documented (a file of 48 trivial assertions reproduces it; baseline without added files is clean). * fix(platform): dispose plugin-provided services on deactivate ServiceRegistry had no owner tracking and no disposeBySource, and toolPluginRuntime.deactivate() disposed routes and uiRoutes but not services. A provider whose close() forgets its handle left the service registered against a torn-down module, and reinstall then threw "duplicate provider". Same bug class PR #536 fixed for Express routers, one layer down. - serviceRegistry.ts: owner tracking on provide()/replace(), and disposeBySource() unwinding LIFO — an older `replace` restore would otherwise reinstate a provider a newer one has since shadowed. `owner` is optional, so core's ~25 boot-time provide() calls stay untracked and can never be bulk-disposed. - pluginContext.ts: ctx.services.provide/replace pass agentId, so attribution comes from the kernel-known id and never from a caller-supplied argument. No plugin-api contract change. - toolPluginRuntime.ts: disposeBySource before the awaited close(), same 5s-budget reasoning as the route disposal, plus the activate-failure rollback. - dynamicAgentRuntime.ts: same gap confirmed and mirrored. 13 tests. Verified fail-without-fix in three staged reverts: reverting both runtimes gives 4 real assertion failures (not TypeErrors); reverting the context fix alone fails the attribution test; full pre-fix state fails 11. Also fixes toolPluginRuntimeRouteDisposal.test.ts's fixture, which omitted the now-required serviceRegistry dep — fixed the fixture rather than making the production call defensive, since the real wiring always supplies it. 5,058 pass, typecheck and lint clean. TWO OTHER FIXES FROM THIS BATCH WERE DELIBERATELY NOT SHIPPED — see the follow-up notes. Adding '.sql' to the zip allowlist would weaponise a pre-existing path traversal into arbitrary SQL execution, and wrapping the migrators in an unbounded advisory lock would convert a rare race into a deterministic boot failure. Both verified against the code. Known gaps in this fix, both worth follow-ups: - withTimeout is a bare Promise.race and does not cancel, so a timed-out activate can still register services after the rollback ran. - DynamicAgentRuntime.activate() has no rollback block at all, and its route disposal still sits after the awaited close() — the pre-#536 ordering. * chore(470): resync with main (#537) and re-baseline; refresh status main brought the #440/#537 embedding work, which added 10 more dev-platform references (mostly tests): 3,293 → 3,303 across src, test and packages. Third hand-edit of the baseline, same legitimate reason as the previous two — main ADDED dev-platform code; core did not re-acquire a dependency. That distinction is now stated in the README so the next raise is not read as a regression. Also fixes a sentence an earlier perl replacement broke in the README ("... It / But it counts ...") and refreshes the status section, which still claimed only Tailwind + ratchet were in flight. It now records what actually landed, what went out separately, and — more useful — the three fixes that were implemented and deliberately NOT shipped because review found real harm in them.
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
1 participant
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.
Problem
Die Setup-Felder von 5 built-in Plugin-Manifesten sind mit
id:+description:deklariert — der Parser (manifestLoader.ts) liest aberf['key']undf['help']:Jedes Setup-Feld dieser 5 Plugins wurde dadurch beim Parsen verworfen → catalog
required_secretsleer → die Admin-UI rendert kein Eingabefeld. Operator-Konsequenz: ein rotierteranthropic_api_key/database_urllässt sich gar nicht über die UI neu eintragen.Fix
In allen 5 Manifesten in
setup.fields[]:id:→key:,description:→help:. Identity-descriptionund der 2-Space-links-Block bleiben unangetastet.harness-orchestratoranthropic_api_key+ 3harness-verifieranthropic_api_key+ 7harness-orchestrator-extrasanthropic_api_key+ 16harness-knowledge-graph-neondatabase_url+ 22harness-knowledge-graph-inmemorygraph_tenant_idKein Runtime-Verhalten ändert sich — die Plugins lesen Config/Secrets via
ctx.config/ctx.secretsunabhängig vom Manifest. Der Fix stellt nur UI-Rendering +hasRequiredSecret-Klassifikation wieder her. Die 7 bereits korrekten Manifeste nutzenkey:/help:— das richtet die 5 Ausreißer aus.Test plan
key+help, Secret-Felder (anthropic_api_key×3,database_url) erkanntnpm run typecheck— grünnpm test— 2486/2486 grün, 0 Fehleranthropic_api_key-Feld → rotierten Key eintragen