OverviewFilesCommitsIssuesDocsReleasesPeers

pgh commits / 3dae42a

0.1.137: supervisor re-resolves the child env on every spawn
The supervisor parsed PGH_SUP_CHILD_ENV ONCE at start and reused it for every respawn. So an OTA respawn (exit 75) re-exec'd the NEW code with the OLD environment: `pgh app update` installs a tree whose manifest declares new defaults.env, the app restarts, and none of it applies. `effectiveEnv` was always correct — it simply never ran again. Measured on tabertron 2026-08-05. favela-web auto-updated to 0.3.33 and came up without FAVELA_AP_BRIDGE, added to the manifest in that very release. Worse, it was also missing ATPROTO_OVERLAY_ROOT, which has been in the manifest since 0.2.25 — that variable points favela-web at hpp's SHARED record store, and without it the daemon silently fell back to its own empty overlay, so co-hosted DIDs were rebuilt from firehose backfill instead of the authoritative baseline. The timeline rendered normally, so nothing surfaced it. INSTALLED != RUNNING, one layer down. Env is now re-resolved per spawn — the same discipline the `cwd` already uses two lines below, for the same class of reason: a long-lived supervisor must not cache anything the install can change underneath it. Extracted to lib/child-env.js rather than left inline, because supervisor.js is a SCRIPT — importing it starts a respawn loop, so nothing in it could be asserted on. A defect here mis-configures every app on the fleet at once and usually still boots, so "it looked right" is not an acceptable standard. Two hazards handled: - PGH_SUP_* must be STRIPPED. effectiveEnv() starts from process.env, which in the supervisor holds all of it — re-resolving naively hands the app its own supervisor config, including the entire PGH_SUP_CHILD_ENV JSON blob as a string variable. - It NEVER throws. A supervisor that cannot spawn because a manifest read failed leaves the app DOWN, which is strictly worse than running one release behind on env. Every failure path returns the env the parent computed at start, and says so. The empty-env guard came from a failing test: {...null} is {}, which passed a typeof-only check and would have spawned an app with no PATH or HOME — failing in a way that looks nothing like a config problem. Also logs the changed keys once per change, because the original failure was invisible from every surface. 9 tests / 22 asserts. Adjacent suites re-run and green: supervisor-cwd-swap, supervisor-pidfile, find-running-supervisor, app-state-relocation. NB `npm test` is broken on this machine independently of this change — `brittle test/*.test.js` under node hits a bare-only import via hyper-overlay ("ReferenceError: Bare is not defined"); confirmed by stashing these changes and reproducing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3dae42af16d54285e0650ad5d2ba6890fc3d2167
by plankatron <fleeky@gmail.com> · 2026-08-05T20:45:57.000Z

Changed files (4) +195 -4

lib/child-env.js +74 -0
……@@ -0,0 +1,74 @@
1// child-env — resolve the environment a supervised app is spawned with.
2//
3// Extracted from supervisor.js so it is TESTABLE. supervisor.js is a script: importing it starts a
4// respawn loop, so nothing inside it could be asserted on. This is load-bearing enough that
5// "it looked right" is not an acceptable standard — a defect here mis-configures every app on the
6// fleet at once, silently, because a wrong env usually still boots.
7//
8// THE BUG THIS EXISTS FOR. The supervisor parsed PGH_SUP_CHILD_ENV once at start and reused it for
9// every respawn. An OTA respawn (exit 75) therefore re-exec'd the NEW code with the OLD environment:
10// `pgh app update` installs a tree whose manifest declares new `defaults.env`, the app restarts, and
11// none of it applies. `effectiveEnv` was always correct; it simply never ran again.
12//
13// Measured on tabertron 2026-08-05: favela-web auto-updated to 0.3.33 and came up without
14// FAVELA_AP_BRIDGE (added in that release) AND without ATPROTO_OVERLAY_ROOT — in the manifest since
15// 0.2.25. The latter points favela-web at hpp's SHARED record store; absent, it fell back to its own
16// empty overlay and co-hosted DIDs were rebuilt from firehose backfill instead of the authoritative
17// baseline. Everything still rendered, so nothing surfaced it.
18
19// The supervisor's own plumbing. Present in the SUPERVISOR's process.env but never in the child's,
20// because the parent computed the child env in a process that had none of it. effectiveEnv() starts
21// from process.env, so re-resolving inside the supervisor leaks all of this into the app — including
22// the entire PGH_SUP_CHILD_ENV JSON blob as a single string variable.
23export const SUP_ONLY = Object.freeze([
24 'PGH_SUP_NAME',
25 'PGH_SUP_ENTRYPOINT',
26 'PGH_SUP_RUNTIME',
27 'PGH_SUP_RUNTIME_PATH',
28 'PGH_SUP_ARGS_JSON',
29 'PGH_SUP_CHILD_ENV'
30])
31
32// Resolve the env for the next spawn.
33//
34// deps are injected (rather than imported) so a test can drive this without a real install tree:
35// readInstallManifest(name) -> manifest | null
36// effectiveEnv(name, manifest) -> object (merges process.env + defaults.env + user overrides)
37// fallback -> the env the PARENT computed at `pgh app start`
38// onWarn(msg) -> optional logger
39//
40// ⚠ NEVER THROWS. A supervisor that cannot spawn because a manifest read failed leaves the app DOWN,
41// which is strictly worse than running one release behind on env. Any failure returns `fallback`.
42export function resolveChildEnv ({ name, readInstallManifest, effectiveEnv, fallback = {}, onWarn = null } = {}) {
43 try {
44 if (!name) return fallback
45 const manifest = readInstallManifest(name)
46 // No manifest is NOT an error — an adopted or hand-placed app may have none. Keep what works.
47 if (!manifest) return fallback
48 const env = effectiveEnv(name, manifest)
49 // An empty result is as wrong as a non-object: effectiveEnv() always merges process.env, so a
50 // zero-key answer means something upstream failed. Spawning with no environment would strip PATH
51 // and HOME from the app — it would fail in a way that looks nothing like a config problem.
52 if (!env || typeof env !== 'object' || Object.keys(env).length === 0) return fallback
53 for (const k of SUP_ONLY) delete env[k]
54 // The parent sets this explicitly after effectiveEnv(): an install name can differ from the
55 // package name (adopted dev installs), and the app uses it to register with pgh-sidecar.
56 env.PGH_APP_NAME = name
57 return env
58 } catch (err) {
59 if (onWarn) { try { onWarn('env re-resolve failed, reusing the env from start: ' + (err && err.message)) } catch {} }
60 return fallback
61 }
62}
63
64// Which keys changed between two resolved envs. Used to say so ONCE in the log — the failure above
65// was invisible from every surface, and an env that silently differs from the manifest must not be
66// invisible again. Values are compared, not just key presence, so a CHANGED value is reported too.
67export function envDelta (before, after) {
68 if (!before) return []
69 const out = []
70 for (const k of new Set([...Object.keys(before), ...Object.keys(after)])) {
71 if (before[k] !== after[k]) out.push(k)
72 }
73 return out.sort()
74}
lib/supervisor.js +27 -2
……@@ -25,7 +25,8 @@
2525import process from '#node:process'
2626import path from '#node:path'
2727import { spawn } from '#node:child_process'
28import { writeState, writePid, clearPid } from './app-state.js'
28import { writeState, writePid, clearPid, readInstallManifest, effectiveEnv } from './app-state.js'
29import { resolveChildEnv as resolveEnv, envDelta } from './child-env.js'
2930
3031const NAME = process.env.PGH_SUP_NAME
3132const ENTRYPOINT = process.env.PGH_SUP_ENTRYPOINT
……@@ -37,8 +38,29 @@ const RUNTIME = process.env.PGH_SUP_RUNTIME || 'node'
3738const RUNTIME_PATH = process.env.PGH_SUP_RUNTIME_PATH ||
3839 (path.basename(process.execPath) === RUNTIME ? process.execPath : RUNTIME)
3940const ARGS = JSON.parse(process.env.PGH_SUP_ARGS_JSON || '[]')
41// The env the PARENT computed at `pgh app start`. Used for the first spawn and as the fallback
42// whenever a re-read fails — never as the permanent answer. See resolveChildEnv().
4043const CHILD_ENV = JSON.parse(process.env.PGH_SUP_CHILD_ENV || '{}')
4144
45// Env is RE-RESOLVED every spawn — the logic (and the reason) lives in child-env.js, which is
46// importable and therefore testable; this script is not. Same discipline as the re-resolved `cwd`
47// below: a long-lived supervisor must not cache anything the install can change underneath it.
48const childEnvFor = () => resolveEnv({
49 name: NAME,
50 readInstallManifest,
51 effectiveEnv,
52 fallback: CHILD_ENV,
53 onWarn: (m) => log(m)
54})
55
56// Say it ONCE when it changes. The failure this fixes was invisible from every surface.
57let prevEnv = null
58function logEnvDelta (env) {
59 const changed = envDelta(prevEnv, env)
60 if (prevEnv && changed.length) log('child env changed since last spawn: ' + changed.join(', '))
61 prevEnv = env
62}
63
4264const BACKOFF_MIN_MS = 1000
4365const BACKOFF_MAX_MS = 30000
4466const FAST_FAIL_S = 5
……@@ -95,9 +117,12 @@ async function loop () {
95117 while (!shuttingDown) {
96118 const startedAtMs = Date.now()
97119 log('spawning ' + RUNTIME_PATH + ' ' + ENTRYPOINT + (ARGS.length ? ' ' + ARGS.join(' ') : ''))
120 // Re-resolved every spawn, so an OTA respawn picks up the new tree's manifest env.
121 const childEnv = childEnvFor()
122 logEnvDelta(childEnv)
98123 child = spawn(RUNTIME_PATH, [ENTRYPOINT, ...ARGS], {
99124 stdio: ['ignore', 'inherit', 'inherit'],
100 env: CHILD_ENV,
125 env: childEnv,
101126 // Launch from the app dir, RE-RESOLVED every spawn. The app's OTA
102127 // (pgh-runtime) atomically swaps its install dir to a new inode; this
103128 // supervisor is a separate pgh process that keeps running, so its own
package.json +2 -2
……@@ -1,7 +1,7 @@
11{
22 "name": "pgh",
3 "version": "0.1.136",
4 "description": "pgh — P2P content-addressed package manager and app launcher",
3 "version": "0.1.137",
4 "description": "pgh \u2014 P2P content-addressed package manager and app launcher",
55 "type": "module",
66 "main": "index.js",
77 "bin": {
test/child-env.test.js +92 -0
……@@ -0,0 +1,92 @@
1// The env a supervised app is spawned with must be RE-RESOLVED on every respawn.
2//
3// Regression: the supervisor parsed PGH_SUP_CHILD_ENV once at start and reused it forever, so an OTA
4// respawn re-exec'd the new code with the old environment. Measured on tabertron 2026-08-05 —
5// favela-web came up missing both a variable added in that release AND one that had been in the
6// manifest since 0.2.25 (ATPROTO_OVERLAY_ROOT, which points it at hpp's shared record store).
7// Everything still rendered, so nothing surfaced it.
8import test from 'brittle'
9import { resolveChildEnv, envDelta, SUP_ONLY } from '../lib/child-env.js'
10
11const MANIFEST = { defaults: { env: { FOO: 'from-manifest', ADDED_LATER: 'yes' } } }
12const deps = (manifest, env) => ({
13 readInstallManifest: () => manifest,
14 effectiveEnv: () => ({ ...env })
15})
16
17test('picks up manifest env that did not exist when the supervisor started', (t) => {
18 const fallback = { FOO: 'stale' } // what the parent computed at start
19 const env = resolveChildEnv({
20 name: 'app', fallback,
21 ...deps(MANIFEST, { FOO: 'from-manifest', ADDED_LATER: 'yes' })
22 })
23 t.is(env.FOO, 'from-manifest', 'a changed value is picked up')
24 t.is(env.ADDED_LATER, 'yes', 'and a NEW variable appears — this is the actual bug')
25 t.is(fallback.FOO, 'stale', 'the fallback object is not mutated')
26})
27
28test('strips the supervisor plumbing — it was never in the child env', (t) => {
29 // effectiveEnv() starts from process.env, which in the SUPERVISOR contains all of this. Leaking it
30 // would hand the app its own supervisor config, including the whole child-env JSON blob.
31 const leaky = { FOO: 'x' }
32 for (const k of SUP_ONLY) leaky[k] = 'LEAKED'
33 const env = resolveChildEnv({ name: 'app', fallback: {}, ...deps(MANIFEST, leaky) })
34 for (const k of SUP_ONLY) t.absent(k in env, `${k} is stripped`)
35 t.is(env.FOO, 'x', 'while real variables survive')
36})
37
38test('always sets PGH_APP_NAME — the install name can differ from the package name', (t) => {
39 // A realistic env is never empty — effectiveEnv() always merges process.env, so PATH is present.
40 const env = resolveChildEnv({ name: 'pear-git-v3-dev', fallback: {}, ...deps(MANIFEST, { PATH: '/usr/bin' }) })
41 t.is(env.PGH_APP_NAME, 'pear-git-v3-dev', 'adopted dev installs register under the INSTALL name')
42})
43
44// ── never throws: down is worse than stale ──────────────────────────────────────────────────────
45test('a throwing manifest read falls back rather than leaving the app down', (t) => {
46 const warns = []
47 const env = resolveChildEnv({
48 name: 'app', fallback: { FOO: 'stale' }, onWarn: (m) => warns.push(m),
49 readInstallManifest: () => { throw new Error('ENOENT') },
50 effectiveEnv: () => ({})
51 })
52 t.is(env.FOO, 'stale', 'the app still starts, with the env it had')
53 t.is(warns.length, 1, 'and it is LOUD — a silent fallback is how the original bug hid')
54})
55
56test('a throwing effectiveEnv falls back too', (t) => {
57 const env = resolveChildEnv({
58 name: 'app', fallback: { FOO: 'stale' },
59 readInstallManifest: () => MANIFEST,
60 effectiveEnv: () => { throw new Error('bad user config') }
61 })
62 t.is(env.FOO, 'stale')
63})
64
65test('no manifest is not an error — adopted apps may have none', (t) => {
66 const env = resolveChildEnv({ name: 'app', fallback: { FOO: 'stale' }, ...deps(null, {}) })
67 t.is(env.FOO, 'stale', 'keeps what works')
68})
69
70test('a non-object or EMPTY env falls back rather than spawning with nothing', (t) => {
71 const bad = (v) => resolveChildEnv({
72 name: 'app', fallback: { FOO: 'stale' },
73 readInstallManifest: () => MANIFEST, effectiveEnv: () => v
74 })
75 t.is(bad(null).FOO, 'stale', 'null falls back')
76 t.is(bad('nope').FOO, 'stale', 'a non-object falls back')
77 // Found by this test: {...null} is {}, which passed a typeof-only guard. Spawning with an empty
78 // env strips PATH and HOME — the app dies in a way that looks nothing like a config problem.
79 t.is(bad({}).FOO, 'stale', 'an EMPTY object falls back too')
80})
81
82test('a missing name falls back', (t) => {
83 t.is(resolveChildEnv({ fallback: { FOO: 'stale' }, ...deps(MANIFEST, {}) }).FOO, 'stale')
84})
85
86// ── the delta report ────────────────────────────────────────────────────────────────────────────
87test('envDelta names added, removed AND changed keys', (t) => {
88 t.alike(envDelta({ A: '1', B: '2' }, { A: '1', B: 'CHANGED', C: 'new' }), ['B', 'C'],
89 'a changed VALUE counts, not just a new key')
90 t.alike(envDelta({ A: '1' }, { A: '1' }), [], 'no change reports nothing')
91 t.alike(envDelta(null, { A: '1' }), [], 'the first spawn has nothing to compare against')
92})