mirror of
https://github.com/mvschwarz/openrig-studio.git
synced 2026-11-07 05:56:21 -10:00
Presence is not meaning: three provisioning defects from a real VPS
cloud-impl found all three provisioning a real VPS, named the shared shape,
and reported two of them against their own code. They are the same error at three
layers: something EXISTS, so it is treated as valid, and the check cannot tell
"declared and incomplete" from "declared absent" or "present and unusable".
D11 — install-app.mjs refused a deliberate "no provider". `apps/workspace` declares
`provider: { required: false }` with a note saying it needs none, and the checker
read the KEY'S PRESENCE as a declaration: three failures against a manifest that
was documenting its own correctness. WORKSPACE was uninstallable on any box. A
provider is declared by a PACKAGE, not by a key existing. One shared predicate
rather than two edits, because two sites deciding one question is the drift shape
this repo has already paid for three times — reverting either site alone still
brings failures back, which is how I know both mattered.
D9 — the provider branch tested whether a lockfile EXISTS, not whether it WORKS,
and committed to `npm ci` on that basis. A stub lockfile locking nothing sailed
past the file test and died inside npm. Present-but-unusable is worse than absent:
absent fails where the branch already handles it. Now it tries the exact path,
SAYS the lockfile was unusable, and falls through. Verified against real npm and a
real stub lockfile, with a control proving npm ci genuinely refuses it.
D10 — dependencies were installed for every provider on disk regardless of which
apps were enabled, so a provider nobody asked for could stop the whole provision.
D10 IS ALSO A CORRECTION TO MY OWN CLAIM, and that is the part worth recording.
I shipped the curated four-app default saying "two providers, no ffmpeg" — in the
README, in the commit message, and in my report to orch. It was FALSE: the app list
narrowed, the provider install did not, so all seven providers' dependencies landed
anyway. I asserted a benefit I never measured, in the same change where I was
careful to log what the cap dropped. The claim is true now because this makes it
true, not because it was true then.
Resolution walks JSON in node and is read line by line, never word-split. It
covers both ways an app needs a backend: naming one, and REQUIRING a verb somebody
else serves — `calls` is provider-agnostic by design, so an unserved required call
means reconciliation refuses the install. Optional calls pull nothing, mirroring
the ladder in app-manifest.md where only `required: true` may start a provider no
app names as its own.
298 tests. The shell paths were driven directly against fixtures and real npm;
this targets Linux and the host is Darwin, so the provision was not run end to end.
This commit is contained in:
1 parent
9c824a7b06
commit
6e0da5037c
4 files changed
+261
-5
No files matched your search
@@ -0,0 +1,73 @@
|
||||
#!/usr/bin/env node
|
||||
// Which provider directories does THIS set of apps actually need?
|
||||
//
|
||||
// node needed-providers.mjs <appsRoot> <app> [app...] -> one directory name per line
|
||||
//
|
||||
// WHY THIS EXISTS: provisioning installed dependencies for every provider on
|
||||
// disk regardless of which apps were enabled, so a provider nobody asked for
|
||||
// could stop the whole install. A box asking for four apps should not be
|
||||
// stopped by the eleventh provider. Found by cloud-impl on a real VPS, where
|
||||
// exactly that happened.
|
||||
//
|
||||
// WHY IT IS NODE AND NOT SHELL: this walks JSON and emits a set. Structured data
|
||||
// does not round-trip through the shell's word list, and the caller reads the
|
||||
// output line by line rather than splitting it.
|
||||
//
|
||||
// TWO WAYS AN APP NEEDS A PROVIDER, and only the first is obvious:
|
||||
// 1. it NAMES one — `provider.package`
|
||||
// 2. it REQUIRES A VERB somebody else serves — `calls` with `required: true`.
|
||||
// `calls` is provider-agnostic by design (app-manifest.md), so the box works
|
||||
// out who answers. An app whose required call is unserved is refused at
|
||||
// reconciliation, and a provider whose deps were never installed cannot
|
||||
// start to serve it.
|
||||
// Optional calls deliberately pull nothing in: `required: false` grants no
|
||||
// authority to start a provider, so it grants none to install one either.
|
||||
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
const [appsRoot, ...apps] = process.argv.slice(2);
|
||||
if (!appsRoot || !apps.length) {
|
||||
console.error("usage: needed-providers.mjs <appsRoot> <app> [app...]");
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
const readJson = (p) => {
|
||||
try { return JSON.parse(fs.readFileSync(p, "utf8")); } catch { return null; }
|
||||
};
|
||||
const dirOf = (pkg) => (typeof pkg === "string" && pkg ? path.basename(pkg) : null);
|
||||
|
||||
const providersDir = path.join(appsRoot, "providers");
|
||||
const onDisk = fs.existsSync(providersDir)
|
||||
? fs.readdirSync(providersDir).filter((d) => fs.statSync(path.join(providersDir, d)).isDirectory())
|
||||
: [];
|
||||
|
||||
// verb -> provider directory, read from each provider's own declaration. The
|
||||
// provider is authoritative about which verbs it answers; nothing else can say.
|
||||
const answers = new Map();
|
||||
for (const d of onDisk) {
|
||||
const decl = readJson(path.join(providersDir, d, "provider.json"));
|
||||
for (const v of decl?.verbs ?? []) if (!answers.has(v)) answers.set(v, d);
|
||||
}
|
||||
|
||||
const needed = new Set();
|
||||
for (const app of apps) {
|
||||
const m = readJson(path.join(appsRoot, "apps", app, "app.json"));
|
||||
if (!m) continue;
|
||||
|
||||
const direct = dirOf(m.provider?.package);
|
||||
if (direct) needed.add(direct);
|
||||
|
||||
for (const [verb, spec] of Object.entries(m.calls ?? {})) {
|
||||
if (!spec?.required) continue;
|
||||
// A verb ending in `/` is a prefix match (app-manifest.md), so an exact
|
||||
// lookup alone would miss `/api/export-status/<id>`-shaped declarations.
|
||||
const who = answers.get(verb)
|
||||
?? [...answers.entries()].find(([v]) => v.endsWith("/") && verb.startsWith(v))?.[1];
|
||||
if (who) needed.add(who);
|
||||
}
|
||||
}
|
||||
|
||||
// Only ones that exist. A named-but-absent provider is reconciliation's problem
|
||||
// to report, not this script's to invent.
|
||||
for (const d of [...needed].sort()) if (onDisk.includes(d)) console.log(d);
|
||||
@@ -271,11 +271,48 @@ install_provider_deps() {
|
||||
return 0
|
||||
}
|
||||
|
||||
for p in "$STUDIO_DIR"/apps/providers/*/; do
|
||||
n=$(basename "$p")
|
||||
# ONLY THE PROVIDERS THE ENABLED APPS ACTUALLY NEED.
|
||||
#
|
||||
# This used to loop over every provider directory on disk, so a provider nobody
|
||||
# asked for could stop the whole provision — and did, on a real VPS. A box asking
|
||||
# for four apps should not be blocked by the eleventh provider.
|
||||
#
|
||||
# It also made the curated app default a half-truth: selecting four apps still
|
||||
# installed all seven providers' dependencies, so "two providers, no ffmpeg" was
|
||||
# a claim about the app list rather than about what actually gets installed.
|
||||
# Resolution is in node because it walks JSON and returns a set; the result is
|
||||
# read LINE BY LINE rather than word-split.
|
||||
NEEDED_PROVIDERS=()
|
||||
if [ "$DRY_RUN" = 0 ]; then
|
||||
while IFS= read -r line; do [ -n "$line" ] && NEEDED_PROVIDERS+=("$line"); done < <(
|
||||
"$NODE_BIN" "$(dirname "$0")/needed-providers.mjs" "$STUDIO_DIR/apps" "${APP_LIST[@]}" 2>/dev/null || true)
|
||||
fi
|
||||
ALL_PROVIDERS=(); for d in "$STUDIO_DIR"/apps/providers/*/; do [ -d "$d" ] && ALL_PROVIDERS+=("$(basename "$d")"); done
|
||||
printf ' %d of %d provider(s) needed by the enabled apps: %s\n' \
|
||||
"${#NEEDED_PROVIDERS[@]}" "${#ALL_PROVIDERS[@]}" "${NEEDED_PROVIDERS[*]:-none}"
|
||||
|
||||
for n in "${NEEDED_PROVIDERS[@]}"; do
|
||||
p="$STUDIO_DIR/apps/providers/$n/"
|
||||
[ -d "$p" ] || { warn "provider '$n' is needed but not present — reconciliation will name it"; continue; }
|
||||
# A LOCKFILE THAT EXISTS IS NOT A LOCKFILE THAT WORKS.
|
||||
#
|
||||
# This branched on the file's EXISTENCE and committed to `npm ci`. A provider
|
||||
# shipped a STUB lockfile — one entry, locking nothing, against a package.json
|
||||
# with real dependencies — and `npm ci` correctly refused it, which killed the
|
||||
# provision. Present-but-unusable is WORSE than absent: absent fails at a point
|
||||
# the branch already handles, present sails past the file test and dies inside
|
||||
# npm. Presence standing in for meaning, the same shape as the other two defects
|
||||
# in the report that found this.
|
||||
#
|
||||
# So: try the fast, exact path, and if it refuses, SAY SO and take the slow one.
|
||||
# Do not pre-judge the file from its existence.
|
||||
if [ -f "$p/package-lock.json" ]; then
|
||||
printf ' %-16s npm ci (lockfile present)\n' "$n"
|
||||
install_provider_deps "$n" "npm ci --prefix '$p' --no-audit --no-fund"
|
||||
if ! install_provider_deps "$n" "npm ci --prefix '$p' --no-audit --no-fund"; then
|
||||
warn "provider '$n': npm ci refused the lockfile (see above) — falling back to npm install"
|
||||
printf ' %-16s npm install (lockfile unusable)\n' "$n"
|
||||
install_provider_deps "$n" "npm install --prefix '$p' --no-audit --no-fund"
|
||||
fi
|
||||
else
|
||||
printf ' %-16s NO LOCKFILE -> npm install (declared deps: %s)\n' "$n" \
|
||||
"$($NODE_BIN -p "Object.keys(require('$p/package.json').dependencies||{}).length" 2>/dev/null || echo '?')"
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
// Which providers an app set actually needs.
|
||||
//
|
||||
// node --test 'test/*.test.mjs'
|
||||
//
|
||||
// Provisioning installed dependencies for EVERY provider on disk regardless of
|
||||
// which apps were enabled, so a provider nobody asked for could stop the whole
|
||||
// install — which is exactly what happened on a real VPS. It also made the curated
|
||||
// app default a half-truth: choosing four apps still installed all seven
|
||||
// providers, so "two providers, no ffmpeg" described the app list rather than what
|
||||
// landed on the box.
|
||||
//
|
||||
// Found and reported by cloud-impl. The under-installing direction matters as much
|
||||
// as the over-installing one, so both are pinned here: a set that is too NARROW
|
||||
// fails at reconciliation with a named error, which is loud; a set that is too WIDE
|
||||
// fails by dragging in a provider nobody wanted, which is what blocked the box.
|
||||
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
|
||||
const SCRIPT = path.join(REPO, "provision", "needed-providers.mjs");
|
||||
|
||||
function fixture(apps, providers) {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "needprov-"));
|
||||
for (const [id, manifest] of Object.entries(apps)) {
|
||||
const d = path.join(root, "apps", id);
|
||||
fs.mkdirSync(d, { recursive: true });
|
||||
fs.writeFileSync(path.join(d, "app.json"), JSON.stringify(manifest, null, 2));
|
||||
}
|
||||
for (const [id, decl] of Object.entries(providers)) {
|
||||
const d = path.join(root, "providers", id);
|
||||
fs.mkdirSync(d, { recursive: true });
|
||||
if (decl) fs.writeFileSync(path.join(d, "provider.json"), JSON.stringify(decl, null, 2));
|
||||
}
|
||||
return root;
|
||||
}
|
||||
const run = (root, ...apps) =>
|
||||
execFileSync(process.execPath, [SCRIPT, root, ...apps], { encoding: "utf8" })
|
||||
.split("\n").map((s) => s.trim()).filter(Boolean);
|
||||
|
||||
const PROVIDERS = {
|
||||
"studio-host": { package: "@openrig/studio-host", verbs: ["/api/files/tree"] },
|
||||
"studio-factory": { package: "@openrig/studio-factory", verbs: ["/api/factory/state"] },
|
||||
"studio-video": { package: "@openrig/studio-video", verbs: ["/api/clips"] },
|
||||
"studio-effects": { package: "@openrig/studio-effects", verbs: ["/api/effects"] },
|
||||
};
|
||||
|
||||
test("an app that NAMES a provider pulls exactly that one", () => {
|
||||
const root = fixture({ files: { id: "files", provider: { package: "@openrig/studio-host" } } }, PROVIDERS);
|
||||
assert.deepEqual(run(root, "files"), ["studio-host"]);
|
||||
});
|
||||
|
||||
test("an app that declares NO provider pulls nothing", () => {
|
||||
// The workspace shape: `required: false` with no package, deliberately.
|
||||
const root = fixture({
|
||||
workspace: { id: "workspace", provider: { required: false, _note: "no backend on purpose" } },
|
||||
designer: { id: "designer" },
|
||||
}, PROVIDERS);
|
||||
assert.deepEqual(run(root, "workspace", "designer"), [],
|
||||
"an app with no backend dragged a provider install onto the box");
|
||||
});
|
||||
|
||||
test("THE WHOLE POINT: a narrow app set does not pull every provider on disk", () => {
|
||||
const root = fixture({
|
||||
workspace: { id: "workspace", provider: { required: false } },
|
||||
files: { id: "files", provider: { package: "@openrig/studio-host" } },
|
||||
factory: { id: "factory", provider: { package: "@openrig/studio-factory" } },
|
||||
cutdown: { id: "cutdown", provider: { package: "@openrig/studio-video" } },
|
||||
gallery: { id: "gallery", provider: { package: "@openrig/studio-effects" } },
|
||||
}, PROVIDERS);
|
||||
const got = run(root, "workspace", "files", "factory");
|
||||
assert.deepEqual(got, ["studio-factory", "studio-host"]);
|
||||
assert.ok(!got.includes("studio-video") && !got.includes("studio-effects"),
|
||||
"a provider no enabled app references would still have its dependencies installed, and could still stop the box");
|
||||
});
|
||||
|
||||
test("a REQUIRED cross-provider call pulls the provider that answers it", () => {
|
||||
// `calls` is provider-agnostic by design, so an app can require a verb another
|
||||
// backend serves. Under-installing that is not merely tidy — the provider cannot
|
||||
// start, so the verb is unserved and reconciliation refuses the install.
|
||||
const root = fixture({
|
||||
agents: { id: "agents", calls: { "/api/factory/state": { required: true } } },
|
||||
}, PROVIDERS);
|
||||
assert.deepEqual(run(root, "agents"), ["studio-factory"]);
|
||||
});
|
||||
|
||||
test("an OPTIONAL call pulls nothing — required:false grants no authority", () => {
|
||||
// Mirrors the unmatched-call ladder in app-manifest.md: only `required: true`
|
||||
// may start a provider no app names as its own, so only it may install one.
|
||||
const root = fixture({
|
||||
agents: { id: "agents", calls: { "/api/factory/state": { required: false } } },
|
||||
}, PROVIDERS);
|
||||
assert.deepEqual(run(root, "agents"), []);
|
||||
});
|
||||
|
||||
test("a PREFIX verb declaration still matches", () => {
|
||||
// app-manifest.md: a verb ending in `/` is a prefix, so an exact-key lookup
|
||||
// alone silently misses `/api/export-status/<jobId>`.
|
||||
const root = fixture(
|
||||
{ j: { id: "j", calls: { "/api/export-status/42": { required: true } } } },
|
||||
{ ...PROVIDERS, "studio-jobs": { package: "@openrig/studio-jobs", verbs: ["/api/export-status/"] } });
|
||||
assert.deepEqual(run(root, "j"), ["studio-jobs"]);
|
||||
});
|
||||
|
||||
test("a provider NAMED but absent from disk is not invented", () => {
|
||||
// Reconciliation names a missing provider with a proper error; this script's job
|
||||
// is to say what to install, and it cannot install what is not there.
|
||||
const root = fixture({ x: { id: "x", provider: { package: "@openrig/studio-ghost" } } }, PROVIDERS);
|
||||
assert.deepEqual(run(root, "x"), []);
|
||||
});
|
||||
|
||||
test("positive control: the fixture CAN produce every provider", () => {
|
||||
// Without this, every assertion above would also pass against a script that
|
||||
// always returned an empty list.
|
||||
const root = fixture({
|
||||
a: { id: "a", provider: { package: "@openrig/studio-host" } },
|
||||
b: { id: "b", provider: { package: "@openrig/studio-factory" } },
|
||||
c: { id: "c", provider: { package: "@openrig/studio-video" } },
|
||||
d: { id: "d", provider: { package: "@openrig/studio-effects" } },
|
||||
}, PROVIDERS);
|
||||
assert.deepEqual(run(root, "a", "b", "c", "d"),
|
||||
["studio-effects", "studio-factory", "studio-host", "studio-video"]);
|
||||
});
|
||||
+20
-2
@@ -115,12 +115,30 @@ function checkClosure(dir, m) {
|
||||
}
|
||||
}
|
||||
|
||||
// "THIS APP HAS NO BACKEND" CAN BE SAID TWO WAYS AND BOTH ARE CORRECT MANIFESTS.
|
||||
//
|
||||
// Omitting `provider` entirely is one. Declaring `{ required: false }` with no
|
||||
// package is the other — an app SAYING it needs none, rather than leaving a reader
|
||||
// to infer it from an absence. `apps/workspace` does exactly that, with a note
|
||||
// explaining why, and it was UNINSTALLABLE ON ANY BOX: this checker read the
|
||||
// KEY'S PRESENCE as a declaration and emitted three failures against a manifest
|
||||
// that was documenting its own correctness.
|
||||
//
|
||||
// PRESENCE IS NOT MEANING. A provider is declared by a PACKAGE, not by a key
|
||||
// existing. Found by cloud-impl provisioning a real VPS, and it is the same shape
|
||||
// as both of their own findings in the same report.
|
||||
//
|
||||
// The genuinely malformed case is still caught: a `provider` key with no package
|
||||
// and no explicit `required: false` says nothing at all, and that still fails.
|
||||
const declaresNoProvider = (prov) =>
|
||||
!prov || (!prov.package && prov.required === false);
|
||||
|
||||
// DEFECT THIS CATCHES: cutdown's provider serves /media/, /cutprev/ and /cuts/.
|
||||
// Nothing declared them, so the studio front proxied only /api/* and the app
|
||||
// listed clips it could not play. A backend is not only its /api/ verbs.
|
||||
function checkRoutes(m) {
|
||||
const prov = m.provider;
|
||||
if (!prov) { ok("ultralight — no provider to route to"); return; }
|
||||
if (declaresNoProvider(prov)) { ok("ultralight — no provider to route to"); return; }
|
||||
if (!prov.package) fail("provider declared without a package name");
|
||||
|
||||
// Routing facts live on the PROVIDER once it has migrated. Checking only the
|
||||
@@ -147,7 +165,7 @@ function checkRoutes(m) {
|
||||
// hardcodes each one by name. A sixth app would need code here to run at all.
|
||||
function checkRunnable(m) {
|
||||
const prov = m.provider;
|
||||
if (!prov) return;
|
||||
if (declaresNoProvider(prov)) return;
|
||||
// How to run is the PROVIDER's fact. An app that has migrated declares only a
|
||||
// reference, so demanding provider.run.entry from the app manifest refuses
|
||||
// exactly the shape the contract requires.
|
||||
|
||||
Reference in new issue
Block a user