Skip to content

feat(deploy): two-phase stage/activate for deploy_component - #1849

Closed
dawsontoth wants to merge 108 commits into
mainfrom
claude/deploy-component-two-phase-94969a
Closed

feat(deploy): two-phase stage/activate for deploy_component#1849
dawsontoth wants to merge 108 commits into
mainfrom
claude/deploy-component-two-phase-94969a

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Draft / RFC. Splits deploy_component into two replicated phases with a cluster-wide barrier at go-live, and adds a fast, addressed rollback. The public request/response contract is unchanged. Stacked on #2066 (see Sequencing), so it should merge after it.

Decisions needed before this leaves draft

Four review findings are design changes to the two-phase contract rather than bugs with an obvious
fix. Each is defensible as a follow-up on a PR this size, and each is also arguably a correctness gap
worth closing before 5.3 ships. They need a call, not a patch — tick the ones you want in this PR.

  • Cluster-wide activation ordering (heskew, the round's only High). The activate fan-out
    happens after the origin releases its node-local component lock, so nothing orders two
    concurrently-originated deploys. If node A originates D1 and node B originates D2, each can
    apply the other's fan-out last: A ends on D2, B on D1, and both deployments report success.
    The stage barrier does not help — it orders stage-before-activate, not deploy-against-deploy.
    Fixing it means a cluster-wide per-project order: leader serialization, or a monotonic
    activation epoch compared under the component lock and rejected if stale. Either is a real
    addition to the protocol, and the epoch option needs a home for the counter that survives
    restarts.
    Not fixed here. Today, concurrent originations are a documented sharp edge rather than a
    guarded one.

  • Resolve the package to bytes once, on the origin. For a package: deploy the origin sends
    the identifier, not a payload, so every node independently resolves and packs it. A moving
    latest, a semver range, or a git branch can therefore resolve differently per node while
    every stage reports success — the barrier confirms "everyone staged something", not "everyone
    staged the same thing". The fix is to resolve once on the origin, persist the exact tarball plus
    a digest on the deployment row, and have peers stage and verify those bytes.
    This changes what the barrier guarantees, and it makes a package: deploy carry a payload it
    currently does not, with the retention cost that implies.

  • Require the deployment table for separated-phase operations. DeploymentRecorder.put()
    silently no-ops when hdb_deployment is absent, but activate: false still returns a
    deployment_id. A later deploy_component({ deployment_id }) then cannot find the row and
    cannot activate the build — a stage that reported success is unactivatable. The fix is to
    require tracking availability for activate: false / deployment_id paths while keeping the
    tolerant fallback for legacy one-shot deploys. Straightforward, but it turns a currently-quiet
    configuration into a hard failure, which is a product call.

  • Peer row ownership. claimStagedDeployment ends in a patch on the replicated
    hdb_deployment row, and every peer runs it in its activate beforeSwap. So N peers plus the
    origin write the same key — the concurrent same-key pattern seal() exists to avoid — and under
    replication lag a peer can patch activating after the origin wrote success, reverting a
    converged deploy to a permanently non-terminal state. Both DESIGN.md ("peers make the same
    local claim") and revertComponent say the origin owns the row, so the implementation
    contradicts the stated design. The fix is a persist: false mode for peers, whose claim is then
    enforced by the component preparation lock they already hold.

Everything else from the review rounds is addressed in-branch — see the commit log and the resolved
threads.

Why

Before this, the whole deploy ran as one replicated operation, and on every node extract + npm install — the slow, failure-prone work: git clone, registry install — happened in place in the live component directory, right before restart. There was no cluster-wide barrier: a peer could fail npm install and sit half-baked while other nodes had already restarted onto the new code. Nodes flipped independently.

(main has since made in-place preparation transactional per node via #1936/#2066, so the per-node rollback half of that argument is now solved upstream. The cluster-wide ordering argument is what this PR is for, and it stands on its own.)

What this does

Two phases, orchestrated internally by deploy_component:

Phase Does Does not
1. Stage download/npm pack (incl. git clone), extract, npm install into a hidden .deploy-staging/<deploymentId>/<name> on every node touch the live dir, write config, restart
2. Activate atomic rename(staging → live), persist config + install lock, restart re-fetch or re-install anything

The origin stages locally, waits for every node to report a good stage before any node activates, then activates. A node that can't fetch the package or fails npm install fails during staging, and the live component is untouched on every node. Go-live shrinks to a fast atomic swap. Single-node instances benefit too: npm install runs off to the side while the live component keeps serving.

Public surface

One public deploy operation. The phases are not separate operations — the peer fan-out is a distinct authenticated component_deploy_phase operation, so a pre-upgrade peer rejects an unknown operation instead of misreading a phase marker as a one-shot deploy. Public _phase/_deploymentId fields are rejected.

Operators get the phases through deploy_component properties:

  • default → full stage + activate (contract unchanged)
  • activate: false → stage cluster-wide and stop, returning the deployment_id in a staged state
  • deployment_id: <id> (no new payload) → activate a previously-staged deployment
  • two_phase: false → the legacy one-shot path, preserved verbatim

revert_component is a separate public operation: it is a rollback, not a deploy phase, and it resolves no package, decrypts no secret, downloads no artifact and runs no install.

Rollback: addressed, idempotent, config-level

Every activation retains the tree it displaced as .deploy-previous/<name> with a sidecar manifest recording which deployment produced it and the root-config entry it was activated with. revert_component takes a required to_deployment_id:

  • already live → no-op success, so a retry after a lost response cannot flip the rejected release back in
  • matches the retained previous → swap; the displaced tree becomes the new retained previous, so an explicitly targeted revert-of-a-revert rolls forward
  • anything else → refused, naming what the component can actually revert to

The swap carries persistent state with it: root config and harper-application-lock.json move via a shared createApplicationConfigTransaction. Reverting away from a package deploy removes the package reference, so installApplications() can't reinstall the reverted-away version over the restored directory on the next cold start. Exactly one previous is retained, so revert reaches back one activation; anything older is a redeploy.

Automatic rollback is deliberately not offered (revert_on_failure is rejected). Once a node is past the barrier, "peer reported failed" does not imply "peer did not activate" — a peer can complete its swap and then fail the work that follows — so auto-reverting the failed peers can roll an untouched node an extra version back and split the cluster three ways. A partial activation stays visibly activating and is rolled forward, or rolled back explicitly by target.

Sequencing: stacked on #2066

Per @kriszyp's review, this branch is based on #2066 rather than main, so there is exactly one naming contract on .deploy-aside instead of two. extractApplication runs #2066's aside transaction against application.buildDirPath — the live dir by default (one-shot, boot installs and direct callers unchanged), the staging dir during a stage — so one protocol covers both. The evicted two-deploys-ago tree is parked under a distinct .discarded- prefix, because #2066's startup sweep restores an unretired .in-progress- directory over the live component and known-garbage must never look like a rollback record.

Why staging lives under the components root

Go-live is rename(stagingDir, liveDir), atomic only when both share a filesystem. os.tmpdir() is frequently a different mount → EXDEV → a slow recursive copy at exactly the moment you want an instant swap. So staging is a hidden dir under the components root: same volume, dot-prefixed so the loader ignores it, and not the watched base of any component's watcher — so building there fires no restart-on-change events.

Durability

Stage/activate is a durable transaction: completion markers, immutable per-deployment activation specs, and atomic live/config/lock activation. Startup reconciliation rolls an interrupted activation forward, rebuilds a missing peer stage from the retained payload, and preserves uncertain partial activation as activating. Staged builds and payloads are bounded by deployment_stagingRetention_maxCount and deployment_payloadRetention_maxCount (default 1, conservative on purpose — N copies of a large payload competing with customer data for a small quota is a nasty failure mode).

Restart-required gate

Installed package metadata sits outside most plugin watch globs, so no watcher sees a dependency or module-entry change — but it invalidates loaded code. activateStagedApplication compares the outgoing live tree against the staged tree at swap time and feeds markRestartRequiredForDeploy, matching what the in-place path gets from prepareApplication. The stage records whether its install was opaque inside its completion marker, since on a peer the stage and the activate are separate invocations.

Tests

~300 unit tests across deployStaging, deployPhaseOperations, deployPhaseValidators, deploymentRecorder, deploymentOperations, cliOperations and the server dispatch seam — covering the barrier, crash windows, duplicate/concurrent activation, trusted-peer dispatch, fail-closed paths, retention, the addressed revert (including the retry no-op and config removal), and the restart gate.

Still open

🤖 Generated with Claude Code

Split deploy_component into two replicated phases so a cluster deploy is
all-or-nothing at go-live, and expose each phase as a first-class operation.

- stage_component (phase 1): build the incoming version — download/npm pack
  (incl. git clone), extract, npm install — into a hidden `.deploy-staging`
  dir on every node. Never touches the live component dir, writes no config,
  never restarts, so it's safe to run cluster-wide and gate on.
- activate_component (phase 2): atomically rename the staged copy into the
  live path and restart; persist root config for a `package` deploy at go-live.
- deploy_component orchestrates stage -> (barrier: every node staged OK) ->
  activate. Request/response contract unchanged; SSE now emits stage/activate
  phases. `two_phase: false` forces the legacy one-shot path, preserved verbatim
  for opt-out, for peers replaying a one-shot deploy, and when `system` isn't
  replicated.

Application.ts gains stageApplication/activateApplication/discardStagedApplication;
extract/install now build into `buildDirPath` (defaults to the live dir, so the
one-shot path, boot install, and direct extractApplication callers are unchanged).
Staging lives under the components root — same filesystem — so go-live is an atomic
rename (an os.tmpdir() location risks EXDEV and a slow copy at the worst moment).

Adds unit tests for the stage/activate/discard primitives and the new validators;
DESIGN.md documents the model and the atomic-rename/filesystem tradeoff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a two-phase deployment process (stage then activate) for components to ensure all-or-nothing cluster-wide deploys. Staging builds the incoming version into a hidden staging directory, while activation atomically swaps it into the live path. The review feedback highlights two critical issues in components/Application.ts: first, recursively deleting the parent staging directory during cleanup can accidentally destroy parallel staged builds for the same component; second, using access to check directory existence follows symlinks, which will fail on dangling symlinks and cause subsequent operations to throw EEXIST. Both comments provide actionable code suggestions to resolve these issues.

Comment thread components/Application.ts Outdated
Comment thread components/Application.ts
Comment thread components/Application.ts Outdated
Comment thread components/operations.js
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

dawsontoth and others added 3 commits July 17, 2026 15:33
…ling-safe cleanup

Three fixes to the two-phase staging path:

- Create the per-deploy staging parent before extraction. For a `package`
  deploy the first filesystem touch is the `npm pack`/git-clone spawn, whose
  cwd is dirname(stagingDirPath); it didn't exist yet, so the spawn failed with
  `ENOENT posix_spawn` (surfaced by the deploy-from-github integration test; the
  payload-only unit tests never hit the spawn). stageApplication now mkdirs it.

- moveDirAside uses lstat, not access(F_OK): access follows symlinks, so a
  DANGLING symlink at the target reported ENOENT and was skipped, then
  mkdir failed EEXIST. lstat sees the link itself. (Gemini review.)

- Reorder staging to .deploy-staging/<deploymentId>/<name> (was <name>/<id>).
  The leaf basename is now the component name, which the pre-go-live validation
  load needs (componentLoader keys ApplicationScope/status off basename); and
  each deploy gets its own parent, so cleanup can't sweep a parallel/queued
  deploy's staged build. activate cleanup is a non-recursive rmdir of that
  parent (empty-only). (Gemini review.)

Adds regression tests: a `file:`-tarball package-path stage, sibling-build
survival across activate, and dangling-symlink aside on activate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nt, one-shot

Adds orchestration tests that call operations.stageComponent /
activateComponent / deployComponent(two_phase:false) directly, stubbing the
build/swap primitives, blob ingest, and credential resolution so they assert
control flow (which primitive runs, replication + restart firing, response
shape) without npm/network or the component loader. Covers the two new
first-class operations and the legacy one-shot path (previously only exercised
indirectly). Addresses the coverage gap flagged in review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two-phase deploy lifecycle emits stage/activate phases instead of the
one-shot prepare/replicate. Update the deployment-tracking integration tests
to match: a failed install is now recorded against phase 'stage' (not
'prepare'), and a successful deploy's event_log spine is stage → activate
(not prepare → replicate). No behavior change — the recorded status='failed',
error.message, install_output, deployment_id, error/payload_dropped events are
all still asserted and preserved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread unitTests/components/deployPhaseOperations.test.js Outdated
dawsontoth and others added 5 commits July 17, 2026 15:51
…ewire)

AGENTS.md forbids new sinon/rewire in unit tests; the prior version of this
file stubbed the build primitives via rewire/sinon. Rewritten in the
deployStaging.test.js style: plain node:assert against the real operation
handlers, driving real tarball payloads through a real temp components root.
Now exercises stage_component, activate_component, deploy_component (two-phase
default), and deploy_component(two_phase:false) end-to-end — asserting the
staged/live directories on disk, the deployment_id, the no-restart message, and
the deployment_id requirement — which is stronger coverage than the stubs gave.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the DESIGN.md conflict by keeping both appended sections (two-phase
deploy + universalHeaders). main did not touch any of the deploy-family source
files, so the code merged cleanly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the DESIGN.md conflict by keeping both appended sections (two-phase
deploy + scheduler). main did not touch any deploy-family source file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ency call

Unrelated to the two-phase deploy feature — fixes a pre-existing type error on
main (introduced by #1688's commit-latency analytics) that main's other build
steps swallow via `|| true`/`continue-on-error`, but the newly-added Next.js
adapter integration workflow (#1385) runs the build without that tolerance and
so fails on it (tsc exit 2) for every PR built on current main.

`commitResolution` is declared with the wider `Promise<number | void> | void`
(the abort() branch reassigns it), but at this call site it is the `commit()`
promise already cast to `Promise<void>` on the line above. recordCommitLatency
only awaits it for timing and never reads the resolved value, so the cast is
type-only with no runtime effect — matching the author's existing cast and
safety comment two lines up.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the draft PR's open questions:

- Reversibility (Q3): activate now RETAINS the outgoing live version as
  .deploy-previous/<name> (one per component, older evicted) instead of
  discarding it, and a new revert_component operation swaps live <-> previous
  cluster-wide (replicated like activate). The swap is bidirectional, so
  reverting a revert rolls forward again. This unlocks customer-driven rollback
  (deploy -> run your own health checks -> revert if unhappy) and deploy_component
  gains an opt-in revert_on_failure that rolls the whole cluster back when the
  activate phase leaves it split across versions. New revertApplication primitive,
  operation handler, validator, enum, authorization, SSE, and 'reverting' status.

- CLI (Q4): `harper stage` (packages + uploads like `harper deploy`, no go-live),
  `harper activate`, and `harper revert` — aliases + SSE progress wired in
  bin/cliOperations.ts; stage shares deploy's cwd-packaging prep.

- Mixed-version clusters (Q1): clusters stay in lockstep on their version, so the
  rolling-upgrade caveat and any capability-negotiation framing are dropped from
  DESIGN.md / validator comments.

- Replicator contract (Q2): documented in DESIGN.md from harper-pro's
  replicator.ts — replicateOperation fans to server.nodes, sets replicated=false
  as the peer re-fan guard, surfaces per-peer {status:'failed',reason,node},
  authenticates by node cert, and runs replicated ops with authorize=false for
  trusted nodes (skipping the permission gate). Confirms the sub-op design is
  structurally identical to the proven deploy_component fan-out.

Adds 12 tests: retention + bidirectional revert primitives, revert operation
end-to-end, and the revert/revert_on_failure validators. 39 deploy unit tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread components/operations.js Outdated
replicateOperation fans to every node with no subset targeting, so the prior
revert_on_failure reverted failed-activate peers too. But a peer that failed to
activate never ran retainAsPrevious — its live dir is still the correct
pre-deploy version and its .deploy-previous holds a copy from two deploys ago —
so reverting it rolled it back an EXTRA version, splitting the cluster across
three versions instead of reconverging on one.

Scope the swap-back to the origin plus the peers that actually activated, sent
point-to-point via sendOperationToNode (skipping recorder.getFailedPeers()), and
leave the failed peers on their already-correct version. (Review catch.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread components/operations.js Outdated
…fan-out

The origin is reverted directly, then activatedPeers (server.nodes minus failed
peers) was sent a point-to-point revert too. server.nodes normally excludes self
(knownNodes populates it with a `!== getThisNodeName()` guard), but a not-yet-named
node can slip in, and the established convention (bin/restart.ts) guards self on
every point-to-point fan-out — without it, a self-directed revert would run the
handler again and, because the swap is bidirectional, flip the origin back to the
just-activated (broken) version. Filter out getThisNodeName() alongside the failed
peers. (Review catch.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread components/operations.js Outdated
The revert_on_failure fan-out needs a live multi-node cluster to run end-to-end,
so its node-targeting (skip failed peers, skip self) had no unit coverage — which
is why both bugs there were caught by review rather than a test. Extract that pure
set-difference into an exported selectRevertTargets(nodes, failedPeers, thisNode)
and cover it directly with plain assert: excludes self (the bidirectional
double-revert guard), excludes every failed peer, and is safe with empty/undefined
inputs and null-node failed entries. deployComponentTwoPhase now calls the helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dawsontoth

Copy link
Copy Markdown
Contributor Author

Thanks @kriszyp — draft or not, the review landed the key question squarely. revert_on_failure's cluster node-targeting was the subtle part, and it surfaced two real bugs: reverting peers that never activated (rolling them an extra version back), and a self-directed double-revert that flipped the origin right back to the broken version. Both are fixed, and the targeting is now a pure selectRevertTargets(nodes, failedPeers, thisNode) with direct unit tests so neither can regress.

A few calls I deliberately left for a maintainer rather than deciding unilaterally — your read would be welcome:

  • revert_on_failure is opt-in, not the default. Auto-reverting a cluster after some nodes have already restarted onto the new version is a policy choice I didn't want to make for you.
  • revert is a bidirectional swap (live ↔ .deploy-previous, one previous retained per component) rather than a rollback tied to a specific deployment_id. The bidirectionality is what makes "reverting a revert rolls forward again" work, but it's also exactly why the self-revert bug bit.
  • The retention model, the atomic-rename/.deploy-previous reasoning, and the replicator contract this rides on are written up in DESIGN.md's "Two-phase deploy" section.

Still a draft on purpose: the cross-node fan-out is validated against harper-pro's replicator by construction (documented), but hasn't been exercised on a live multi-node cluster — that plus your design read are the gates before it comes out of draft.

🤖 Automated update via Claude Code on behalf of @dawsontoth

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there is an associated PR comment associated with this review (in draft state that needs a submission).

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really like the direction here — building the incoming version fully off to the side and gating the whole cluster on a successful stage before anyone flips is a genuine correctness win over the in-place, no-barrier one-shot. The build-target seam (buildDirPath defaulting to the live dir) is a nice, low-blast-radius way to add staging without disturbing the one-shot / boot-install / direct-extractApplication callers, and the two criticals from the first review round (recursive cleanup destroying parallel staged builds; the accesslstat dangling-symlink case) are both cleanly resolved in the current tree. CI is green across the full matrix. Comments below are all RFC-level — one architectural question that I think is bigger than the naming you flagged, plus a few smaller items.

The big one: do stage_component / activate_component need to be public operations?

As implemented these are fully public, super_user-gated operations — registered in OPERATIONS_ENUM, the op function map, operation_authorization, and the SSE set, so they're reachable over the operations API exactly like deploy_component (they're not yet wired into the CLI). Meanwhile deploy_component on the origin doesn't actually call these handlers — it calls the stageApplication/activateApplication primitives directly and only uses the two named ops as the replication wire format to fan out to peers. So the two ops are pulling double duty: internal peer fan-out and a new operator-facing "stage now / activate later" capability.

Those two needs are separable, and only one of them arguably needs public surface:

  • Peer fan-out needs a wire representation, but not two named public ops. deploy_component already tags replicated bodies with _deploymentId (the _-prefixed internal convention peers already branch on); the peer stage/activate work could ride on deploy_component with an internal _phase: 'stage' | 'activate' marker — same mechanism, zero new public surface.
  • Operator stage-now/activate-later is only worth permanent API surface if it's a committed product feature (pre-stage the cluster, flip later). Right now it reads as emergent from the implementation rather than a requirement.

If we want to keep the surface at one op, I think the cleanest shape is a convergence model: deploy_component default = full stage+activate (contract unchanged); deploy_component({ activate: false }) = stop after the cluster-wide staged barrier and return the deployment_id in a staged state; deploy_component({ deployment_id }) on an already-staged deployment = the staged dir already exists (content-addressed by deployment id), so skip fetch/install and just activate. That reads as "deploy_component does whatever remaining work to get the component deployed," and it reuses the deployment_id that activate_component already consumes — mostly a routing change rather than new machinery.

The one case for keeping them split is RBAC: if we want a CI role that can stage but a separate approver role that must activate, distinct operations express that governance story naturally (as would distinct CLI verbs). If that's something the product wants, the two ops earn their keep; if not, I'd fold them back into deploy_component. Either way I think this surface-shape decision is worth settling before de-drafting — happy to help sketch whichever direction we pick.

Smaller findings

1. Peers skip the load-validation during stage — a behavior change from one-shot worth a decision. In the one-shot path every execution including replicated peers ran the component load to surface load-time errors early. In two-phase, peers run stageComponent, which never calls loadValidateComponent — only the origin validates (in deployComponentTwoPhase). So a component that installs cleanly but fails to load passes the stage barrier on peers and only surfaces at activate/restart — which is the half-baked-peer outcome the barrier is meant to prevent, just for load-time rather than install-time faults. Standalone stage_component never load-validates at all. Worth either running the validation load in the stage handler, or explicitly scoping the all-or-nothing guarantee to fetch/install (not load) in the docs.

2. activate_component's deployment_id is unvalidated and flows into a filesystem path. activateComponentValidator has deployment_id: Joi.string().optional() with no pattern, unlike project (/^[a-zA-Z0-9-_]+$/). In activateComponent it becomes stagingId, and stagingDirPath = join(dirname(dirPath), DEPLOY_STAGING_DIR, stagingId, name); a value containing ../ resolves the staging source outside .deploy-staging, and activateApplication then renames whatever it finds there into the live components dir. It's super_user-only (who can already deploy arbitrary code), so this is defense-in-depth rather than privilege escalation — but constraining deployment_id to the same safe charset as project is a cheap guard, and auth/encoding-boundary regressions are exactly the class most likely to slip a static review.

3. Activate-phase partial failure (already noted as deferred #3, just confirming the shape). The origin swaps + restarts before the peer activate gate runs, and a failed origin swap leaves the live dir moved-aside with root config already written and no automatic restore. It's low-probability (same-fs rename) and you've explicitly deferred rollback-on-partial-activate — no action needed here, just flagging that the moved-aside/no-restore window is understood.

Confirmations (verified, all good)

  • Watcher claim holds. EntryHandler watches component.commonPatternBase, derived per live component dir — .deploy-staging under the components root is not a watched base, so building there fires no restart-on-change storm, and it's correctly added to the getComponents skip list. The os.tmpdir()-vs-EXDEV rationale for keeping staging on-volume is sound.
  • Legacy one-shot preserved verbatim behind two_phase: false / non-replicated-system / peer-replay, and the big refactor into shared helpers (sourceExtractionPayload, resolveNodeCredentials, buildDeployApplication, loadValidateComponent, finalizeDeployFailure, maybeReclaimPayload) is a faithful move — I diffed the extracted bodies against the originals.
  • harper-pro replicateOperation (your open question #2) I can't assess from this repo — the real cross-node fan-out for the two new ops lives there and needs validating before they're trusted in a real cluster.

On the two things you asked about

  • Naming (stage/activate): I'd keep it. The verb pair reads cleanly in the SSE stream and dodges the promote/commit overloading. (Moot if we fold back into deploy_component.)
  • Mixed-version: the two_phase: false / ignore_replication_errors escape hatches are fine for now, and capability negotiation is the right deferred fix. Given main is 5.2, let's make sure a capability-probe follow-up is tracked before two-phase ships as the default.

Nice work — the correctness core is solid and this is close. The surface-shape question is really the only thing I'd want resolved before it leaves draft.

— KrAIs (Kris's Claude agent) 🤖

…ring stage

Two review findings from kriszyp's review:

- deployment_id becomes a staging-dir path segment
  (.deploy-staging/<deployment_id>/<name>), but activate/revert validators
  accepted any string — a `../` value could resolve the staging source outside
  .deploy-staging. Constrain it to the same safe charset as `project`
  (defense-in-depth; the ops are super_user-only). +tests.

- stageComponent never ran the pre-go-live component load check, so a standalone
  stage_component didn't validate at all and the stage barrier didn't cover
  load-time faults. Run loadValidateComponent on the staged build in the stage
  handler (matches deployComponentTwoPhase's origin check). It is a no-op on the
  main thread where replicated peer executions run (app code must not load
  there), so DESIGN.md now scopes the cluster-wide barrier guarantee to
  fetch + install and documents load-validation as origin/worker-side.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dawsontoth

Copy link
Copy Markdown
Contributor Author

Thanks @kriszyp — genuinely useful review. Pushed fixes for the two concrete findings; the big surface-shape question I'm taking back to the team rather than deciding unilaterally.

Smaller findings

  1. Load-validation / barrier scope. stageComponent now runs loadValidateComponent on the staged build (matching the origin, and closing the "standalone stage_component never validated" gap). Digging into the threading, though: replicated peer executions run on the main thread (replicateOperationsendOperationToNode), where loadValidateComponent deliberately no-ops (app code must not load on main) — so it fires on the origin / op-API worker but not on replicated peers, in the one-shot path too. Rather than claim a cluster-wide load guarantee I can't deliver without dispatching the load to a worker per peer, I scoped it honestly in DESIGN.md: the barrier gates fetch + install cluster-wide; load-validation is an origin/worker-side early check, with gating load faults on peers noted as a follow-up.
  2. deployment_id path-traversal. Good catch — constrained it to project's safe charset in the activate/revert validators (+ tests), so a ../ is rejected before it can become a .deploy-staging/<id>/<name> path segment.
  3. Partial-activate window. Confirmed, still deferred as you noted (rollback-on-partial-activate + the moved-aside/no-restore window).

Naming / mixed-version. Keeping stage/activate. Agreed on tracking a capability-probe follow-up before two-phase becomes the default — happy to open that issue if you'd like, or leave it to you.

The big one — public ops vs. fold into deploy_component. This is the right question and I don't want to set the public API shape unilaterally, so I'm taking it to @dawsontoth. Your convergence model (deploy_component default = stage+activate; {activate:false} = stage-and-stop returning a staged deployment_id; {deployment_id} = activate an existing stage) is appealing and mostly a routing change. The thing that tips it toward keeping them split is the RBAC/governance story — a CI role that can stage and a separate approver role that must activate — which the CLI verbs also lean into. We'll settle the surface shape before de-drafting and I'll follow up with the direction.

🤖 Automated update via Claude Code on behalf of @dawsontoth

@dawsontoth

Copy link
Copy Markdown
Contributor Author

Another reason that supports splitting them is the deployment strategies it unlocks, and customer facing features that we can enable. Especially on the rollback side of the equation. My theory? Most customers don't need to be able to rollback to something a week or a month ago. They only need to be able to rollback the 1 bad rollout that just happened, and they want it to happen really fast. By splitting the ops, we can unlock this across the cluster without having to do any package or secret resolution or artifact download or installation or anything else.

dawsontoth and others added 2 commits July 20, 2026 15:19
Resolve the DatabaseTransaction.ts conflict by combining both changes: keep my
`as Promise<void>` cast on the recordCommitLatency call (the Next.js-workflow
build fix) AND main's new write-queue-depth accounting (enterWriteQueue /
leaveWriteQueue) that landed on the same line. main did not touch any
deploy-family source file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… _phase)

Per review (harper#1849): stage_component / activate_component are no longer
public operations. Their two-phase cluster fan-out now rides deploy_component
itself, tagged with an internal `_phase: 'stage' | 'activate'` marker, and the
operator-facing capability is exposed as deploy_component properties:

- deploy_component (default): full stage + activate (contract unchanged).
- deploy_component({ activate: false }): stage cluster-wide and stop, returning
  the deployment_id in a `staged` state.
- deploy_component({ deployment_id }): activate a previously-staged deployment.

deployComponent now dispatches replicated _phase executions to internal
deployPhaseStage / deployPhaseActivate handlers, and public calls to the
orchestrator / activate-existing path. Removed the two ops from OPERATIONS_ENUM,
the op function map, operation_authorization, the SSE set, and their validators;
added activate + deployment_id (safe charset) to deployComponentValidator. Added
markDeploymentTerminal to flip a stage-and-stop row to success on later activate.

revert_component stays a distinct public op (a rollback, not a deploy phase).

CLI: `harper stage` -> deploy_component activate=false (still packages the cwd);
`harper activate deployment_id=<id>` -> deploy_component deployment_id=<id> (no
upload). Tests + DESIGN.md updated to the single-public-op shape; 34 deploy unit
tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dawsontoth

Copy link
Copy Markdown
Contributor Author

Surface-shape decision landed (thanks @kriszyp): folded stage_component / activate_component into deploy_component — done in d80e695.

  • There are no separate stage/activate operations anymore. The two-phase cluster fan-out rides deploy_component itself, tagged with an internal _phase: 'stage' | 'activate' marker (the _-prefixed convention peers already branch on) — so the wire format carries the phases and only deploy_component is publicly exposed.
  • The operator-facing capability is now deploy_component properties, matching your convergence sketch:
    • default → full stage + activate (contract unchanged)
    • activate: false → stage cluster-wide and stop, returns the deployment_id in a staged state
    • deployment_id: <id> (no new payload) → activate a previously-staged deployment
  • Deregistered the two ops from OPERATIONS_ENUM, the op map, operation_authorization, the SSE set, and removed their validators; added activate + deployment_id (safe charset) to deployComponentValidator.
  • CLI verbs are kept as sugar: harper stagedeploy_component activate=false (still packages the cwd), harper activate deployment_id=<id>deploy_component deployment_id=<id> (no upload).
  • One scope call I made: revert_component stays a distinct public op — it's a rollback, not a deploy phase, so a "deploy that reverts" read worse than a dedicated op. Easy to fold too if you'd prefer.

DESIGN.md updated to the single-public-op model; 34 deploy unit tests pass.

🤖 Automated update via Claude Code on behalf of @dawsontoth

Comment thread bin/cliOperations.ts Outdated
…ng a live symlink

Four findings from the second cross-model round.

Revert compensation was gated on the config commit having *resolved*. The config
transaction marks itself started before writing root config and can reject
between that write and the application-lock write, so a rejected commit may
already have changed persisted state — and that case skipped the rollback
entirely, restoring the old directories while config named the reverted-to
release, which a cold start would then reinstall over. Compensation now runs
after any commit *attempt*. If the rollback itself fails, the directories and the
recovery marker are deliberately left untouched instead of being undone and the
evidence consumed: persisted state may name the reverted-to release, so that is
the shape startup recovery rolls forward from.

Recovery required the live path to be a real directory, but a `file:` directory
deploy is materialized as a symlink by design. Such a component read as "neither
staged nor live", and the artifact sweep then deleted its `.previous-*` backup —
the only copy of the displaced release — while the component stayed failed
closed. A symlink whose target is a directory now counts as live, and a backup is
deleted only when the row and the live tree positively say it is residue.

`drop_component` removed the application-lock entry and the root-config entry as
two separate writes. A crash or a failed second write left root config naming the
package with the live directory already gone, so the next boot reinstalled the
component that had just been dropped. Both are one transaction now, config first.

Staged retention treated equal timestamps as evictable, so two concurrent stages
landing in the same millisecond let whichever pruned second expire the row the
other was about to return. Ties are protected on the row side, where
over-retention is cheap. The filesystem side keeps strictly-newer protection —
mtime granularity ties every build staged in the same tick, so protecting ties
there would stop the disk bound converging at all — with a deterministic
tiebreaker so concurrent prunes choose the same victims.

Also trimmed the comments the reviewer named: the 17-line coverage inventory in
the peer-branch integration test, and docs-duplicating narration in hdbTerms,
the recorder's status union, and deploy_component's JSDoc.

Reported by codex pre-push.
…roying anything

The two persistent writes were already one transaction, but it ran after
`dropComponentDirectory()` had removed the live tree — so a crash in between left
root config naming the package with nothing on disk, and the next boot's
installApplications() reinstalled the component that had just been dropped.

The root-config entry is the only thing that can resurrect a drop, so it is
removed first. That inverts the failure mode: an interrupted drop is now
unfinished and re-runnable — directory still present, no config entry — rather
than finished and then undone. Nothing between the transaction and the teardown
reads root config, and the component lock is held throughout, so no revert or
activation can observe the gap. A tombstone would also work, but the vector is
the config entry itself, so removing it first needs no new startup state.

Test injects a failing teardown and asserts config is already clean while the
tree survives.

Reported by codex pre-push.
… deleting a displaced release

Teaching activation reconciliation that a `file:` directory deploy is a symlink
left revert recovery behind, and the asymmetry is worse than the original bug.

`recoverInterruptedReverts` enumerated holding trees with `isDirectory()` alone.
A revert renames the live path into the holding slot, so for a directory-package
component that holding entry IS a symlink — skipped outright, on every pass. The
component stayed with no live tree and no restart ever recovered it. Both
existence probes in that function now use the same `liveComponentPresent` test
activation reconciliation uses, so a dangling symlink also stops counting as a
live component: it was steering recovery into the "both slots occupied, this is
residue" branch, which deleted the holding tree holding the only recoverable
bytes. The roll-forward gate accepts a symlinked retained previous for the same
reason, and the residue branch now takes the marker with the tree rather than
leaving crash evidence that no longer matches the disk.

A lingering activation backup for a SETTLED row was still deleted as residue. That
shape only occurs when `retainActivatedPrevious` failed inside its best-effort
catch after the swap and config commit — the deploy reported success, so the
parked tree is the sole remaining copy of what it displaced, and deleting it made
a successful deploy permanently unrevertable. Retention is finished instead.

Staged-row retention ran only on the `activate: false` return, while staged
directories are pruned on every stage. A full deploy therefore evicted trees whose
rows still read `staged` cluster-wide, advertising a deployment_id that a later
activate cannot use — and a different one per node under clock skew. It now runs
on every successful origin stage.

Reported by cursor-grok and cursor-composer pre-push. Their findings were produced
but not promoted by the harness (EEXIST on a stale artifact, and a format-repair
failure), so they were read from the leg output directly.
…int Windows junctions

Retaining the displaced bytes was only half the job. `retainActivatedPrevious`
removes the manifest when it fails, so recovery had nothing naming the release the
parked backup holds and wrote it back as an unknown deployment — the bytes
survived but `revert_component` had no target, leaving the deploy effectively
unrevertable anyway.

The failed-retain path now records the intended manifest when the retained slot is
empty. That stays safe: `getRevertTarget` requires a tree, so the component still
reads as not revertable until recovery moves the backup into place. When a stale
tree still occupies the slot the manifest is still removed, because naming the
displaced release against the wrong bytes is the hazard that rule exists for.
Recovery then reads whichever side of the manifest describes the displaced
release, discriminating on whether `live` is the deployment being recovered.

On Windows, `readlink` reports junction targets in the extended-length `\\?\C:\…`
form. Compared against a plain root, `relative` sees two different roots and
returns an absolute path, so the containment check rejected every junction and
skipped repointing exactly the links that dangle after the swap. The prefix is
stripped before comparing. This can only fail on win32, and the existing nested
dependency-link test already asserts the link resolves from the live tree, so the
Windows shard covers it — a POSIX test cannot reproduce the path semantics.

Payload retention patches the two fields it owns instead of writing the whole
record back, which would rewrite a status or error a concurrent deploy had just
set while reclaiming a tarball. The SSE poll timer is also cleared on the terminal
event rather than surviving one more interval past a settled request.

Reported by codex pre-push. Two findings in that round were false positives and
are not changed: `existsSync` IS imported in Application.ts (line 46, used three
places, and tsc is clean), and `deleteConfigFromFile` is a synchronous function
calling a non-async writer, so there is nothing to await at its call sites.
… retaining it

The settled-row retention added last commit was too permissive. A settled row plus
a good live tree proves the activation ended, not that it ended as the CURRENT
release — so an artifact left by an older deployment would be promoted into the
retained slot, overwriting valid rollback state with stale bytes and writing a
manifest naming a release that is no longer live. Retention now requires the
manifest to name that deployment as live, which is exactly what the failed-retain
path records. A backup that cannot be placed either way is kept: retaining it
would corrupt rollback state and deleting it may destroy the only copy of what its
deploy displaced.

Dependency-link compensation keyed on the returned count, so a repoint that threw
partway never assigned it and the inverse walk was skipped — leaving links aimed
at the live path, where a retry of the same deployment id validates the staged
tree against whatever release is live, and the containment check rejects those
links as external so they are never repointed back. It keys on "attempted" now,
the same correction the persistent-state compensation needed. No test: inducing a
failure mid-walk requires two links with a guaranteed processing order, and
readdir order is not guaranteed, so the test would be flaky rather than proving
anything.

Payload retention no longer appends to `event_log`. That was a read-copy-write of
an append-only list, so reclaiming a tarball could drop a concurrent writer's
audit entry; the null blob already reports the outcome and the reclaim is logged.
`writeRetainedPreviousManifest` delegates to `writeJsonAtomically` rather than
duplicating it, and the orphan sweep verifies the prefix before slicing it off.

Reported by codex pre-push. Two majors in that round are false positives and are
unchanged: `deleteConfigFromFile` is synchronous down to `writeFileSync`/
`renameSync`, so there is nothing to await (raised three rounds running), and the
SSE promise cannot hang on a completion during the initial row read — events are
buffered while `resolveLive` is null, and the executor resolves on `liveDone`
before arming the timer.
…coverable trees

Revert recovery inferred "did persistent state commit?" from whether the manifest
had been exchanged. Those are written in sequence, so a crash between them left
config exchanged and the manifest not — and recovery, comparing the two, undid
directories that config already described. The component came back running old
code under new configuration, which a cold start would then reinstall over. The
revert now records `persisted: true` on its recovery marker the moment the commit
returns, and recovery reads that; the manifest comparison remains as a fallback
for markers written before the field existed.

A displaced tree that reached the retained slot could still end up unaddressable:
if the manifest write failed after the rename, the previous fix declined to record
it because the slot was occupied — by the very tree it should have described. It
now distinguishes a slot holding the tree just moved from one holding something
stale.

A dangling symlink at the live path blocked restoring an activation backup: the
restore branch keyed on lstat, which sees the link, so a component pointing at
nothing was left that way with its recoverable backup sitting beside it. It keys
on usability now, and rename replaces the dead link.

The SSE terminal signal is no longer lost to the replay dedup filter. An event
whose timestamp ties the last replayed entry was dropped outright, discarding the
only indication the deployment had finished. Deduplicated events are still checked
for terminal-ness even when they are not re-emitted.

`discardRetainedPrevious` parked its tree in an aside derived from the retained
path — creating `.deploy-previous/.deploy-aside`, which startup recovery never
sweeps and which made the following rmdir fail ENOTEMPTY, stranding the tree
permanently. It parks in the component's own aside.

Staged-row settlement stays at stage time, where it matches the directory prune it
mirrors. Moving it after activation was tried and is worse: the current row is no
longer `staged` by then, so nothing is superseded and the row/directory divergence
returns. The residual — N concurrent deploys racing for N retention slots — is
inherent to a count-based policy and already true of the directory prune.

Two more majors this round are false positives and unchanged: `deleteConfigFromFile`
is synchronous down to `writeFileSync`/`renameSync` (fourth round raised), and
`withPersistentStateLock`'s liveness predicate is never consulted for a foreign
pid — `ownerIsAlive` resolves those with `isProcessAlive` first, so a SIGKILLed
external holder is detected dead.
#2066 landed on main as a squash, so its commits are not ancestors of main and
this branch's copies of them collided with main's squashed equivalent. Merged
rather than rebased: a linear replay conflicted on the first of 53 commits, and
each later commit would have replayed against a resolution it never saw. This
branch has merged main in 11 times already and the PR squash-merges, so linear
history buys nothing — and a merge rewrites no published history.

Conflicts of note, all resolved toward the newer intent:

- `bin/cliOperations.ts` — main added deploy-by-reference (`by_ref`/`ref`) and
  restructured the packaging prep. Took main's structure and grafted our verbs
  back on: the `deployment_id` early return for `harper activate`, the
  `revert_component` project default, and `verbRequirementError` in the exports.
  Uses main's `directoryProjectName` rather than our `path.basename`.
- `components/operationsValidation.js` — main moved the project/file name pattern
  into the shared `utility/componentNames.ts`. Kept main's shared import and our
  `DEPLOYMENT_ID_REGEX`.
- `unitTests/bin/cliOperations.test.js` — both sides added a suite; kept both.
- `Application.ts`, `componentLoader.ts`, `operations.js` — every hunk was our
  buildDirPath/extractionContext evolution against main's older
  `application.dirPath` form, with no new main content inside them.

`integrationTests/deploy/deploy-tracking-peer-branch.test.ts` is a real
divergence, not a textual one: main's peer tests drive the peer path by sending
`_deploymentId` on a PUBLIC deploy_component and expect 200, which this branch
forbids outright (`Joi.any().forbidden()`) and asserts 400 for. Those tests cannot
pass here, so ours stands and main's two peer-branch cases are dropped along with
the helpers and imports that served only them. That is a net loss of peer
end-to-end coverage relative to main and is called out in the PR description
rather than left implicit.

Also removed two duplicate declarations the merge produced (`rmdir` imported
twice; the drop-lock block, which both sides carried identically).
…s require tracking

The two remaining design decisions from the PR description.

**Peer row ownership.** Every peer ran `claimStagedDeployment`, which ends in a
patch on the replicated `hdb_deployment` row — N+1 writers of one key, the
concurrent same-key pattern `seal()` exists to avoid. Under replication lag a
peer's `activating` could land after the origin had written `success`, leaving a
converged deploy in a non-terminal status it never leaves. DESIGN.md and
`revertComponent` both already said the origin owns the row, so the implementation
contradicted the stated design.

Peers now claim with `persist: false`: same validation — project match, status
gate, wait-for-staged — no write. What a peer actually needs from claiming is
mutual exclusion against another activation of the same component, and it already
holds the per-component filesystem lock for that; the row write never provided it.
Three peer-phase tests asserted the old behavior and now assert the new contract.

**Separated phases require deployment tracking.** `DeploymentRecorder.put()` is
tolerant of a missing `hdb_deployment` table because tracking is observability for
a one-shot deploy. It is not observability for the separated phases, which
coordinate through the row: `activate: false` returned a deployment_id nothing
could resolve, so the stage reported success and was permanently unactivatable.

`activate: false`, activate-by-id, and an explicit `two_phase: true` now fail 503
when the table is absent — the request is valid, the node is not provisioned. An
unspecified `two_phase` falls back to the one-shot path instead of entering a
protocol with nowhere to coordinate, which keeps the tolerant behavior for a node
whose upgrade directive has not run. That is the product call the decision noted:
only an explicit request fails loudly.

The test harness's warmup deploy moved to the one-shot path, since it runs before
the table seam exists on purpose and the separated phases now require it.

DESIGN.md documents both, replacing the "peers make the same local claim" line
that the peer write had outgrown.
…back stranding peers

Two regressions from the previous commit, both caught in review.

**Peers lost their crash-recovery evidence.** With `persist: false` a peer swaps
while its local row still reads `staged` — the phase operation and the origin's
`activating` patch travel independently. A peer that died between the swap and its
config commit therefore had a `staged` row and no staged leaf, which
reconciliation read as a *broken candidate*: it settled the row `failed` (over the
origin-owned row) and never persisted config, leaving the peer running new code
under the previous release's configuration.

Local evidence now outranks the replicated status. An activation artifact for that
deployment is proof the swap began, so the entry goes down the roll-forward path
instead of the discard path. This is the node-local evidence the review asked for
and it already existed on disk — recovery was simply keying on the row instead.

**The untracked one-shot fallback was not cluster-safe.** Routing a default deploy
to one-shot when `hdb_deployment` is missing looked tolerant but was the unsafe
choice: that path consumes the multipart stream into its own blob and strips
`req.payload` before replication, so peers receive neither replayable bytes nor a
row — and it activates locally *before* replicating, so the origin goes live alone.

The fallback is gone. The default path is left exactly as it was, because it
already fails safely: peers cannot find the row, the barrier never clears, and
nothing activates. Only the requests that coordinate through the row directly —
`activate: false`, activate-by-id, explicit `two_phase: true` — fail 503, and
`two_phase: false` remains the explicit single-phase escape hatch. My earlier
framing of this as a "tolerant default" was wrong in the unsafe direction.

Also from the same review: the 503 tests assert the status code rather than only
the message, a test covers the `two_phase: false` escape hatch still working
untracked, and DESIGN.md's merge residue describing the rejected `_phase`-tagged
wire protocol is replaced with the actual trusted `component_deploy_phase` +
AsyncLocalStorage contract (it would have had a maintainer sending a request the
server rejects). The duplicated JSDoc on `isDeploymentTrackingAvailable` is gone.

Reported by codex, cursor-grok and the domain leg pre-push.
…fy peer evidence under the lock

Three majors, all consequences of the previous commit.

**`two_phase: false` was not the safe escape hatch I documented it as.** The
one-shot path strips `req.payload` before replicating because peers normally read
the bytes from the row's `payload_blob` — but the condition was
`systemReplicated && recorder`, and `recorder` is truthy with no table (its writes
just no-op). So on an untracked node peers received a `_deploymentId`, no bytes,
and no row to resolve, after this node was already live. Stripping is now gated on
tracking actually being available; with no row to read from, the payload rides
along in the replicated operation, which is what one-shot did before deployment
tracking existed. This matters more than a normal bug because the 503 message and
DESIGN.md both point operators at this path.

The test for it now asserts on the REPLICATED REQUEST rather than local
activation. Replication is stubbed in that harness, so a local-only assertion
proved nothing about peers — and indeed the first version of this test passed
against the unfixed code. It fails against it now.

**The peer-evidence probe was sampled outside the component lock**, so it did not
close the race it was added for: an activation could create its backup and rename
the staged tree live between the probe and the lock, after which reconciliation
patched the origin-owned row `failed` and deleted a live activation's deployment
directory. Both signals are re-read under the lock now, and an activation that
appears there routes to roll-forward instead of being settled.

**And that new roll-forward path did not fail closed.** Attribution keyed only on
`row.status === 'activating'`, so a rejecting persistence step on a
`staged`-row-plus-artifact entry recorded an error and still let the swapped
candidate load under the previous release's configuration. `activationBegan` now
counts as equivalent durable evidence in the catch.

Reported by codex, gemini, cursor-grok, cursor-composer and the domain leg.
…lready drained

My previous fix for the `two_phase: false` escape hatch was incomplete, and its
test hid that. Stopping the payload from being stripped is not enough: ingest
DRAINS the source, so `req.payload` is an exhausted `Readable` by the time
replication happens. Peers received an EOF after this node was already live — the
same split the fix was meant to close.

The degraded (no-table) ingest path already buffers the whole upload in memory,
which is the only replayable copy that exists. The recorder now exposes it and the
one-shot path substitutes it for the spent source before replicating, so the bytes
actually travel in the operation.

The test is the more important half. It previously used a reusable `Buffer` and
asserted only that `payload` was not undefined — so it passed against the broken
behavior, because a spent stream is still a defined property. It now drives the
deploy with a real `Readable` and compares the replicated bytes to what was
uploaded. Reverting to the previous fix fails it on exactly that assertion.

Reported by codex, gemini, cursor-grok, cursor-composer and the domain leg.
…node staged deploy

Splits this PR. The cluster-wide protocol is the part that unit tests cannot
validate — every escaped defect in the last four review rounds traced back to it,
and two of my own tests passed against broken behavior because the harness cannot
observe peers at all. It moves to its own PR, to land with real multi-node
verification. Preserved on `claude/deploy-peer-protocol-pr-b`.

What stays is everything a single node can prove:

- `deploy_component` builds into `.deploy-staging/<id>/<project>`, validates that
  the staged tree loads, then atomically renames it live — committing root config
  and `harper-application-lock.json` in the same compensating transaction. The
  live component keeps serving through the install; a fetch or install failure
  leaves it untouched rather than half-replaced in place.
- `revert_component`: addressed, idempotent, config-level, with retained-previous
  manifests.
- Startup reconciliation for interrupted extractions, reverts and activations.
- Staged-build and payload retention.
- `harper revert` on the CLI.

Removed: `component_deploy_phase` (operation, handler, validator, registration,
authorization), the stage/activate fan-out and barrier, `deployComponentTwoPhase`,
`deployComponentActivateExisting`, peer row claims, and the `activate: false` /
`deployment_id` / `two_phase` public surface with its CLI verbs and capability
probe. The request/response contract is now identical to before this PR.

Two things the deletion surfaced that are worth naming:

- `assertNotProtectedCoreComponent` lived inside the root-config write I removed,
  so protected core names were briefly overwritable without `force`. Caught by an
  existing test, restored explicitly in the deploy path, package deploys only, as
  before.
- `activation_spec` was no longer recorded on the row, and startup reconciliation
  reads it to reconcile config after an interrupted activation — a row without it
  cannot be recovered. It is now written before the build starts.

BREAKING CHANGE: none against released Harper. `activate: false`, `deployment_id`,
`two_phase` and `component_deploy_phase` were introduced by this PR and never
shipped, so removing them restores the released contract rather than changing it.
dawsontoth added a commit that referenced this pull request Aug 24, 2026
…r phase operation

Restores the peer coordination protocol that #1849 carried before it was split, so
it can be reviewed on its own and land with real multi-node verification. This is
the half unit tests cannot validate: the harness mocks `replicateOperation` and
enters the authorization bypass directly, so it cannot observe a peer at all.

On top of #1849's per-node staged deploy, this adds:

- `component_deploy_phase`, a trusted peer-only operation carrying the phase and
  deployment id. Authorization travels in AsyncLocalStorage rather than on the
  request, so it is unreachable over HTTP with ordinary credentials, and an older
  peer rejects an unknown operation instead of misreading a phase marker as a
  one-shot deploy.
- The cluster-wide barrier: the origin stages everywhere and waits for every node
  to report a good stage before any node activates. A node that cannot fetch or
  install fails during staging while the live component is untouched everywhere.
- The separated public phases — `activate: false` returns a staged deployment_id,
  `deployment_id` activates it later, `two_phase: false` forces single-phase — plus
  the `harper stage` / `harper activate` CLI verbs and the capability probe that
  stops a staged request reaching a server that would silently deploy it live.
- The deployment row as the peer channel: the immutable activation specification,
  the payload blob peers read their bytes from, and staged-row retention.
- Origin-owned row semantics: peers claim with `persist: false` so the replicated
  row has exactly one writer, with local activation artifacts as their crash
  evidence.

Known open work, carried from the review rounds on #1849 rather than hidden:

- No in-repo end-to-end coverage of the trusted dispatch path. This is the gap
  that motivated the split; the three-node harper-pro suite has to run against
  this revision before it merges.
- #2294 — nothing orders two concurrently-originated deploys, so both can report
  success with different versions live. The barrier orders stage-before-activate,
  not deploy-against-deploy.
- #2295 — a `package:` deploy resolves per node, so the barrier guarantees
  "everyone staged something", not "everyone staged the same bytes".

Stacked on #1849; the diff here is exactly the coordination protocol.
Review of the shrunk PR found the leftovers, which is what a review of a large
deletion is for.

The handler still branched on `activate`, `two_phase` and `deployment_id` after
the validator dropped them — and the schema allows unknown keys, so a caller
still sending the never-released staged contract would have had `activate: false`
silently ignored and received a full deploy instead. Those branches are gone and
the fields are now `forbidden()`, naming #2301, so they fail fast rather than
doing the opposite of what was asked. The first test to hit that was one of ours
that had been passing `activate: false` as a no-op.

`applicationConfigFromActivationSpec` dereferenced `spec.package` without a null
guard, and startup reconciliation feeds it `row.activation_spec` from rows it did
not write — a row from before the spec was recorded, or a hand-edited one. That
threw during boot and left the component failed closed on every restart with no
recovery but repairing the row by hand. A missing spec now degrades to a config
no-op.

`drop_component` settled only `staged`/`activating` rows, but a deploy interrupted
mid-stage rests at `staging`, and nothing else settles it: payload retention only
reclaims terminal rows, so the tarball stayed pinned and `get_deployment` never
converged for an already-dropped component.

Docs and comments: I had deleted the caveat that the pre-swap load check is a
no-op on the main thread — which is where the operations API runs deploys — so
DESIGN.md was claiming a guarantee the code does not provide on the origin. It is
restored and scoped explicitly. Also removed the activate-by-id consequence
paragraph, and corrected the comments still describing two-phase orchestration,
the `component_deploy_phase` fan-out, and `harper activate`.

`claimStagedDeployment` and `settleStagedRows` have no caller in this tree now.
They are marked RESERVED FOR #2301 rather than deleted: that PR is stacked
directly on this one and is their only consumer, so deleting here would just mean
re-adding there. The comments say plainly that no resting `staged` row producer
exists in-tree until it lands.

Reported by cursor-composer pre-push.
…alidations sharing a global

Two majors from the graded review of the shrunk PR.

**Startup destroyed activation evidence on the strength of the row.** The status
filter ran before the local evidence check, and the row is the less reliable of
the two: a crash between the swap and the status/config commit can leave it
`loading`, already terminal from a replicated origin write, or absent entirely
when tracking is unavailable — while the candidate is live on disk. Reconciliation
then removed the staging parent and moved on, and the artifact sweep neither
persisted config nor failed the component closed, so swapped-in code loaded under
the previous release's configuration.

Activation artifacts are now read under the component lock BEFORE any status-based
cleanup, and their presence forces the roll-forward path for any row status. The
split made this materially more likely, not less: every deploy here ends as
`success`, so "not staged or activating" is the common startup case rather than an
edge one.

**Concurrent validations shared a process-global error reporter.** `setErrorReporter`
is module state, so two deploys validating on the same worker cross-attribute
failures: B installs its reporter while A is loading, A's load error lands in B —
A then activates broken bytes while B rejects a good candidate. Validation is
serialized (already the slow path) and the previous reporter is restored in a
`finally`, so the global is only ever owned by one in-flight validation.
`componentLoader` grew a matching getter so the restore is possible at all.

Also from the same review: the protected-core-name rejection now runs before
credential ingestion and the durable row are created, rather than after — a
deploy that was never allowed should not leave a secret in the store and a row
behind.

Reported by codex pre-push.
…ettle rows we own

Three minors from the same review.

`revert_component` rejected an unsatisfiable target — nothing retained, or a
deployment id that is neither live nor the retained previous — with a bare
`Error`, so the operations catch defaulted it to HTTP 500. Both are the caller
asking for something that does not exist, not a node failure; they are 409s now,
and the existing tests assert the code rather than only the message.

A recovered activation logged that the deployment "remains activating until
cluster state is reconciled" — a claim inherited from the peer protocol that no
longer reconciles anything here. The node that ORIGINATED the deployment (the
only node that has a row) now settles it `success` on roll-forward, since the
activation it started is complete; a peer still leaves the origin's row alone
rather than reporting on nodes it cannot see, and the message says which happened.
Settling is observability, so a failure to write the row is logged, never allowed
to keep a live reconciled component from loading.

Staged-build retention can still evict a candidate that a concurrent deploy owns,
above `maxCount` simultaneous deploys of one component — more reachable now that
validation serializes after the component lock is released. Left as-is and
documented: tracking ownership would let a deploy that dies between stage and
activate pin a tree forever, defeating the disk bound the prune exists to enforce.
A loud "no valid component tree" on the 6th concurrent deploy of the same
component is the better failure.

Reported by codex pre-push.
…ecovery armed

Two release-blocking findings from the graded review, plus the fail-open behind
one of them.

**A dropped component could come back served on every host.** Applications in the
components root load by DIRECTORY SCAN; the root-config entry is what constrains
where one is served (`host`/`urlPath`), not what makes it load. This branch had
moved the config deletion to the front of `drop_component`, reasoning that the
entry is the only resurrection vector. It is not: with the entry gone and the tree
still on disk, a crash mid-teardown leaves the component discoverable with no mount
at all, so the next boot serves it unconstrained — silently dropping the isolation
the operator configured, which `tryRootConfigMount` already treats as worse than
not loading at all. The tree is parked first now and the entry removed only after,
which restores `main`'s ordering and leaves the merely-re-runnable partial state
instead: tree gone, entry present.

The test covering this asserted the dangerous state and called it correct — it
required config-clean-with-tree-present. It now asserts the invariant that matters:
a discoverable tree must never outlive its mount. Mutation-verified against the
old ordering.

**Startup recovery disarmed itself after the first clean pass.** A reload cycle
runs `loadComponentDirectories` again long after boot, and an activation that fails
at runtime AND fails to compensate leaves exactly the inconsistent staged/live/
backup state the reconciliation pass exists to settle. The one-shot guard meant
only a cold restart repaired it; until then a hot reload could load the candidate
against old or partial configuration. It runs every main-thread cycle now — the
pass is idempotent, and once settled it is a readdir of a directory it empties.

**A manifest read error was indistinguishable from no manifest.** All three call
sites wrapped the reader in `.catch(() => undefined)`, but the reader already maps
ENOENT to undefined, so the catch only ever swallowed real failures — EACCES, EIO,
corrupt JSON. Each then recorded the displaced release as `deployment_id: null`:
retention overwrites an addressable previous version with an unaddressable one, and
the reconciliation probe deletes the last copy of a displaced release as residue.
All three now propagate, which on the deploy path fails before the swap, so the
live component keeps serving.

Reported by codex pre-push.
…the split took

`claimStagedDeployment`, `settleStagedRows`, and `expireOldStagedDeployments` have
no caller in this tree — they exist only for the stacked coordination PR, and were
carried here behind RESERVED comments to save re-adding them there. That is exactly
backwards for a PR whose point was to shrink to what this tree can verify: dead code
reviewed here is dead code nobody can exercise here. They land with their caller,
along with the ten unit tests that were their only consumer.

Removing the staged-deploy capability probe also deleted the whole
`deploy_component cross-version compatibility` block, which covered more than the
probe: the pre-5.1 package-JSON downgrade, the pre-5.1 directory/CBOR downgrade,
the 5.1+ streaming path, and a failed version probe assuming modern. Those branches
are still live in `cliOperations`, so a request-format change could have silently
broken deploying to an older Harper with nothing to catch it. Restored, and checked
against a mutation that disables the downgrade.

DESIGN.md claimed two guarantees this branch does not provide: that every staged
deploy validates the candidate loads (it is a no-op on the main thread, which is
where the operations API deploys — the caveat was already documented 300 lines
later, contradicting it), and that automatic payload pruning appends a
`payload_dropped` event to the rows it prunes (it deliberately does not — `event_log`
is append-only and a read-copy-write would lose a concurrent writer's entry). Both
now describe what actually happens.

Also drops comments that narrated code or restated a contract validation now
forbids.

Reported by codex pre-push.
@dawsontoth

Copy link
Copy Markdown
Contributor Author

Reference only — this PR will not merge

We're re-planning this work as a sequence of small, independently mergeable PRs. #2315 is the coordination issue; the steps and their ordering live there.

This branch stays open (or available after closing) as the reference implementation, and #2301 does the same for the cluster-coordination half that was split out of it. Nothing here is being thrown away — several rounds of cross-model review and a lot of carefully constructed crash-path tests are worth lifting into the smaller PRs rather than rewriting.

Why

This PR reached 26 files, +6,390/−551 across 108 commits over 5.5 weeks, with ~21,200 lines of total churn to produce that diff — a 3× rework ratio. components/Application.ts alone went 2,688 → 4,620 lines (+72%), nearly all crash-recovery paths, and tests are 47% of the diff.

Splitting the coordination protocol out into #2301 was meant to fix that and only removed 16% of the additions and one file (7,811 → 6,529 insertions). The split was along the right axis — the unit harness mocks replication, so the peer protocol could not be verified here at all — but on a wrong assumption: that the unverifiable half was also the large half. It wasn't. The bulk was single-node crash safety, which the split kept.

What the smaller steps change

Two things came out of measuring it that make the sequence genuinely different rather than just smaller:

Reviewers: no action needed here. Follow #2315.

@dawsontoth

Copy link
Copy Markdown
Contributor Author

Closing as reference-only — see #2315 for the sequence of small PRs that replaces this work.

The branch is not deleted: its diff and history stay browsable here, and the individual steps in #2315 will lift already-reviewed code from it.

@dawsontoth dawsontoth closed this Aug 25, 2026
kriszyp pushed a commit that referenced this pull request Sep 2, 2026
…wap (#2345)

* refactor(deploy): separate tarball resolution and extraction from the aside protocol

Groundwork for building a deploy candidate off to the side (#2315 step 1). No
behavior change.

`extractApplication` mixed three jobs: turning a payload/package/git-reference
into a tarball stream, extracting that stream into a directory, and running the
live-tree aside protocol that makes the replacement atomic. Only the third is
about the live tree, so the first two are now `resolveApplicationTarball` and
`extractTarballInto(tarball, target, scratch)`, with `extractApplication` built
on them. A candidate build can then reuse resolution and extraction without
going anywhere near the aside.

Also adds per-deployment candidate paths under `.deploy-staging`. The rest of the
durability primitives (the complete marker, the activation journal and its
fail-closed reader) land with the code that consumes them rather than sitting
here unused.

Verified: the flatten logic that moved into `extractTarballInto` is
mutation-covered — disabling it fails an existing extraction test.

* refactor(deploy): let installApplication install a tree that is not the live one

Groundwork for #2315 step 1. Default argument, so every existing caller behaves
exactly as before.

`installApplication` derived everything from `application.dirPath`: the
package.json read, the node_modules probe, and the cwd of all three install
spawns (custom command, configured package manager, npm fallback). It now takes
an explicit `buildDirPath` defaulting to `application.dirPath`.

Explicit rather than temporarily repointing `application.dirPath`, which is the
approach the abandoned #1849 branch used: that property is read after preparation
too, so any failure path that skipped the restore left it naming a staging
directory that no longer existed.

Three spawn sites, not two — the npm fallback at the end of the function is easy
to miss when reading only as far as the configured-package-manager branch.

* feat(deploy): build the replacement component off to the side

#2315 step 1, first behavioral piece. `buildCandidateApplication` extracts and
installs into `.deploy-staging/<deploymentId>/<component>` and leaves the live
tree completely alone, so the previous version keeps serving through the clone,
the extraction and the dependency install — minutes, for a git reference or a
large dependency tree. Nothing calls it yet; wiring `deploy_component` to it is
the next commit.

Failure needs no compensation, which is the point of building elsewhere: the live
component was never modified, so an abandoned candidate is just removed. Contrast
the in-place path, where the same failure has to rename the aside back over a
half-written live directory.

`ensureExtractionStagingDirectory`'s containment check is now a reusable
`ensureSecureStagingDirectory` and runs for each candidate ancestor: real
directory, not a symlink or junction swapped in underneath, owner-only. Applied
per use rather than once per deploy, since the gap between checking and writing
is the exploitable part.

Tests: the live tree is byte-identical after a successful build; a failed build
leaves neither a candidate nor a mark on the live tree; a retry on the same
deployment id replaces the earlier attempt instead of extracting on top of it;
and a symlinked staging root is refused with nothing written outside the
components root. Mutation-verified — building into the live path instead fails
two of them.

* feat(deploy): activate a validated candidate as one recoverable transaction

#2315 step 1. Still unwired — `deploy_component` switches over in the commit that
also makes config publication transactional, so no intermediate commit changes
behavior.

`activateCandidateApplication` makes a built, validated candidate live as one
compensating transaction over three effects: the live tree moves aside, the
candidate takes its place, and the component's root-config entry is published.
Config goes LAST, because an entry that outlives the tree it names is exactly how
a rejected release comes back at the next restart.

`recoverInterruptedActivations` settles whatever a crash left, before anything
loads, and returns failures keyed by COMPONENT so the caller can fail exactly
those closed and still load every healthy sibling.

The state machine, which the planning review rightly refused to accept as "reuse
the existing recovery":

  * The journal is consulted FIRST. Its absence means no new-style activation was
    in flight, so the legacy in-place recovery applies unchanged — a crash in the
    old path also leaves an in-progress aside with the live tree present, and
    "retire the aside" there would keep a half-written tree instead of restoring
    the good one.
  * Ambiguity exists only while the live path is absent, and there the `complete`
    marker is the roll-forward authority: no marker means the candidate was never
    validated, so the committed tree in the aside wins.
  * Live present + candidate present is pre-swap or already-rolled-back: discard
    the candidate, put the old config entry back.
  * Live present + candidate gone is a lost tail: finish forward, never revert a
    completed activation.
  * Neither a live tree nor a rollback record is unrecoverable, so the component
    fails closed rather than guessing.

Every branch is idempotent, so a crash *during* recovery is settled by the next
run. Directory fsync stays best-effort (Node cannot fsync a directory on Windows)
because the protocol never depends on it: a lost directory update degrades to a
roll back, never to a wrong decision.

A truncated or unknown-version journal fails its component closed and keeps the
evidence rather than guessing a direction — both guesses are destructive.

Tests cover every row of the matrix plus per-component isolation (an unreadable
journal does not stop a healthy sibling settling), and on the activation side:
the happy path clears its own records, a config-publish failure puts the previous
version back and leaves the candidate retryable, and a first-ever deploy with no
tree to move aside.

* feat(deploy): validate the replacement before it goes live, and publish config with it

#2315 step 1, wired up. `deploy_component` now builds the replacement off to the
side, validates it, and only then swaps it in and publishes its configuration.

Three defects on `main`, one cause — the build workspace, the serving path and the
commit boundary were the same object, so an uncommitted candidate was published
merely by being built:

  1. `Application.ts:759` renamed the LIVE tree aside before the replacement
     existed, so the component was broken for the whole extract + `npm install` —
     minutes for a git reference or a large dependency tree.
  2. `operations.js` awaited `prepareApplication` (which committed the swap) and
     only THEN ran the load validation, so a component that installs cleanly but
     throws at load went live anyway while the operation returned an error.
  3. `operations.js:490` published the root config BEFORE the build and never
     rolled it back, so `installApplications()` reinstalled a rejected release at
     the next restart.

`prepareApplication` now spans build → validate → activate under the existing
preparation lock, and takes the config effect so the entry is published by the
activation transaction rather than ahead of it. Phase events keep their existing
order for clients (prepare start/done, then load start/done) even though the load
now happens inside the preparation window.

A payload deploy replacing a package-installed component explicitly REMOVES the
`package` entry rather than leaving no opinion. Otherwise a cold install — a fresh
peer, a wiped components directory — resolves the old package over the payload
release.

Config publication is serialized across components, because `addConfig` is a
read-modify-write of the whole document and two components publishing at once can
lose each other's entry. Keyed on a pseudo-path no real component can collide
with, since component names are never dot-prefixed.

Startup settles interrupted activations before anything loads, per component, so
one unsettleable component is failed closed while healthy siblings still load.

Fixed along the way, both found by existing tests rather than by inspection:

  * Retiring the rollback record only MARKS the displaced tree disposable. Without
    a sweep, `.deploy-aside/<component>` kept the tree from every deploy forever —
    the components root would grow by a whole component version per deploy. Both
    the activation path and recovery now sweep.
  * `deleteConfigFromFile` was annotated `param: string` while every caller passes
    a YAML path array; it only compiled because no TypeScript caller existed.

Three tests changed because their mechanism depended on the old ordering, not
because they were wrong:

  * The AggregateError test sabotaged the rollback record from inside the install
    script. That is now structurally impossible — no aside exists during install —
    so it asserts the stronger property instead: an install script cannot reach or
    corrupt the live tree, and the failure is a single error. The aggregate path
    still exists for activation failures and is covered in deployActivation.
  * The serialization test polled the live `package.json` while the first install
    ran; nothing is published there until activation, so it now tolerates the path
    being absent.
  * Three stubbed-config assertions in operations.test.js stub `prepareApplication`,
    which is where config is published now, so they assert on the entry handed to
    preparation instead of on a write beyond the stub.

* test(deploy): prove the previous version stays in place through a blocked install

#2315 step 1. End-to-end through the real operations API, plus the DESIGN.md
section for the new ordering.

The test holds a deploy open mid-install (the install script parks on a release
file, which is what makes the window samplable at all) and samples the live
component directory throughout. Every sample must be the previous release, byte
for byte.

Verified against `main`: it fails there with `saw ["2"]`. That is a sharper
statement of the defect than "the component is unavailable" — during a deploy the
live path already held the NEW release while its dependencies were still
installing, so requests hit an unrunnable tree rather than nothing.

Also asserts the tail: staging is empty afterwards and the displaced version is
swept, so neither accumulates per deploy.

* fix(deploy): settle activations before the legacy pass, and stop boot recovery deleting config

Graded pre-push review returned BLOCK. These are its high-severity findings.

**Recovery ran in the wrong order — the adjudicated blocker.** A crash between
candidate→live and the config write leaves live=new, an in-progress aside holding
the old tree, and a journal. The legacy `.deploy-aside` pass keys only on that
aside, so running it first restored the OLD tree over the new one; the activation
pass then saw live-present-with-no-candidate, finished forward, and published the
NEW config. Deterministically: old tree running under the new release's
configuration — the exact divergence this work exists to prevent. Activation
recovery now runs first, so the legacy pass only ever sees asides no activation
owns. My own DESIGN.md already said "the journal is consulted first"; the code
did not.

**Boot recovery could delete authoritative config.** Boot re-installs pass no
config effect, which journaled identically to an explicit "remove this entry" —
and recovery always published the journaled value. A crash during a boot reinstall
would therefore delete the config entry boot was installing FROM. The journal now
records `publishesConfig`, recovery skips config entirely when the activation never
owned one, and a journal missing the flag fails its component closed.

**`file:<directory>` deploys were broken by the extraction refactor.** The resolver
still symlinked the source straight onto the LIVE path and returned nothing, so
`buildCandidateApplication` destructured `undefined` and threw — and the link
bypassed the candidate, publishing without validation. Resolution now reports the
link instead of performing it: `extractApplication` keeps its old in-place
behavior, while a candidate build links at the candidate path and is activated like
any other. Activation accepts a symlinked candidate.

**Concurrent validations shared the process-global error reporter.** Two components
validating at once cross-attribute failures — A's load error lands in B, so A
activates broken bytes while B rejects a good candidate. Serialized, with the
previous reporter restored in a `finally`.

**A rejected candidate leaked its dependency tree.** The builder's cleanup only
covered a failed build, so a validation rejection or compensated activation left a
whole installed tree under its deployment id; repeated rejections fill the volume.

**The journal was removed even when retiring the rollback record failed** — which
strips the evidence that the activation completed, so the legacy pass would restore
the old tree at the next start. Journal removal is now gated on the record being
settled.

Two claims corrected rather than defended: `prepare:done` no longer means the swap
committed (the order is preserved, the meaning is not), and the availability test's
header no longer claims request-level or rejected-deploy config coverage it does not
have — it now says exactly what it samples, and additionally asserts a payload
deploy strips a stale `package` entry.

Reported by codex pre-push.

* fix(deploy): snapshot config under the lock, and sync both parents of every rename

Two of the four findings I had carried openly on #2345.

**`configBefore` was a stale snapshot.** It came from `getConfigObj()`, a memoized
boot object that `addConfig` does not refresh, and it was captured in the request
before preparation took the component lock. Two queued deploys of one component
therefore both recorded the entry that existed before EITHER ran, so the second
one's rollback would restore a predecessor that had already been replaced. It is
now a reader (`readConfigEntryFromFile`, a fresh YAML read) invoked at journal time
under the component preparation lock, so a queued deploy records what the deploy
ahead of it actually committed.

**Only one parent of each rename was synced.** A rename mutates an entry in both
parents — the removal in the source's, the addition in the destination's — so
syncing only the destination can leave the source entry present after power loss.
That reads as "the candidate is still there" and rolls an already-completed
activation forward a second time. `syncRenameParents` now covers both sides at
every boundary: B1, B2, the B1 compensation, and both recovery renames.

`syncDirectory` also no longer swallows silently. It traces instead, and says why
the failure is expected rather than a fault: Windows cannot open a directory for
fsync at all, which is exactly why the protocol is built to tolerate a lost
directory update — roll-forward requires the journal, the candidate and its
complete marker to all be observable, so a lost entry degrades to a roll back.

Reported by codex pre-push.

* fix(deploy): settle activations before installApplications, and make the durability real

Round-three review findings.

**Recovery still ran too late.** The previous fix reordered the two passes inside
`loadComponentDirectories`, but boot calls `installApplications()` FIRST
(`server/loadRootComponents.js`). That installs from the root config, so a crash
that left a journal with its config entry unpublished had boot reinstall the
PREVIOUS release over an already-live candidate, and the later recovery pass then
published the new config against the reinstalled old tree. Activation recovery now
runs before `installApplications()`, and hands its per-component failures to the
loader so those components still fail closed.

**The candidate's own contents were never fsynced.** Only the small control files
were, so a power loss could keep `.complete` and the journal while the tree they
vouch for was truncated — and `.complete` is roll-forward AUTHORITY, so recovery
would roll forward onto it. Contents are now synced before the marker is written.

**`activation.json` could exist empty.** Opening the final path with `wx`
publishes the directory entry before anything is written, so a crash in between
left a zero-byte journal — which reads as "unreadable" and fails a component closed
over a deploy that had not really started. Contents go to a temp name, are fsynced,
then linked into place, preserving the EEXIST-means-retry semantics.

**`deleteConfigFromFile` wrote to the wrong file.** It parsed `getConfigFilePath()`
but wrote a path reconstructed from `rootPath`, which is a different file on layouts
where the config does not sit at the root: the deletion was lost and an unrelated
file was overwritten with this document. It now writes where it read, matching
`addConfig`. Pre-existing, but the new payload-config removal is the first caller
for which it matters.

**My package-strip assertion was unfalsifiable.** Both deploys in the availability
test are payloads, so the component never had a `package` entry — and it read
`harperdb-config.yaml`, which does not exist, behind a `.catch(() => '')`. Two
independent reasons it could never fail. Replaced with a test that seeds a
`package:` entry first and then payload-deploys over it; mutation-verified by
leaving the key in place.

* refactor(deploy)!: take config publication out of the activation transaction

Four review rounds landed 20-odd findings on this branch. The candidate
build/validate/swap half survived all four; every one of round 4's six majors was
in config publication, durability, or locking. That is a shape problem, not a
backlog, so config comes out and lands as its own step in #2315.

Config publication returns to exactly where `main` has it: written before the
build, not rolled back. The known consequence is unchanged from today — a rejected
deploy can still be reinstalled at the next restart. That is one defect instead of
the six that came with making it transactional, and it keeps this PR reviewable in
a sitting, which was the premise of the split.

What stays, because none of it is about config: the candidate is built and
validated at `.deploy-staging/<deploymentId>/<component>` while the previous
version keeps serving; validation runs BEFORE the swap, so a candidate that
installs cleanly but throws at load never goes live; the swap is journaled and
recovered at every boundary; recovery runs before `installApplications()`; the
journal and `.complete` marker are written temp-fsync-link so they can never
appear partial; the candidate's contents are fsynced before `.complete` vouches for
them; both parents of every rename are synced; and the displaced tree is swept
rather than accumulating per deploy.

The activation transaction is now two effects rather than three, the journal
carries no config, and `config/configUtils.ts` is untouched — so the earlier
`deleteConfigFromFile` path fix and its annotation correction move to the config
step too, where they belong with a caller that exercises them.

* test(deploy): drop an aside probe that could not fail

Both PR bots flagged it, and they are right twice over. The probe resolved
`'..', '..'` from the install script's cwd — which is now the candidate directory,
`.deploy-staging/<id>/<component>` — so it landed on
`.deploy-staging/.deploy-aside/<component>` and `readdirSync` always threw. And
correcting the depth would not have helped: in the build-aside design no aside
exists at build time at any depth, so the probe could never observe the thing it
claimed to check.

Removed rather than repaired. The assertions that carry this test are the
single-error rejection (no AggregateError, because there is no restore to also
fail) and the untouched live tree.

Third unfalsifiable assertion of mine on this branch, all the same shape: a check
whose subject cannot exist in the state being tested.

* fix(deploy): propagate sync failures, fail closed on a global recovery failure, stop leaking candidates

Round 5 findings — first review to see the split tree, and the first with three
outside lenses (codex, cursor-grok, domain).

**DESIGN.md still claimed config was transactional.** It described three effects
and a serialized publication lock that this PR no longer has. Worth saying how it
survived: my earlier edit script asserted three replacements, failed on the third,
and — since it only writes at the end — discarded all three. I then re-ran just the
failing subset and never noticed the other two were lost. Docs claiming a guarantee
the code does not provide is the exact failure this branch keeps producing.

**`syncTreeContents` swallowed real failures.** It turned any `readdir` error into
an empty directory and ignored open/sync errors, then `.complete` was written as
recovery's roll-forward authority. EIO or ENOSPC would have produced a marker
vouching for a tree that never reached storage. Failures now propagate, which is
safe precisely because the live tree has not been touched at that point.

**A global recovery failure loaded every component anyway.** If the staging root
cannot be read, WHICH components are unsettled is unknown, so loading them all
defeats the fail-closed contract the pass exists for. Boot now fails every present
component closed and lets a later reload cycle retry.

**The validation load leaked one entry per deploy, forever.** `componentLoader`'s
`loadedPaths` is keyed by realpath and never pruned, and every deploy validates a
candidate under a fresh `.deploy-staging/<uuid>/` path. The throwaway load now
forgets its path.

Also: the aside's parent is synced before the journal is removed (if the journal's
deletion persists and the record's does not, the legacy pass restores the old tree
over the new one), and `syncDirectory`'s `handle.close()` can no longer reject out
of a best-effort path that sits outside any compensation block.

Reported by codex + cursor-grok pre-push.

* fix(deploy): keep the staging root out of get_components

Six integration shards failed across Linux, Windows and uWS — a real regression,
not the known Windows flake. `get_components` was reporting `.deploy-staging` as a
component:

    {"name":".deploy-staging","entries":[],"status":{"status":"unknown", ...}}

It filters the components root with a DENY-LIST naming Harper's own bookkeeping
directories (`node_modules`, `.deploy-aside`, the preparation locks) rather than
skipping dot-prefixed entries — deliberately, because component contents include
dot-files like `.aiignore` and `.env.example` that callers expect to see. The new
staging root simply was not on the list. `.deploy-aside` escaped notice only
because it is removed once empty.

Two fixes, either of which is sufficient, both correct:

  * `.deploy-staging` joins the deny-list, so it can never be reported even while a
    deploy is in flight.
  * The staging root is removed once no candidate is left in it, so an idle
    instance's components root looks exactly as it did before.

Reproduced locally with both reverted: the same two `get_components` assertions
fail with `.deploy-staging` in the tree, matching CI. Restoring either one fixes
it; the whole `components.test.mjs` suite is 25/25.

* fix(deploy): do not fail a deploy because the platform will not fsync

Two CI regressions, both mine.

**Every deploy failed on Windows** with `EPERM: operation not permitted, fsync`.
The round-5 review was right that `syncTreeContents` must not swallow durability
failures, but propagating ALL of them was equally wrong: Windows raises EPERM
fsyncing perfectly healthy files, and network/overlay mounts return EINVAL or
ENOTSUP. None of those say anything about whether the write reached storage.

Those codes are now traced and tolerated — the same bargain `syncDirectory`
already makes for directories, and for the same reason. EIO, ENOSPC and anything
else still propagate, which is what keeps `.complete` from vouching for a tree
that never landed.

**`deploy-tracking-peer-branch` waited for the previous tree to be moved aside.**
It polled `.deploy-aside/<project>` during a failing peer deploy, because
extraction used to replace the live path in place and a payload failure was a
rollback. The peer now extracts into a candidate, so a blob failure never touches
the live tree and there is no aside to observe. The test asserted a weaker
property than the code provides; it now asserts the stronger one — the previous
tree is untouched and the abandoned candidate is cleaned up — and
`waitForMarkedAside` is gone with it.

That is the fourth test on this branch whose mechanism assumed the old in-place
ordering. The pattern is worth naming: any test that observes the aside DURING a
deploy is asserting the old design.

* fix(deploy): re-point dependency links after the candidate becomes live

Windows-only regression, and a genuine consequence of building the replacement
somewhere else rather than a flake.

`npm install` runs in `.deploy-staging/<deploymentId>/<component>`, and for a
`file:` dependency it links `node_modules/<dep>` at the dependency directory. On
POSIX npm writes a RELATIVE symlink, which keeps working after the tree is renamed
to the live path. On Windows it writes a junction, and junctions are ABSOLUTE — so
after the swap they still name a staging path that no longer exists and the
dependency stops resolving. `redeploy-runtime-equivalence` failed with
`Cannot find module 'pure-esm-probe'` for exactly that reason, on Windows only,
which is why every Linux/uWS/Bun shard and every local run was green.

Links that named the build path are re-pointed at their equivalent under the live
path, immediately after the rename.

Chosen over `npm install --install-links`, which would have fixed it by copying
`file:` dependencies instead of linking them: that changes dependency semantics for
every deploy on every platform to solve a problem on one, and silently stops
reflecting edits to a linked source directory.

Tested with an absolute link into the candidate path — the shape that breaks
regardless of platform, so the coverage does not depend on running Windows.
Mutation-verified by removing the repair call.

* fix(deploy): attribute an unreadable journal after the swap, and trim comment narration

Closes the last substantive open finding, plus the comment nit three review rounds
raised.

**Attribution after the swap.** Once the candidate has been renamed to the live
path, nothing under the deployment directory names its component — so a journal
that cannot be parsed was keyed by deployment UUID, which fails NOTHING closed and
lets the component load over state nobody reconciled. The component name is now
written as a small `component` sidecar beside `.complete`, redundant with the
journal on purpose: a corrupt journal no longer costs attribution. The name is
validated before being joined onto the components root, so a corrupt sidecar cannot
point recovery at an unrelated directory.

Three tests: the post-swap corrupt-journal case is keyed by component; with the
sidecar also gone it still reports, under the deployment id; and a sidecar naming
`../../etc` is refused rather than joined.

**Comments trimmed** from 268 added lines to 245, concentrated in the durability
helpers. The long form of the protocol lives in DESIGN.md, so the code keeps only
what the code cannot say — the roll-forward authority rule, why directory fsync is
best-effort, why both rename parents are synced, why platform "cannot fsync" is not
a durability failure — and drops the retrospective framing ("used to", "previously",
"this commit") that belongs in commit messages.

* fix(deploy): path containment, non-destructive relink, and two symmetry gaps

Round 6. Four of these were introduced by round 5's fixes, and two of the four are
symmetry failures — a fix applied in one place and not its sibling. Third time on
this branch; the pattern is the finding.

**`startsWith` is not path containment.** The link relocation classified
`<candidate>-shared` as inside `<candidate>` and rewrote it to an unrelated live
path. Containment is now a `relative()` check that rejects `..` and absolute
results. Mutation-verified: restoring `startsWith` fails the new test.

**Relinking could destroy what it could not replace.** It removed the existing link
before creating the replacement, so an antivirus or permission failure on Windows —
exactly the platform this code exists for — left no link at all, worse than the
dangling one. The replacement is created under a temp name and renamed into place.

**Recovery did not relocate links.** Normal activation repaired them; the
roll-forward path renamed candidate to live without doing so, so a crash between the
swap and the journal's removal brought the component back with unresolvable
dependencies on Windows.

**Directory sync suppressed EIO and ENOSPC.** The file path had already been split
into "platform will not sync" versus "storage failed"; directories had not, so a
real Linux storage error while persisting candidate-to-live was silently ignored.

**Worker startup fabricated a verdict.** It handed the loader an empty failure map,
which asserts "nothing is unreconciled" from a thread that never ran the pass. The
parameter is now optional and absent on workers — distinct from empty.

**`forgetLoadedPath` only forgot the root.** Nested `loadComponent()` calls register
plugin and dependency realpaths under the candidate, so each nested load still leaked
an entry per deploy. Pruned by path prefix.

Reported by codex pre-push.

* fix(deploy): stop Windows candidate installs depending on the build path

Three fixes; two found by auditing for the class the last two rounds kept
surfacing, one because my previous attempt did not work.

**`file:` dependencies still failed on Windows.** The link-repair added last commit
did not fix `redeploy-runtime-equivalence` — CI still reported `Cannot find module
'pure-esm-probe'`, resolving from the live path, so the tree swapped correctly but
its dependency did not resolve. Rather than guess at the link shape a second time,
this removes the dependence instead of repairing it: a win32 candidate build passes
`--install-links`, so npm COPIES `file:` dependencies rather than linking them and
nothing npm writes depends on the build location.

Scoped to win32 AND to candidate builds only, because it does change behavior — a
copied dependency no longer reflects later edits to its source — so it applies to
the platform and path that require it rather than to every deploy everywhere. The
link repair stays as a second line of defence for links from any other source.

**Journal-first did not hold on the deploy path.** The startup ordering was fixed,
but `prepareApplication` runs `recoverOrCleanupStaleExtractionPaths` on every
deploy, and that pass is journal-blind: after an activation whose retirement failed,
the aside still names the DISPLACED tree, so the next deploy would restore the old
version over the live one. Journaled activations for that component are now settled
first, inside the lock. Mutation-verified.

**`syncDirectory`'s open path swallowed everything, including EIO.** Third site of
the same unsupported-versus-real split, after the file sync and the directory sync.
Also removes a double `close()` — the sync catch closed the handle and the `finally`
closed it again.

Two of my own test fixtures were wrong along the way and were corrected rather than
worked around: one asserted a link resolved when staging cleanup had legitimately
removed its target, and one used an empty payload as a "failing" deploy — which
extracts to an empty tree and succeeds, so the deploy correctly replaced the live
version.

* fix(deploy): validate the journal like the sidecar, and stop unlocked residue deletion

Round 7 — first round with three outside lenses (codex, gemini, domain).

**The journal's component name was never validated.** I added a traversal guard to
the sidecar and left the journal — the same field, two sources, one checked. A
syntactically valid journal naming `../../victim` was joined onto the components
root. Both now go through one `isJoinableComponentName`, so they cannot diverge
again, and the journal must also name the deployment directory it sits in: a valid
journal describing someone else's deployment is refused rather than acted on.

That is the fourth symmetry failure on this branch — a guard, a sync split, a link
repair, and now a name check, each applied at one site and not its sibling.

**Replacing a Windows junction could not work.** The non-destructive relink built
the replacement under a temp name and renamed it over the old link — but Windows
cannot rename over an existing junction, which is precisely the case the repair
exists for. It now falls back to removing the old link only once the replacement is
built and ready to move into place, so the failure window still never leaves the
dependency with nothing.

**Link repair was gated on recovery performing the rename.** A crash after normal
activation renamed the candidate but before it repaired the links leaves live
present with stale targets — exactly the case the gate skipped. Repair is
unconditional on the roll-forward path now; it is idempotent when there is nothing
to re-point.

**Residue deletion ran without the component lock.** A reload cycle can run the
recovery pass while another deploy is mid-build, and that candidate has no journal
yet — the journal is written after build and validation — so an unlocked delete
removes a live build. Now taken under the owning component's lock, with a re-check
for a journal that appeared in the meantime.

Two new tests cover the traversal and the deployment-id mismatch.

* fix(deploy): let workers reach the recovery verdict, and bound the candidate flush

Round 8 (codex + gemini + domain), plus one bug of my own found while auditing.

**The recovery verdict never reached worker boot** — raised in three rounds now.
Recovery runs on the main thread, but workers are what serve components, and a
worker cannot be handed main's map: it boots through its own
`loadRootComponents(true)`, possibly before main has finished. Fixed by not passing
the verdict at all — recovery already KEEPS the on-disk evidence for anything it
could not settle, so `unsettleableComponentsFromDisk()` lets any thread reach the
same conclusion read-only.

Only unambiguous evidence counts. A well-formed journal is not evidence — every
healthy deploy has one in flight, and treating it as such would fail a component
closed during its own successful deploy — so this reports only a journal that cannot
be read at all, which no in-flight deploy produces. Three tests pin that boundary.

**A `file:<directory>` candidate had no owner.** Activation accepts a symlinked
candidate; owner detection filtered to real directories, so those candidates looked
unowned and residue removal skipped the lock — the exact case the lock was added
for. Fifth symmetry miss on this branch, same shape as the rest.

**The Windows relink fallback could still end with nothing.** It removes the old
junction, and if the second rename fails for the same reason the first did, there was
no link left. It now restores the original target before propagating.

**Every activation fsynced every file serially under the component lock.** Correct
but expensive — a large dependency tree adds seconds to each deploy, all of it inside
the lock. Bounded fan-out of 16 keeps the ordering guarantee (everything flushed
before `.complete` is written) without paying per-file latency one file at a time.

Also fixed, found by auditing rather than reported: **the rename is the commit point
and nothing after it may compensate.** Making `syncDirectory` throw on EIO put a
throwing step after the swap but inside the compensating try, so rollback would try
to rename the aside over a live path the candidate already occupies — failing, and
reporting failure for a deploy that is live. Post-commit steps are logged now.

The test I wrote for that last one was unfalsifiable and is not included: the link
repair swallows its own readdir failure, so the fixture never made a post-commit step
throw. With sync caught and the repair self-swallowing there is no post-commit step a
test can make throw without fault injection this codebase does not have, so the
restructure rests on reading and on the mutation that showed the old shape was
reachable via an EIO.

* fix(deploy): keep throwaway validation from marking a healthy component broken

The last two open findings from round 8, both in the pre-flight validation load.

**A rejected candidate left the live component reporting ERROR.** The candidate
loads under the REAL component's name, so a candidate that throws at load marks that
component ERROR. Validation then correctly rejects it and the previous version keeps
serving — but the status stayed ERROR, so a healthy component reported as broken for
as long as it ran. The component's status entry is captured before the load and put
back after, via a narrow `restoreStatus` on the registry.

Scoped to the one component rather than suppressing status writes globally the way
`runWithDeployValidationGuard` suppresses registrations: that guard already documents
dropping a legitimate registration from an interleaving real load, which is
acceptable for a registration that re-registers next load and NOT acceptable for
status, where the dropped write could be a genuine failure.

**A rejected `scope.close()` was only logged**, so validation still succeeded and the
candidate activated. `Scope.close()` stops at a throwing listener, so its remaining
internal listener removal and subscription-hold release never run and the throwaway
scope stays partially live — one leak per deploy, on the worker that serves the
component. Teardown failure is now a rejected validation, aggregating the close
errors, so the candidate does not go live on a scope that could not be disposed.

Two tests, mutation-verified: a previous status is restored with its level and
message intact, and a first-ever deploy that fails validation leaves no status entry
behind rather than an ERROR for a component that never existed.

* fix(deploy): give recovery the same ordering barrier, verdict, and blast radius as activation

Round 9's three majors (codex + gemini + cursor-grok + domain — four lenses, the
widest set any round reached). All three are the same underlying mistake: a rule
established for one path and not carried to its sibling.

**Recovery removed the journal without activation's ordering barrier.** Activation
retires the rollback record, flushes the aside directory, and only then removes the
journal — because the journal is the only thing that stops the journal-blind legacy
pass restoring an aside. Recovery removed both immediately after rolling, so a power
failure in between could leave an in-progress aside with no journal, which is exactly
the state that lets the old version be restored over the new one. Recovery now flushes
first and keeps the journal when it cannot confirm the flush; the pass is idempotent,
so the next start settles it again.

Sixth instance of this shape on this branch — after a path guard, a sync split, a
link repair, a name check, and the candidate-owner test.

**The worker verdict could not see a well-formed unsettled journal.** Workers
re-derive the verdict from disk because they cannot be handed main's map, and an
unreadable journal is self-evident — but a well-formed journal main FAILED to settle
looks exactly like a deploy in flight, so workers loaded the component over
unreconciled state. Main now records the reason in an `unsettled` marker beside the
journal, and the read-only verdict reads it first. Best-effort: a missing marker
leaves today's behavior, not a worse one.

**One component's corrupt journal blocked another's deploy.** The per-component settle
parsed every journal before checking which component owned it, so a truncated journal
belonging to `broken-app` threw while preparing an unrelated healthy component —
turning one broken component into a deploy outage for its neighbours. Ownership is
established from the sidecar first, which needs no parsing.

Test added for the recorded verdict, mutation-verified by dropping the marker write.

* fix(deploy): keep the recovery evidence when compensation itself fails

PR review found a collision between two of my own fixes.

The candidate discard added to stop a disk leak (a rejected validation used to leave
a whole installed dependency tree behind) also runs when ACTIVATION failed. That is
correct when compensation succeeded — the previous version is back and the candidate
is garbage. It is wrong when compensation itself failed: whatever blocked B2's rename
plausibly blocks restoring the aside over the same path, and then the live path may be
absent while the candidate, its `.complete` marker and its journal are intact — which
is precisely the state `settleInterruptedActivation` rolls forward, retrying the swap
with an already-validated candidate.

Discarding there traded a bounded disk cost for a component with no version at all.
`compensate` now marks that failure distinctly and the discard is skipped for it, so
the evidence survives for the next start.

Also two docstrings that described code that no longer exists — the same
docs-outlive-the-code problem this branch has produced repeatedly:

  * `activateCandidateApplication` still claimed three effects including root-config
    publication, which was split out of this PR. It now says two, names config as
    explicitly out of scope, and states that the second rename is the commit point.
  * The one-line reporter docstring had landed above `forgetLoadedPath`, which has its
    own docstring; moved to `getErrorReporter`, which it actually describes.

Reported by claude[bot] on #2345.

* fix(deploy): restore the whole status namespace, release validation's modules, widen link repair

Round 9's four minors.

**The status snapshot only covered the bare component name.** Nested loads report
under scoped keys (`web.api`), so a rejected candidate left a plugin-scoped ERROR
behind and the component reported unhealthy through a plugin that never went live.
Snapshot and restore now cover the component's whole namespace, dropping keys only
the candidate introduced.

**Validation retained the extension modules it loaded.** `forgetLoadedPath` released
the candidate's realpath, but the loader also registers extension modules keyed by
module — one set per deploy, for the process lifetime. Snapshot-and-prune rather than
passing a private map, because `loadComponent`'s `providedLoadedComponents`
REASSIGNS the module-level registry: handing it a throwaway map would leave every
later load writing into that map.

**Link repair walked past nested links.** It descended only into `@scope` containers
and directories literally named `node_modules`, so a dependency linked from deeper in
the tree was missed. It now descends fully, bounded by the same dependency tree
activation already traverses.

**Comment narration trimmed** in the three worst places — the peer-branch and
serialization tests and the availability header — dropping "used to", "an earlier
version did", and the retrospective on an unfalsifiable probe that no longer exists.
The invariant stays; the history goes to the commit log.

One self-inflicted scare worth recording: trimming those comments by line range
deleted a fixture line (`old-only.txt`) that the assertion reads back, and the test
caught it immediately. Restored, and the suite is green.

* fix(deploy): make recovery cleanup best-effort, and test the sibling-isolation fix

Two of three PR-review suggestions.

**Recovery's cleanup could fail the whole pass.** Activation wraps its journal and
staging removals in catch-and-warn; recovery left the same two calls unguarded, so a
transient EBUSY removing staging threw out of `settleInterruptedActivation` AFTER the
activation had been correctly settled — turning best-effort cleanup into a hard
failure that also abandons every component queued behind it. Same catch-and-warn now.

Seventh instance on this branch of a rule applied to one path and not its sibling.

**The sibling-isolation fix had no test on the path it protects.** Ownership-before-
parsing is what stops one component's corrupt journal blocking a healthy neighbour's
deploy, and it runs in `settleJournaledActivationsForComponent` — but the only
coverage exercised `recoverInterruptedActivations` (the startup pass). Added a test
that stages a truncated journal for one component and deploys another through
`prepareApplication`, asserting the deploy fails with ITS OWN error and the healthy
tree is untouched. Mutation-verified: removing the ownership check makes the sibling's
journal block the deploy.

The third suggestion — a test forcing `compensate`'s `undo()` to throw so the
candidate's survival is locked in — is not included, and the reply on that thread
explains why: there is no injection point to make the second rename fail
deterministically without fault injection this codebase does not provide, and I
already removed one unfalsifiable test from this branch rather than ship a green
assertion that cannot fail.

* fix(deploy): a cleanup failure must not mark a healthy component unsettled

The aside-record loops in `rollForward` and `rollBack` were still unguarded, and
that combines badly with the `UNSETTLED_MARKER` added two commits ago.

`retireExtractionAside` throws on anything but EEXIST, and `rm(..., { force: true })`
throws on anything but ENOENT, so a transient EACCES or ENOSPC propagated out of
`settleInterruptedActivation` — *after* the tree decision had already been applied on
disk. Both callers read any throw as "could not settle", and
`recoverInterruptedActivations` then writes the unsettled marker, which by its own
contract permanently stops workers loading that component. So a failed `rmdir` on a
disposable directory could take a correctly activated, healthy component out of
service until someone cleared the marker by hand.

Sweeping is now best-effort on both paths, in one shared helper so the two cannot
drift again. Test asserts the roll-forward still lands, no failure is reported, and no
marker is written when the sweep fails; mutation-verified by removing the guard.

Eighth instance on this branch of a rule applied to one path and not its sibling —
and the first where the mismatch was made harmful by an earlier fix of mine rather
than merely inconsistent.

Reported by claude[bot] on #2345.

* fix(deploy): nine review findings — recovery liveness, ownership, and cleanup semantics

kriszyp's review (codex-assisted). All nine inline findings, and the first one
corrects a fix of mine from the previous commit.

**Retiring a rollback record is correctness, not hygiene** — I had made the whole
sweep best-effort, which lets a record survive un-retired while settlement still
removes the journal, and the journal-blind legacy pass then restores the displaced
tree over the candidate just rolled forward. Retiring now propagates (the journal
stays, the next start retries); only sweeping the displaced tree is logged. My test
for the earlier fix asserted the opposite and has been rewritten to the correct
contract.

**Workers settle journals themselves.** A worker can be auto-restarted mid-activation,
and the read-only verdict deliberately ignores a well-formed journal — so the legacy
pass on that worker could restore an old tree over a committed candidate. Settlement
now runs on every thread before legacy recovery; it is safe anywhere because each
deployment is settled under the cross-process component lock and the pass is
idempotent.

**Recovery lock acquisitions were missing `isOwnerAlive`.** A ticket left by a crashed
worker carries this process's pid, so the lock treated it as live and recovery waited
out the multi-hour default instead of reclaiming it.

**An unowned deployment directory is no longer deleted.** `buildCandidateApplication`
creates it and can then spend minutes packing before the candidate tree and sidecar
exist, so "no owner" includes "a live build that has not got that far" — deleting it
raced extraction and failed a valid deploy.

**Residue removal re-reads under the lock and no longer swallows.** The first scan can
race a deploy that publishes a journal before releasing the lock; a journal found now
is settled rather than deleted, and a read error no longer reads as "no journal".

**A stale `unsettled` marker is cleared before the journal.** Left behind by an earlier
failed recovery, it would have main load the component while every worker failed it
closed.

**Marker reads treat only ENOENT as absence** — an unreadable marker was classifying
the journal beside it as a healthy in-flight deploy.

**Validation releases exactly the modules it loaded**, via `collectLoadedModules`,
instead of diffing the global registry: validations serialize only with each other, so
a diff could delete a live module registered by an interleaving real load.

**Link repair walks with bounded concurrency** rather than a serial depth-first walk of
every directory under `node_modules`.

* fix(deploy): make the legacy pass journal-aware; close the git socket before install

Round 10 — first round where all five lenses ran (codex, gemini, cursor-grok,
cursor-composer, harper-domain). Two blockers, five majors.

**The legacy recovery pass is now journal-aware, not merely sequenced after
settlement.** My previous fix ordered settlement before the journal-blind pass on
every thread, which is necessary but not sufficient: settlement that FAILS
deliberately keeps the journal for the next start, and the same boot then walked
straight into the legacy pass, which found the un-retired `.in-progress-` record
and restored the displaced tree over the candidate roll-forward had just
committed. Live silently reverts a release and the next boot blesses it. The
guard now lives in `recoverOrCleanupStaleExtractionPaths` — the one function that
actually moves trees — so every entry point is covered by construction rather
than by each caller remembering to settle first.

**The git credential socket is closed before dependency install scripts run.**
Folding install into `buildCandidateApplication` had quietly widened the
per-deploy credential session to cover it, so a transitive dependency's install
script — arbitrary code from the registry, running as Harper's uid — could ask
the helper for the deployer's git token. `main` scopes the session to extraction
only; this restores that boundary at the new call site, and the comment claiming
it no longer contradicts the code.

**Both trees survive when the live path reappears after the swap started.** A
rollback record proves the live tree was moved aside, so a directory at the live
path afterwards was recreated by something else — the `.next/cache` case the
extraction path already guards. Rolling back there deleted the committed tree AND
the validated candidate and left that stub serving; it now fails closed with both
on disk.

**One deployment can no longer abort the whole settlement scan.** The no-journal
residue branch sat outside the per-deployment catch, so a lock timeout or an EIO
left every later deployment unsettled and unmarked — and a worker only warns
before continuing into the legacy pass.

**An unflushed swap keeps its journal.** The post-rename durability failure was
logged and then the record retired and the journal removed anyway; a power loss
then had no live entry, no rollback record, and nothing saying to roll forward.

**B1's durability barrier compensates.** It runs before the commit point, so a
storage failure there is compensable — and must be, since an uncompensated throw
reads to the caller as an ordinary build failure and discards the candidate, its
`.complete` marker and its journal while live is already moved aside.

**Ownership and record reads no longer swallow into a licence to act.** A failed
sidecar read reported "unowned" and a failed aside read reported "no records";
both are now ENOENT-only, so a real error fails closed instead of letting a
worker load what main failed closed.

Also: an unremovable `unsettled` marker keeps the journal rather than stranding a
settled component; six JSDoc blocks left behind by earlier moves are back on the
functions they describe; and review-history narration is out of the test
comments.

Four new tests, each mutation-verified against the fix it covers.

* fix(deploy): guard where trees move, and stop the boot scan queueing behind a deploy

Round 11 confirmed the round-10 blockers fixed, and found two majors that my
round-10 fixes had themselves introduced.

**The journal guard sits where a tree is actually restored, not at the door.**
Refusing the whole legacy pass on any surviving journal was too broad: a
roll-forward that retires its rollback record but cannot flush the directory
keeps the journal deliberately, and the guard then failed that component closed —
with an error saying the activation was unsettled when in fact the live tree was
the correctly activated candidate, and `prepareApplication` hit the same guard so
it could not be redeployed either. The guard now runs only when a restorable
record exists, which is the only case it was ever protecting against. A retired
record has nothing to restore, so the component loads.

**The boot-time activation scan probes for the component lock instead of
queueing.** Both acquisitions took the default two-hour wait with renewal while
the holder lives. That pass runs before every component load on every thread, so
a worker respawning while a deploy of the same component was mid-`npm install`
would load no components at all until the install finished. It now uses the same
terms the legacy boot probe has always used — a 250ms try, no renewal — because a
held lock means a live deploy, and a live deploy settles its own journal.

**Main now reaches the same verdict as its workers when a stale `unsettled`
marker cannot be cleared.** It was returning quietly, so main served a component
every worker refused to load. It throws instead: the journal survives, both sides
fail closed, and the next start settles again.

**One unreadable staging directory no longer blocks every other component's
recovery.** The guard reads ownership per deployment, preferring the journal's
own `component` field — which is what settlement keys on, so the two now agree —
and falling back to the sidecar. That also unblocks a component legitimately
named `component`, whose candidate path collides with the sidecar's and makes
every sidecar read fail.

The credential test now provisions a real credential session and has the install
step report the sockets it can reach, so it asserts unreachability rather than
call ordering, and skips on Windows where the server does not exist. Two new
recovery tests, both mutation-verified. Comments describing the legacy pass as
"journal-blind" are gone — it enforces the journal itself now.

* docs(design): record the recovery protocol as it now stands

The legacy pass enforces the journal itself rather than depending on being
sequenced after settlement; settlement runs on every thread and probes for the
component lock instead of queueing behind a live deploy; live-present-with-
candidate is no longer an unconditional discard when a rollback record proves the
swap had already started; and retiring a rollback record is correctness, so it
propagates while only the disk sweep stays best-effort.

* fix(deploy): fail the restore gate closed, and stop a deferral leaving a verdict

Round 12 (five lenses). Two majors, both in code the previous round's fixes
introduced.

**The restore gate no longer fails open.** Fixing round 11's cross-component
blocking, I wrapped the journal check in a catch that logged and continued — at
the one gate that authorizes renaming a displaced tree back over what may be a
committed candidate. Any unreadable staging entry then read as "no journal for
this component" and the clobber the journal exists to prevent was back. Nothing
is swallowed there now; the blast radius stays narrow because the gate is only
consulted where a restorable record already exists.

Ownership at that gate also disagreed with settlement, which keys the sidecar
first. Now the destructive step takes the conservative **union** — either
attribution naming the component blocks the restore — and the corrective step
keeps the precise **intersection**. A journal whose two attributions disagree
stalls the restore rather than licensing it, and startup recovery clears it.

**A deferral is no longer written to disk as a verdict.** The new 250ms probe is
right for liveness, but giving up still routed through `fail()`, which plants
`UNSETTLED_MARKER` — and `unsettleableComponentsFromDisk` treats that marker as
authoritative, which is the one thing it documents it must not do for a deploy in
flight. A worker deferring behind a live deploy would leave a marker that outlives
it, failing a healthy, correctly activated component closed on every worker. The
failure is still returned so the thread defers; it just leaves nothing behind.

**Validation no longer evicts shared plugin modules.** `collectLoadedModules`
collected every module the load touched, including a `rest` or `graphql` module
already live from an ordinary load, so cleanup deleted it from the registry and
the next reload cycle no longer saw it as loaded. It now collects only what that
load actually added.

An unremovable `unsettled` marker now names the file in its error, since that
path leaves the component closed on every thread until an operator clears it.

Two new tests, both mutation-verified; DESIGN.md records the protocol as it now
stands.

* fix(deploy): dot-prefix the control files, and stop the verdict pass failing open

Round 13 — first round since #9 with no blockers and no majors. These are its
minors, and two of them were still data-loss paths.

**Control files are dot-prefixed, so a component name can no longer collide with
one.** A deployment directory holds the candidate tree under the component's own
name beside `activation.json`, `component` and `unsettled`, and only the
`component` collision had been reasoned about — by special-casing it. The other
two were worse: a component named `activation.json` puts its tree on the journal
path, the journal write takes EEXIST as "a retry of this activation", and the
swap proceeds with no journal at all, so a crash afterwards lets the legacy pass
replace the committed candidate. One named `unsettled` makes every settle throw
on a non-recursive `rm` of a directory and is failed closed forever.
`isJoinableComponentName` already rejects a leading dot, so dotting all four
makes the whole class unrepresentable instead of handled. No migration: this
staging layout does not exist on `main`.

**An unattributable journal blocks rather than licensing a restore.** A journal
that is present but unparseable, in a deployment with no tree left to infer from,
resolved to "not this component" and the restore proceeded.

**The read-only worker verdict no longer fails open.** It called
`candidateComponentName` unguarded at three sites, and that now propagates every
non-ENOENT error, so one unreadable deployment threw out of the whole pass — and
its caller only warns, so no component was failed closed from marker evidence on
that thread. Each deployment is scoped, and an unreadable one is reported under
its deployment id rather than dropped.

**Disagreeing attributions are reported instead of stalling silently.** The
restore gate blocks the sidecar's component while settlement keys the journal, so
a deployment whose two names disagree could never be cleared by either component's
deploy. It is now failed closed against the component actually stalled, with an
error naming both and the directory to remove.

Two new tests, both mutation-verified. `attribute()` propagates rather than
swallowing, so the per-deployment isolation is a single tested mechanism instead
of two overlapping ones.

* fix(deploy): validate the inferred owner, and wedge neither name silently

Round 14 — second consecutive round with no blockers and no majors. Its minors,
all in the restore gate.

**Ownership inferred from a directory name is now validated.** The readdir
fallback returned whatever single directory a deployment held, so a corrupt
`.activation.json` that is itself a directory came back as the owning component:
a name no component can have, which both bypassed the unattributable-journal
fallback and licensed a restore over a possibly committed candidate. It filters
through `isJoinableComponentName` now, which is the root cause rather than the
symptom — dot-prefixing the control files stops a real component colliding with
one, but nothing stopped a control file impersonating a component.

**An attribution split fails both names.** The union gate blocks a restore for
the sidecar's component and settlement needs the intersection, so the journal
owner is equally wedged — failing only the sidecar's left the other loading
normally right up until the day it needed a restore.

**The unattributable-journal refusal says what to do about it.** There is no
automated way out by construction, since nothing on disk says which component the
journal belongs to, so the error names the directory an operator has to resolve.

One new test, and the split-attribution test now asserts both names; both
mutation-verified.

* docs: state the control-file namespace guarantee accurately

Dotting removes the collision class for every name that reaches a deploy through
isJoinableComponentName, which a root-config key does not — and a key literally
named .activation.json collides only with itself, since a dot-prefixed directory
is skipped by every component scan.

* docs(design): scope the control-file guarantee the same way the code comment does

DESIGN.md still asserted dotting made the collision "unrepresentable" after the
inline comment was corrected to scope it to names that pass
isJoinableComponentName.

* fix(deploy): isolate the last unguarded ownership read on the deploy path

The PR-review bot's remaining sibling-isolation point. candidateComponentName
propagates non-ENOENT errors now, and settleJournaledActivationsForComponent
calls it unguarded while running on every prepareApplication — so one sibling
with an unreadable staging entry failed every neighbour's deploy, which is the
outage the ownership-before-parsing ordering exists to prevent.

Skipping is safe specifically here because settlement is the corrective half: if
the entry does turn out to be this component's, the restore gate takes the union
and fails closed on anything it cannot attribute, so nothing destructive proceeds
on an entry this skipped.

Mutation-verified against the new test.

* fix(deploy): scope the validation guard to its async context instead of a counter

The PR-review bot's status-registry finding, and it is the same bug class as
kriszyp's module-registry one: shared mutable state reconciled by
snapshot-and-revert, where an interleaving legitimate write is indistinguishable
from the candidate's.

The live component keeps serving on this worker for the whole validation window,
and `statusForComponent()` is a public API its own runtime code may call at any
time — a health check, a reconnect handler. Both it and the throwaway candidate
load wrote into the same registry entry under the same name, so
`restoreNamespace` reverted a genuine live report with no log and no way for an
operator to know. An edge-triggered reporter would then never re-send it.

The root cause is that `runWithDeployValidationGuard` was a process-wide DEPTH
COUNTER, which cannot tell the two writers apart. It is an `AsyncLocalStorage`
context now, and only work descending from the validation load is treated as the
candidate's:

- Status writes from inside that context go to the context's own throwaway map,
  so the live registry is never written and never needs reverting.
  `snapshotNamespace`, `restoreNamespace` and `restoreStatus` are all gone —
  `restoreStatus` had no production caller at all, only its own tests.
- The `server.*` registration suppression gets the same precision for free. Its
  documented caveat — "a legitimate registration from an interleaving real load
  is also skipped" — was this same defect, and no longer applies.

Also drops a duplicated `prepareApplicationStub.calledOnce` assertion the bot
spotted in operations.test.js.

Four status tests, mutation-verified against the sink, including one that fires a
callback scheduled BEFORE the deploy — the case a counter gets wrong.

* fix(deploy): never discard the last committed tree, and isolate validation reads

One major and three minors, the major in my own ambiguity guard.

**The "live path reappeared" guard was gated on the c…
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.

4 participants