Skip to content

[v0.8 Core 9] preserve recovered worker registry authority - #1336

Open
sethkarten wants to merge 2 commits into
v080/core-split-c7-lifecyclefrom
v080/core-split-c9-worker-registry
Open

[v0.8 Core 9] preserve recovered worker registry authority#1336
sethkarten wants to merge 2 commits into
v080/core-split-c7-lifecyclefrom
v080/core-split-c9-worker-registry

Conversation

@sethkarten

@sethkarten sethkarten commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Replacement scope

This PR reconstructs and supersedes the unique implementation delta reviewed in #1267 without rewriting that historical branch. The original PR remains the immutable discussion record: #1267

  • Base: v080/core-split-c7-lifecycle
  • Replacement branch: v080/core-split-c9-worker-registry
  • Replacement commit: 5413a1ca6092689c1fb1eb2f5ced6c300d45b2ad
  • Propagation/reconciliation merge commits are intentionally excluded.
  • fix(mcp): make cleanup failures observable #1264 is intentionally omitted from the replacement stacks because its declared-base-to-head tree delta is empty.
  • Frozen feat(kernel): dispatch host requests as capabilities #1243 (77b188b92dc91365cb2bc41bdb46a50669d104a8) is the shared foundation. For reconstructed deltas it is a proven tree-compatible base, not an ancestry claim about the historical PR stack.

Validation

  • Biome 2.5.5 on the exact changed paths: pass
  • root tsgo --noEmit: pass
  • Core focused suite on the final Core tip with live daemon/RLM environment removed and single-worker execution: 11 files, 388 tests passed
  • MCP focused suite on the final MCP tip: 9 files, 106 tests passed
  • Independent Terra tree/delta review: pass

No original PR was retargeted, closed, merged, or otherwise mutated.


Note

Medium Risk
Changes daemon ownership and worker spawn env for a security-sensitive coordination path; behavior shifts if anything relied on overriding the registry env var.

Overview
Pins the daemon supervisor registry directory so ownership, startup fences, and recovered workers all use the same on-disk authority instead of re-resolving from ambient process.env on each call.

DaemonSupervisor captures supervisorRegistryDir at startup and passes it into waitForDaemonStartupFence, acquireDaemonSupervisorOwnership, and every worker spawn. Worker env sets PRIME_AGENT_INTERNAL_DAEMON_SUPERVISOR_REGISTRY_DIR after inherited and caller launchEnv, so untrusted overrides cannot redirect registry access. defaultDaemonSupervisorRegistryDir is renamed/exported as getDaemonSupervisorRegistryDir with the env constant exported for tests and spawn wiring.

Tests assert both initial and relaunched workers receive the supervisor’s registry path and that launchEnv cannot smuggle a different directory.

Reviewed by Cursor Bugbot for commit 240b454. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Preserve daemon supervisor registry authority across recovered workers

  • Exports DAEMON_SUPERVISOR_REGISTRY_DIR_ENV and renames defaultDaemonSupervisorRegistryDir to getDaemonSupervisorRegistryDir in daemon-supervisor-ownership.ts so other modules can resolve the registry directory consistently.
  • DaemonSupervisor now captures a single supervisorRegistryDir at startup and passes it explicitly to fencing and ownership calls, preventing drift between restarts.
  • When spawning worker processes, the supervisor injects DAEMON_SUPERVISOR_REGISTRY_DIR_ENV into the worker environment after merging all other env vars, so a caller-supplied value cannot override the supervisor's registry directory.
  • Risk: workers that previously relied on inheriting or overriding the registry env var will now always use the supervisor-enforced value.

Macroscope summarized 240b454.

Reconstruct the unique net delta from PR #1267, excluding propagation merges.

@jonaowen jonaowen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The worker override ordering is sound, but the registry authority is not pinned transitively yet.

  1. getDaemonSupervisorRegistryDir() returns environment text verbatim. A relative or Windows drive-relative value is interpreted from the predecessor/supervisor cwd, while workers and the update successor can run with different session/default cwd values. The identical injected string can therefore name different physical registries. Normalize once to an absolute path (or reject non-absolute values) before ownership and forward that exact authority to workers and every successor supervisor.
  2. assertRecoveryAllowed() calls isDaemonShutdownAdmissionActive() without the captured registry, so it re-reads ambient process.env; self-relaunch does the same. Later environment drift can make admission checks consult B while ownership/fences live in A. Pass the capture explicitly through admission/recovery and relaunch, and ensure worker owner/admission/fence reads do not independently drift.

Please add behavioral coverage with distinct supervisor/worker/successor cwd values, persisted-descriptor recovery under a second generation, ambient A→B drift with admission held separately in A/B, update self-relaunch receiving A, and Windows relative/drive-relative plus case-varied caller env keys. The current mocked test only inspects worker SpawnOptions.env for one in-memory descriptor and does not exercise these authority boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants