From 19709dcc8055040c080b3a2b748811c1ca7e32d3 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 17 Jul 2026 15:20:24 -0400 Subject: [PATCH 01/22] feat(deploy): two-phase stage/activate for deploy_component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- DESIGN.md | 43 + components/Application.ts | 323 +++++-- components/deploymentRecorder.ts | 12 +- components/operations.js | 846 +++++++++++++----- components/operationsValidation.js | 145 ++- server/serverHelpers/serverHandlers.js | 2 + server/serverHelpers/serverUtilities.ts | 8 + .../components/deployPhaseValidators.test.js | 86 ++ unitTests/components/deployStaging.test.js | 171 ++++ .../server/fastifyRoutes/operations.test.js | 41 +- utility/hdbTerms.ts | 6 + utility/operation_authorization.ts | 2 + 12 files changed, 1338 insertions(+), 347 deletions(-) create mode 100644 unitTests/components/deployPhaseValidators.test.js create mode 100644 unitTests/components/deployStaging.test.js diff --git a/DESIGN.md b/DESIGN.md index e26b359ff9..37fef08b92 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -305,3 +305,46 @@ guarded against (in the scan or in tar-fs's own pack walk); that's a pre-existin this fix doesn't attempt to solve. `deploy_component`/`package_component` still never validate that declared entry points (`jsResource`/`graphqlSchema`) survived extraction — a truncation from some other future cause would still report success silently; that's a deferred, separate fix. + +## Two-phase deploy: stage then activate (`components/Application.ts`, `components/operations.js`) + +`deploy_component` splits into two replicated phases so a cluster deploy is all-or-nothing at the +point of go-live. **Phase 1 (`stage_component`)** builds the incoming version — download/`npm pack` +(incl. a git clone), extract, `npm install` — into a hidden staging directory on every node. +**Phase 2 (`activate_component`)** atomically renames the staged copy into the live component path and +restarts. `deploy_component` orchestrates the two: it stages on the origin, replicates +`stage_component` to peers, **waits for every node to report a successful stage before any node +activates** (`ignore_replication_errors` opts out of the barrier), then replicates +`activate_component`. If a node can't fetch the package or fails `npm install`, it fails during +staging while the live component is still untouched _on every node_ — where the old one-shot path +could leave a peer half-installed after other peers had already restarted onto the new code. The +request/response contract is unchanged; only the SSE phase names differ (`stage`/`activate` vs the +old `prepare`/`replicate`). `two_phase: false` forces the legacy one-shot path. + +The staging directory (`.deploy-staging//`) lives **under the components root**, +not in `os.tmpdir()`, even though its contents are transient. This is deliberate and load-bearing: +the go-live step is `rename(stagingDir, liveDir)`, which is only atomic when both paths share a +filesystem. `os.tmpdir()` is frequently a different mount (tmpfs, a separate volume); a cross-device +rename throws `EXDEV` and Node has no atomic fallback — you'd be back to a slow recursive copy at the +exact moment you want the swap to be instantaneous, reintroducing the downtime window the split +exists to remove. The leading dot keeps `loadComponentDirectories` from loading it as a phantom +component, and it is **not** the watched base of any component's file watcher (those are rooted at +each live component dir, `EntryHandler`/`deriveCommonPatternBase`) — so building here fires no +restart-on-change events and needs no `deploy:start` watcher suppression. That suppression is now +scoped to `activateApplication`, the only phase that writes the live path. Staging is deterministic +from the deployment id precisely so `activate_component` (a separate replicated operation, and on +peers a separate invocation from `stage_component`) can reconstruct the same path the stage built — +peers build a fresh `Application` per sub-operation, so there is no shared in-memory handle to rely +on. `extractApplication`/`installApplication` build into `application.buildDirPath`, which defaults +to the live dir (`dirPath`) — this is what keeps the legacy one-shot path, boot-time +`installApplications`, and the direct `extractApplication` callers unchanged — and is repointed at +the staging dir only for the duration of a stage. + +Two-phase requires the `system` database to be replicated on the origin (`isSystemDatabaseReplicated`), +since the `hdb_deployment` row's `payload_blob` is how peers fetch the tarball and correlate the two +phases by deployment id. When `system` is excluded from a narrow `REPLICATION_DATABASES`, or the +caller passes `two_phase: false`, or the invocation is a peer replaying a one-shot deploy, +`deploy_component` falls back to `deployComponentOneShot` (the previous behavior, preserved verbatim). +Known gap for a rolling upgrade window: an origin on this version replicating `stage_component` to a +peer that predates these operations will see that peer fail the op; `two_phase: false` or +`ignore_replication_errors` is the escape hatch until capability negotiation lands. diff --git a/components/Application.ts b/components/Application.ts index 9e3e55297a..d5de3bfe69 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -401,10 +401,64 @@ async function packGitReferenceWithoutScripts( } // Hidden directory under the components root holding component versions renamed aside -// during a deploy swap (see extractApplication). The leading dot keeps +// during a deploy swap (see activateApplication). The leading dot keeps // loadComponentDirectories from loading its contents as components. export const ASIDE_STAGING_DIR = '.deploy-aside'; +// Hidden directory under the components root where the INCOMING version of a component is +// fully built (extracted + `npm install`) before it goes live — the counterpart to +// ASIDE_STAGING_DIR, which holds the OUTGOING version. Two-phase deploy stages here first +// (stage_component), then activateApplication renames the staged copy into the live +// component path in one atomic step (activate_component). +// +// It lives UNDER the components root on purpose, even though the bytes are "temporary": +// - Same filesystem as the live path, so the go-live rename() is atomic. An os.tmpdir() +// location is frequently a different mount (tmpfs / separate volume); a cross-device +// rename throws EXDEV and degrades to a slow recursive copy — reintroducing the very +// downtime window the two-phase split exists to remove. +// - The leading dot keeps loadComponentDirectories (componentLoader) from loading it as a +// phantom component, and it is not the watched base of any component's file watcher +// (those are rooted at each live component dir), so building here triggers no +// restart-on-change storm and needs no deploy:start watcher suppression. +export const DEPLOY_STAGING_DIR = '.deploy-staging'; + +/** + * Atomically move `targetDirPath` aside into a hidden, per-component staging directory if it + * exists, returning the aside staging directory (for best-effort cleanup) or null when there was + * nothing to move. + * + * Renaming the old directory aside — instead of clearing it in place — is immune to the race where + * a still-running worker keeps writing into the directory (e.g. a live Next.js app writing into + * `.next/cache`): an in-place recursive rm races that writer and fails with ENOTEMPTY, whereas the + * rename is atomic and the old worker harmlessly keeps writing into the renamed inode until it is + * replaced on restart. The aside lives on the same filesystem (sibling hidden dir under the same + * parent) so the rename stays atomic, and is per-component so a sibling deploy never collides with + * or sweeps another's aside. + */ +async function moveDirAside(targetDirPath: string): Promise { + const asideStagingDir = join(dirname(targetDirPath), ASIDE_STAGING_DIR, basename(targetDirPath)); + try { + await access(targetDirPath, constants.F_OK); + } catch (err) { + if (err.code === 'ENOENT') return null; // nothing there to move + throw err; + } + await mkdir(asideStagingDir, { recursive: true }); + await rename(targetDirPath, join(asideStagingDir, `${process.pid}-${Date.now()}-${randomUUID()}`)); + return asideStagingDir; +} + +// Best-effort removal of a per-component aside staging directory. The old worker may still hold +// files open in a renamed copy (the live writer that motivated the rename), so a failure here is +// expected in that case and logged at trace rather than as a warning — the survivor is swept by +// the next deploy. +function cleanupAsideDir(asideStagingDir: string | null, componentName: string): void { + if (!asideStagingDir) return; + rm(asideStagingDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }).catch((err) => + logger.trace?.(`Deferred cleanup of previous ${componentName} component directory: ${err.message}`) + ); +} + // The credential helper git executes for a private git-reference deploy. It ships alongside this // module (both in source and in dist), holds no secret, and is inert without a live session. export const GIT_CREDENTIAL_HELPER_PATH = join(__dirname, 'gitCredentialHelper.js'); @@ -414,7 +468,10 @@ export const GIT_CREDENTIAL_HELPER_PATH = join(__dirname, 'gitCredentialHelper.j * * Only one of `application.payload` or `application.package` should be specified; otherwise, an error is thrown. * - * Writes the application to the configured components root directory using the `application.name` and overwrites any existing directory. + * Writes the application into `application.buildDirPath`, overwriting any existing directory there. + * By default that is the live component directory (`application.dirPath`); during a two-phase deploy + * `stageApplication` points it at the hidden staging directory instead, so the live path is never + * touched until `activateApplication` swaps the staged copy into place. * * This method should only be called from the main thread */ @@ -447,8 +504,10 @@ export async function extractApplication(application: Application) { tarball = Readable.from(payload as Buffer); } } else { - // Given a package, there are a a couple options - const parentDirPath = dirname(application.dirPath); + // Given a package, there are a a couple options. The tarball is packed next to the build + // target (the staging dir during a two-phase deploy) so it lands on the same filesystem and + // is swept with the staging area rather than littering the live components root. + const parentDirPath = dirname(application.buildDirPath); // If the package identifier is a file path we need to check if its a tarball or a directory if (application.packageIdentifier.startsWith('file:')) { @@ -458,8 +517,11 @@ export async function extractApplication(application: Application) { const stats = await stat(packagePath); if (stats.isDirectory()) { - // If its a directory, symlink - await symlink(packagePath, application.dirPath, 'dir'); + // If its a directory, symlink. A stale build target (e.g. a retried stage) would + // make symlink() throw EEXIST, so clear it first. + await rm(application.buildDirPath, { recursive: true, force: true }); + await mkdir(dirname(application.buildDirPath), { recursive: true }); + await symlink(packagePath, application.buildDirPath, 'dir'); // And return early since we're done; no extraction needed return; } @@ -553,56 +615,38 @@ export async function extractApplication(application: Application) { } } - // Replace any existing component directory atomically instead of clearing it in - // place. A previous version's worker can still be running and actively writing - // into this directory — e.g. a live Next.js app writing into `.next/cache` — and - // an in-place recursive rm races that writer: rm empties `.next`, then its leaf - // `rmdir('.next')` fails with ENOTEMPTY because the worker just re-created a cache - // entry. (`force: true` only suppresses ENOENT; ENOTEMPTY is not retried unless - // `maxRetries` is set, and a continuously-writing app would outlast retries - // anyway.) Renaming the old directory aside is atomic and immune to the race: the - // still-running worker keeps writing into the renamed inode harmlessly until it's - // replaced on restart, and the aside copy is removed best-effort below. - // - // The aside lives under a hidden, component-scoped staging directory inside the - // components root: same filesystem as the source so the rename stays atomic, the - // leading dot keeps loadComponentDirectories from picking it up as a phantom - // component, and the per-component path means a sibling component never collides - // with (or sweeps) another's aside. - const asideStagingDir = join(dirname(application.dirPath), ASIDE_STAGING_DIR, basename(application.dirPath)); - let didRenameAside = false; - try { - await access(application.dirPath, constants.F_OK); - await mkdir(asideStagingDir, { recursive: true }); - await rename(application.dirPath, join(asideStagingDir, `${process.pid}-${Date.now()}-${randomUUID()}`)); - didRenameAside = true; - } catch (err) { - // Ignore does not exist error - if (err.code !== 'ENOENT') { - throw err; - } - } - // Finally, create the application directory fresh - await mkdir(application.dirPath, { recursive: true }); + const buildDirPath = application.buildDirPath; + + // Replace any existing build directory atomically instead of clearing it in place. When the + // build target IS the live directory (the legacy in-place path, and boot-time installs), a + // previous version's worker can still be running and actively writing into it — e.g. a live + // Next.js app writing into `.next/cache` — and an in-place recursive rm races that writer, + // failing with ENOTEMPTY. moveDirAside renames it aside atomically instead. When the build + // target is a fresh two-phase staging dir there is normally nothing to move (a retried stage + // is the exception), so this is a cheap no-op on the common path. + const asideStagingDir = await moveDirAside(buildDirPath); + + // Create the build directory fresh + await mkdir(buildDirPath, { recursive: true }); // Now pipeline the tarball into maybe-gunzip then tar-fs to reliably decompress and extract the contents - await pipeline(tarball, gunzip(), extract(application.dirPath)); + await pipeline(tarball, gunzip(), extract(buildDirPath)); // If the extracted directory contains a single folder, move the contents up one level // The `npm pack` command does this (the top-level folder is called "package") // Other packing tools may have similar behavior, but the directory name is not guaranteed. - const extracted = await readdir(application.dirPath, { withFileTypes: true }); + const extracted = await readdir(buildDirPath, { withFileTypes: true }); if (extracted.length === 1 && extracted[0].isDirectory()) { - const topLevelDirPath = join(application.dirPath, extracted[0].name); + const topLevelDirPath = join(buildDirPath, extracted[0].name); - const tempDirPath = await mkdtemp(application.dirPath); + const tempDirPath = await mkdtemp(buildDirPath); // Copy contents of top-level directory to temp directory (in order to avoid collisions of top-level directory name and one of the contents) await cp(topLevelDirPath, tempDirPath, { recursive: true }); // Remove top-level directory await rm(topLevelDirPath, { recursive: true, force: true }); // Copy contents of temp directory to application directory - await cp(tempDirPath, application.dirPath, { recursive: true }); + await cp(tempDirPath, buildDirPath, { recursive: true }); // Finally, remove the temp dir await rm(tempDirPath, { recursive: true, force: true }); } @@ -612,33 +656,29 @@ export async function extractApplication(application: Application) { await rm(tarballPath, { force: true }); } - // Remove this component's aside copies. The old worker may still hold files open - // in the just-renamed copy (the live writer that motivated the rename), so this is - // best-effort: removing the whole staging subdirectory also clears leftovers from - // earlier deploys whose workers have since exited, and a copy that survives because - // its worker is still live is swept by the next deploy. The failure is expected in - // the live-worker case, so it's logged at trace rather than as a warning. - if (didRenameAside) { - rm(asideStagingDir, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }).catch((err) => - logger.trace?.(`Deferred cleanup of previous ${application.name} component directory: ${err.message}`) - ); - } + // Remove this component's aside copies (best-effort; see cleanupAsideDir). + cleanupAsideDir(asideStagingDir, application.name); } /** - * Install an application to its relative `application.dirPath` using either a + * Install an application into `application.buildDirPath` using either a * configured `application.install` command, a derived package manager from the * application's `package.json#devEngines`, or falling back to the default * package manager, `npm`. * - * Will return early if `node_modules` already exists within the `application.dirPath` + * `buildDirPath` is the live component directory by default, or the hidden staging directory + * during a two-phase deploy (see stageApplication) — so `npm install`, the slowest and most + * failure-prone step, runs against the staged copy and never leaves the live path half-installed. + * + * Will return early if `node_modules` already exists within the build directory. * * This method should only be called from the main thread */ export async function installApplication(application: Application) { + const buildDirPath = application.buildDirPath; let packageJSON: any; try { - packageJSON = JSON.parse(await readFile(join(application.dirPath, 'package.json'), 'utf8')); + packageJSON = JSON.parse(await readFile(join(buildDirPath, 'package.json'), 'utf8')); } catch (err) { if (err.code !== 'ENOENT') throw err; // If no package.json, nothing to install @@ -647,7 +687,7 @@ export async function installApplication(application: Application) { } try { // Does node_modules exist? - await access(join(application.dirPath, 'node_modules'), constants.F_OK); + await access(join(buildDirPath, 'node_modules'), constants.F_OK); application.logger.info(`Application ${application.name} already has node_modules; skipping install`); return; } catch (err) { @@ -665,7 +705,7 @@ export async function installApplication(application: Application) { application.name, command, args, - application.dirPath, + buildDirPath, application.install?.timeout, customOnLine, application.npmUserconfigPath @@ -726,7 +766,7 @@ export async function installApplication(application: Application) { application.name, (application.packageManagerPrefix ? application.packageManagerPrefix + ' ' : '') + packageManager.name, application.install?.allowInstallScripts ? ['install'] : ['install', '--ignore-scripts'], // All of `npm`, `yarn`, and `pnpm` support the `install` command. If we need to configure options here we may have to use some other defaults though - application.dirPath, + buildDirPath, application.install?.timeout, pmOnLine, application.npmUserconfigPath @@ -783,7 +823,7 @@ export async function installApplication(application: Application) { application.name, (application.packageManagerPrefix ? application.packageManagerPrefix + ' ' : '') + 'npm', npmInstallArgs, - application.dirPath, + buildDirPath, application.install?.timeout, npmOnLine, application.npmUserconfigPath @@ -825,6 +865,11 @@ interface ApplicationOptions { // Deploy credentials already resolved to literal tokens, of any kind; partitioned by the // constructor into the npm and git halves, which are injected by entirely different mechanisms. credentials?: ResolvedCredential[]; + // Stable identifier for the hidden staging directory this deploy builds into, so a two-phase + // deploy's `activate_component` step can reconstruct the same staging path a prior + // `stage_component` built (both derive it from the deployment id). Defaults to a random UUID for + // callers that stage and activate against one in-memory Application instance. + stagingId?: string; } export class Application { @@ -846,10 +891,23 @@ export class Application { // Path to the per-deploy `.npmrc`, set by writeTransientNpmrc() during prepareApplication and // passed to the spawn calls; undefined when no registry credentials were provided. npmUserconfigPath?: string; + // Stable id for this deploy's staging directory (see ApplicationOptions.stagingId). + stagingId: string; + // When set, extract/install build here instead of the live `dirPath`. stageApplication() points + // it at `stagingDirPath`; activateApplication() clears it after swapping the staged copy live. + #buildDirPath?: string; #npmrcTempDir?: string; #gitCredentialSession?: GitCredentialSession; - constructor({ name, payload, packageIdentifier, install, onInstallLine, credentials }: ApplicationOptions) { + constructor({ + name, + payload, + packageIdentifier, + install, + onInstallLine, + credentials, + stagingId, + }: ApplicationOptions) { this.name = name; this.payload = payload; this.packageIdentifier = packageIdentifier && derivePackageIdentifier(packageIdentifier); @@ -872,10 +930,37 @@ export class Application { const componentsRoot = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); if (!componentsRoot) throw new Error('componentsRoot is not configured'); this.dirPath = join(componentsRoot, name); + this.stagingId = stagingId ?? randomUUID(); this.logger = logger.loggerWithTag(name); this.packageManagerPrefix = getConfigValue(CONFIG_PARAMS.APPLICATIONS_PACKAGEMANAGERPREFIX); } + // Directory where extract/install currently build. Defaults to the live component directory + // (`dirPath`) — the legacy in-place path, still used by boot-time installApplications() — and is + // repointed at the hidden staging directory for the duration of a two-phase deploy. + get buildDirPath(): string { + return this.#buildDirPath ?? this.dirPath; + } + + // Hidden, per-deploy staging directory the incoming version is built into before it goes live. + // Deterministic from (component name, stagingId) so `activate_component` can find what + // `stage_component` built. Sits under the components root (dirname(dirPath)) so the go-live + // rename() into `dirPath` stays on one filesystem and is therefore atomic. See DEPLOY_STAGING_DIR. + get stagingDirPath(): string { + return join(dirname(this.dirPath), DEPLOY_STAGING_DIR, this.name, this.stagingId); + } + + // Route extract/install into the staging directory. Called by stageApplication(). + useStagingBuildDir(): void { + this.#buildDirPath = this.stagingDirPath; + } + + // Restore the live component directory as the build target. Called by activateApplication() + // once the staged copy has been swapped into place. + useLiveBuildDir(): void { + this.#buildDirPath = undefined; + } + // Write the transient `.npmrc` into a fresh 0700 temp dir (file mode 0600) and record its path // so the deploy's npm spawns authenticate against the private registry. No-op without registry // credentials. @@ -1014,6 +1099,122 @@ export async function prepareApplication(application: Application) { } } +/** + * Phase 1 of a two-phase deploy: build the INCOMING version of a component completely — download / + * `npm pack` (incl. a git clone), extract, and `npm install` — into the hidden staging directory, + * WITHOUT touching the live component directory. + * + * This is the slow, failure-prone half of a deploy, and doing it off to the side has two payoffs: + * - It is safe to run across the whole cluster and gate on: if a node can't fetch the package or + * `npm install` fails, that node reports the failure and NOTHING has changed anywhere — the live + * component is untouched on every node. Contrast the one-shot deploy, where a peer can fail + * mid-install with a half-written live directory while other peers have already gone live. + * - The staging directory is not the watched base of any component's file watcher and is ignored + * by the component loader (leading dot), so building here triggers no restart-on-change storm. + * No deploy:start/deploy:end watcher suppression is needed for this phase — that is reserved for + * activateApplication, which is the only phase that writes the live path. + * + * This method should only be called from the main thread. + * + * @param application The application to stage. + * @returns The absolute path of the staging directory the incoming version was built into. + */ +export async function stageApplication(application: Application): Promise { + application.useStagingBuildDir(); + // Start from a clean slate so a retried stage (same deployment id) can't inherit a half-built + // tree from a previous attempt. + await rm(application.stagingDirPath, { recursive: true, force: true }); + try { + await application.writeTransientNpmrc(); + try { + await application.startGitCredentialSession(); + await extractApplication(application); + } finally { + await application.cleanupGitCredentialSession(); + } + await installApplication(application); + } catch (err) { + // A failed stage leaves nothing live; remove the partial staging tree so it can't accumulate + // or be mistaken for a good build. Best-effort — never mask the original failure. + await rm(application.stagingDirPath, { recursive: true, force: true }).catch(() => {}); + application.useLiveBuildDir(); + throw err; + } finally { + await application.cleanupTransientNpmrc(); + } + return application.stagingDirPath; +} + +/** + * Phase 2 of a two-phase deploy: swap the already-staged incoming version into the live component + * directory in one atomic `rename()`, then let watchers restart onto it. + * + * This is the short, low-risk half — no network, no install, just a directory swap — so the window + * during which the component is being replaced is as small as the filesystem allows, and it is only + * entered once staging has succeeded (cluster-wide, when orchestrated by deploy_component). + * + * Bracketed with deploy:start/deploy:end so every thread's file watchers suppress restart-on-change + * while the live directory is replaced (harper#488) — the same suppression the one-shot deploy used + * to hold for the entire extract+install; here it wraps only the swap. + * + * This method should only be called from the main thread. + */ +export async function activateApplication(application: Application): Promise { + const stagingDirPath = application.stagingDirPath; + try { + await access(stagingDirPath, constants.F_OK); + } catch (err) { + if (err.code === 'ENOENT') { + throw new Error( + `Cannot activate ${application.name}: no staged build found at ${stagingDirPath}. ` + + `Stage the component (stage_component) before activating it.` + ); + } + throw err; + } + await broadcastDeployStart(application.name); + let asideStagingDir: string | null = null; + try { + // Move the current live version aside (atomic; tolerates a still-writing worker), then rename + // the staged copy into place. Both live under the components root, so the rename is same-fs and + // atomic — there is no interval where `dirPath` is a partially populated directory. + asideStagingDir = await moveDirAside(application.dirPath); + await mkdir(dirname(application.dirPath), { recursive: true }); + await rename(stagingDirPath, application.dirPath); + application.useLiveBuildDir(); + } finally { + broadcastDeployEnd(application.name); + } + // Best-effort cleanup of the outgoing version (see cleanupAsideDir) and the now-empty per-component + // staging parent. + cleanupAsideDir(asideStagingDir, application.name); + rm(dirname(stagingDirPath), { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }).catch((err) => + logger.trace?.(`Deferred cleanup of ${application.name} staging directory: ${err.message}`) + ); +} + +/** + * Discard a staged-but-not-activated build (an aborted two-phase deploy). Best-effort: removes the + * staging tree and tears down any transient credential state. The live component directory is never + * touched. Safe to call whether or not staging ever ran. + */ +export async function discardStagedApplication(application: Application): Promise { + try { + await application.cleanupGitCredentialSession(); + } catch { + /* best-effort */ + } + try { + await application.cleanupTransientNpmrc(); + } catch { + /* best-effort */ + } + application.useLiveBuildDir(); + await rm(application.stagingDirPath, { recursive: true, force: true }).catch((err) => + logger.trace?.(`Failed to discard ${application.name} staging directory: ${err.message}`) + ); +} + /** * Install all applications specified in the root config. * diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index be96eec483..73f499bbf1 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -49,8 +49,14 @@ type DeploymentStatus = | 'pending' | 'extracting' | 'installing' + // Two-phase deploy: building the incoming version into staging cluster-wide (stage phase), and the + // terminal resting state of a stage_component that has not yet been activated. + | 'staging' + | 'staged' | 'loading' | 'replicating' + // Two-phase deploy: swapping the staged build into the live path cluster-wide (activate phase). + | 'activating' | 'restarting' | 'success' | 'failed' @@ -395,7 +401,7 @@ export class DeploymentRecorder { this.sealed = true; } - async finish(status: 'success' | 'failed' | 'rolled_back', error?: unknown): Promise { + async finish(status: 'success' | 'failed' | 'rolled_back' | 'staged', error?: unknown): Promise { if (this.finished) return; // Send a terminal sentinel through the emitter (if any) BEFORE we unsubscribe and // remove it from the registry, so any SSE tail subscribers can resolve their wait @@ -564,10 +570,14 @@ function startStatusFor(phase: string | undefined): DeploymentStatus | null { return 'extracting'; case 'install': return 'installing'; + case 'stage': + return 'staging'; case 'load': return 'loading'; case 'replicate': return 'replicating'; + case 'activate': + return 'activating'; case 'restart': return 'restarting'; default: diff --git a/components/operations.js b/components/operations.js index 9231ad3ce4..303512a445 100644 --- a/components/operations.js +++ b/components/operations.js @@ -24,7 +24,15 @@ const { HDB_ERROR_MSGS, HTTP_STATUS_CODES } = hdbErrors; const manageThreads = require('../server/threads/manageThreads.js'); const { packageDirectory } = require('../components/packageComponent.ts'); const { Resources } = require('../resources/Resources.ts'); -const { Application, prepareApplication, ASIDE_STAGING_DIR } = require('./Application.ts'); +const { + Application, + prepareApplication, + stageApplication, + activateApplication, + discardStagedApplication, + ASIDE_STAGING_DIR, + DEPLOY_STAGING_DIR, +} = require('./Application.ts'); const { server } = require('../server/Server.ts'); const { DeploymentRecorder, awaitDeploymentRow, DEFAULT_AWAIT_ROW_TIMEOUT_MS } = require('./deploymentRecorder.ts'); const { ProgressEmitter } = require('../server/serverHelpers/progressEmitter.ts'); @@ -352,11 +360,23 @@ async function packageComponent(req) { } /** - * Can deploy a component in multiple ways. If a 'package' is provided all it will do is write that package to - * harperdb-config, when HDB is restarted the package will be installed in hdb/nodeModules. If a base64 encoded string is passed it - * will write string to a temp tar file and extract that file into the deployed project in hdb/components. + * Deploy a component. Front door for the deploy family: derives the project name, validates, ingests + * any credential token into the secrets store (so it lives as a replicated reference, not embedded), + * then dispatches to the two-phase orchestrator (default) or the legacy one-shot path. + * + * Two-phase (stage → activate) builds the incoming version into a hidden staging directory on EVERY + * node first, verifies it landed everywhere, and only then swaps it live cluster-wide — so a node + * that can't fetch the package or fails `npm install` fails the deploy while the live component is + * still untouched on every node, and the go-live window shrinks to a fast atomic directory swap. + * See stageApplication/activateApplication in components/Application.ts. + * + * The request/response contract is unchanged: same inputs (`package`/payload, `restart`, + * `install_*`, `credentials`, `ignore_replication_errors`, `deployment_timeout`, …), same + * `deployment_id` in the response, same SSE progress stream (now emitting `stage`/`activate` phases + * instead of `prepare`/`replicate`). Pass `two_phase: false` to force the legacy one-shot path. + * * @param req - * @returns {Promise} + * @returns {Promise} */ async function deployComponent(req) { if (req.project) { @@ -374,53 +394,46 @@ async function deployComponent(req) { // replicated ciphertext (reference, not embed); already-reference entries pass through, and with // no custody a literal token stays as a transient, this-node-only fallback (#1158). Peers // re-running a replicated deploy already carry references and never re-ingest. - const { ingestCredentials, resolveCredentials } = require('./secretOperations.ts'); + const { ingestCredentials } = require('./secretOperations.ts'); req.credentials = await ingestCredentials(req, req.credentials, req.project); // References are safe to persist (config + deployment row) and replicate; a no-custody literal - // token is not — it is used only for this node's install below, then stripped before replication. + // token is not — it is used only for this node's install, then stripped before replication. const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); - // Write to root config if the request contains a package identifier - if (req.package) { - // Check if trying to overwrite a core component (requires force) - // Lazy-load to avoid circular dependency with componentLoader - const { TRUSTED_RESOURCE_PLUGINS } = require('./componentLoader.ts'); - if (TRUSTED_RESOURCE_PLUGINS[req.project] && !req.force) { - throw handleHDBError( - new Error(), - `Cannot deploy component with name '${req.project}': this is a protected core component name. Use force: true to overwrite.`, - HTTP_STATUS_CODES.CONFLICT - ); - } - - const applicationConfig = { package: req.package }; - // Avoid writing an empty `install:` block - if (req.install_command || req.install_timeout || req.install_allow_scripts !== undefined) { - applicationConfig.install = { - command: req.install_command, - timeout: req.install_timeout, - allowInstallScripts: req.install_allow_scripts, - }; - } - if (req.urlPath !== undefined) applicationConfig.urlPath = req.urlPath; - // Persist credential references (never tokens) so every cold install of this component — - // reboot, new peer, rollback — re-resolves the credential from the store. - if (credentialReferences.length) applicationConfig.credentials = credentialReferences; - await configUtils.addConfig(req.project, applicationConfig); - } - - // Create a hdb_deployment row up front so the deploy is observable and auditable - // even if the CLI disconnects. The row also holds the payload in a Blob attribute, - // which doubles as the source for peer replication and (later) rollback. - // - // Only the origin node records — peers receiving a replicated deploy_component skip - // recording so we don't accumulate one row per node for the same deploy. The row - // reaches peers via the table's standard replication; the peer-side branch below - // reads payload_blob back from there. + // A peer replaying a replicated ONE-SHOT deploy_component arrives with `_deploymentId` set. In + // two-phase mode the origin never sends deploy_component to peers (it sends stage_component / + // activate_component), so `_deploymentId` here always means "legacy peer". const isReplicatedExecution = typeof req._deploymentId === 'string'; - // An SSE-bound caller already attached a ProgressEmitter (created in the server - // handler so it can also drive the response stream). Reuse it; otherwise spin up a - // fresh emitter so the recorder still gets phase events for non-SSE deploys. + + // Two-phase is the default, but it leans on the system table's replication channel to carry the + // payload and correlate the stage/activate steps across the cluster. Fall back to the legacy + // one-shot deploy when: the caller opted out (`two_phase: false`); this is a peer replaying a + // one-shot deploy; or `system` isn't replicated on this node (a narrow REPLICATION_DATABASES). + if (req.two_phase === false || isReplicatedExecution || !isSystemDatabaseReplicated()) { + return deployComponentOneShot(req, credentialReferences, isReplicatedExecution); + } + return deployComponentTwoPhase(req, credentialReferences); +} + +/** + * Legacy one-shot deploy: extract + `npm install` in place on the origin, then replicate the whole + * deploy_component operation to peers, which each do the same. Preserved verbatim (behavior-for- + * behavior) as the fallback path for `two_phase: false` and for peers replaying a one-shot deploy. + * The wrapper has already derived the project name, validated, and ingested credentials. + */ +async function deployComponentOneShot(req, credentialReferences, isReplicatedExecution) { + const { resolveCredentials } = require('./secretOperations.ts'); + + // Write to root config if the request contains a package identifier + if (req.package) await writeComponentRootConfig(req, credentialReferences); + + // Create a hdb_deployment row up front so the deploy is observable and auditable even if the CLI + // disconnects. The row also holds the payload in a Blob attribute, which doubles as the source for + // peer replication and (later) rollback. Only the origin node records — peers replaying the + // replicated deploy skip recording so we don't accumulate one row per node for the same deploy. + // An SSE-bound caller already attached a ProgressEmitter (created in the server handler so it can + // also drive the response stream). Reuse it; otherwise spin up a fresh emitter so the recorder + // still gets phase events for non-SSE deploys. const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); if (emitter && !req.progress) req.progress = emitter; const recorder = isReplicatedExecution @@ -438,76 +451,27 @@ async function deployComponent(req) { const emit = (event, data) => emitter?.emit(event, data); - // The new payload-via-replicated-row path depends on the `system` database actually - // being replicated on this node. If the cluster is configured with a narrower - // REPLICATION_DATABASES list that excludes `system`, peers won't see the - // hdb_deployment row and falling back to sending req.payload through the operation - // body is the only viable path. + // The payload-via-replicated-row path depends on `system` actually replicating on this node. const systemReplicated = isSystemDatabaseReplicated(); - let extractionPayload = req.payload; - // Bounded ring buffer of install stdout/stderr so a non-SSE caller sees the tail - // in the thrown error. SSE callers still stream every line live. + // Bounded ring buffer of install stdout/stderr so a non-SSE caller sees the tail in the thrown + // error. SSE callers still stream every line live. const installCapture = createInstallCapture(); try { - // On the origin, tee the tarball (Buffer or Readable from the multipart parser) - // through a hash-and-size tap into the row's payload_blob, then re-source extraction - // from the persisted blob. When `system` replicates, the blob becomes the channel - // peers read from; when it doesn't, the blob stays local for audit and rollback. - if (recorder && req.payload != null) { - await recorder.ingestPayload(req.payload); - extractionPayload = recorder.row.payload_blob.stream(); - } else if (isReplicatedExecution && req.payload == null && !req.package) { - // Peer received a replicated deploy without a payload — read the tarball from - // the replicated hdb_deployment row's payload_blob. Blob.stream() blocks on - // in-flight BLOB_CHUNK writes until the chunks land. If the row never arrives - // within the timeout, peer records a failure and origin sees it in peer_results. - // The wait budget defaults to 120s but is overridable per-deploy via - // `deployment_timeout` (ms) for clusters where the system-table channel is - // heavily backlogged (harper-pro#402). - const row = await awaitDeploymentRow(req._deploymentId, { timeoutMs: req.deployment_timeout }); - extractionPayload = row.payload_blob.stream(); - } - - // Resolve credential references into concrete tokens for this node's npm pack/install - // (a no-custody literal-token fallback passes through unchanged). On a peer running a - // replicated deploy, the referenced hdb_secret row may arrive just behind the deploy op, so - // allow a bounded grace period (same budget as the payload-row wait) for it to replicate in. - let credentialsWaitMs = 0; - if (isReplicatedExecution) { - const requested = Number(req.deployment_timeout); - credentialsWaitMs = Number.isFinite(requested) && requested >= 0 ? requested : DEFAULT_AWAIT_ROW_TIMEOUT_MS; - } - const resolvedCredentials = await resolveCredentials(req.credentials, req.project, { - waitMs: credentialsWaitMs, - }); - - const application = new Application({ - name: req.project, - payload: extractionPayload, - packageIdentifier: req.package, - install: { - command: req.install_command, - timeout: req.install_timeout, - allowInstallScripts: req.install_allow_scripts, - }, - // Tee each install line into both the capture buffer (for the thrown-error - // fallback) and the SSE channel (when a caller is streaming). Peers have no - // emitter, so their install output goes to the local logger and the buffer only. - onInstallLine: (manager, stream, line) => { - installCapture.push(manager, stream, line); - if (emitter) emit('install', { manager, stream, line }); - }, - // Deploy credentials (already resolved above), used here for this node's npm pack/install: - // registry entries via a transient .npmrc, git-host entries via the in-memory credential - // socket the clone spawn talks to. - credentials: resolvedCredentials, + const extractionPayload = await sourceExtractionPayload({ req, recorder, isReplicatedExecution }); + const resolvedCredentials = await resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution }); + const application = buildDeployApplication({ + req, + extractionPayload, + resolvedCredentials, + installCapture, + emitter, + emit, }); // Reduce req.credentials to references only (never a token) before it can reach an error/log // path or replication: references are what peers resolve from their own replicated hdb_secret // copy; a no-custody literal token is dropped entirely (peers fall back to their fabric-injected - // NPM_CONFIG_USERCONFIG, as before). This also fixes the prior success-only strip that leaked a - // literal token on a prepare/load failure. + // NPM_CONFIG_USERCONFIG, as before). if (credentialReferences.length) req.credentials = credentialReferences; else delete req.credentials; @@ -515,83 +479,33 @@ async function deployComponent(req) { await prepareApplication(application); emit('phase', { phase: 'prepare', status: 'done' }); - // now we attempt to actually load the component in case there is - // an error we can immediately detect and report, but app code should not run on the main thread - if (!isMainThread && !process.env.HARPER_SAFE_MODE) { - const pseudoResources = new Resources(); - pseudoResources.isWorker = true; - - const componentLoader = require('./componentLoader.ts').default || require('./componentLoader.ts'); - const { trackScopeClose } = require('./scopeShutdown.ts'); - let lastError; - componentLoader.setErrorReporter((error) => (lastError = error)); - emit('phase', { phase: 'load', status: 'start' }); - // This load exists only to surface load-time errors early; the Scopes it creates are - // throwaway. They are collected (instead of registered for worker-shutdown auto-close) so we - // can close them here once validation completes — otherwise each deploy leaks the Scope's - // deploy-lifecycle listeners on this worker, eventually tripping MaxListenersExceededWarning - // (#1462). - const validationScopes = new Set(); - const validation = (async () => { - try { - await componentLoader.loadComponent(application.dirPath, pseudoResources, undefined, { - collectScopes: validationScopes, - }); - } finally { - const closeResults = await Promise.allSettled(Array.from(validationScopes, (scope) => scope.close())); - for (const result of closeResults) { - if (result.status === 'rejected') log.warn('Failed to close a deploy-validation Scope', result.reason); - } - } - })(); - // Track the load+close so a concurrent worker shutdown waits for these scopes to finish - // disposing — a plugin may start a native runtime in handleApplication — before realExit. - trackScopeClose(validation); - await validation; - emit('phase', { phase: 'load', status: 'done' }); - - if (lastError) throw lastError; - } + // Load the component to surface load-time errors early (throwaway scopes; see loadValidateComponent). + await loadValidateComponent({ dirPath: application.dirPath, emit }); + const rollingRestart = req.restart === 'rolling'; // if doing a rolling restart set restart to false so that other nodes don't also restart. req.restart = rollingRestart ? false : req.restart; - // ProgressEmitter holds function listeners that can't survive the replication - // channel's serialization; strip it unconditionally. + // ProgressEmitter holds function listeners that can't survive the replication channel's + // serialization; strip it unconditionally. delete req.progress; - // req.credentials was already deleted immediately after the Application ctor (above) so the - // token never reaches the replication channel or a peer's operation log; peers authenticate - // against the private registry via their own fabric-injected NPM_CONFIG_USERCONFIG on reinstall. if (systemReplicated && recorder) { - // The hdb_deployment row + payload_blob will reach peers via table replication, - // so peers can look up the payload by deployment_id. Drop req.payload to keep - // the operation body small (the operations channel has frame-size limits the - // blob-replication channel doesn't share). _deploymentId is the handoff that - // lets peers find the replicated row. + // The hdb_deployment row + payload_blob reach peers via table replication, so peers look up + // the payload by deployment_id. Drop req.payload to keep the operation body small. delete req.payload; } - // As each peer settles, update the origin row so observers polling get_deployment - // see per-peer progress in real time rather than only at the aggregate end. - // replicateOperation in harper-pro accepts an optional onPeerResult callback that - // fires per peer; callers without the callback (older replicator) fall back to - // the aggregate response.replicated below. const onPeerResult = recorder ? (result) => { recorder.recordPeer(result); emit('peer', result); } : undefined; - // Seal the recorder before the replicate phase so the row's terminal write (finish()) - // isn't part of the tight put burst that can commit out of order on a peer and revert - // it (harperdb/harper#1170). onPeerResult/peer_results accumulate in memory and land in - // finish()'s single write; live SSE 'peer' events still fire below. + // Seal before the replicate phase so the row's terminal write (finish()) isn't part of the tight + // put burst that can commit out of order on a peer and revert it (harperdb/harper#1170). recorder?.seal(); emit('phase', { phase: 'replicate', status: 'start' }); let response = await server.replication.replicateOperation(req, { onPeerResult }); emit('phase', { phase: 'replicate', status: 'done' }); if (recorder && response?.replicated) { - // Fallback path for replicators that don't honor onPeerResult: re-record the - // aggregate. recordPeer's upsert-by-node-name semantics make this idempotent - // when the per-peer callback already fired for these. recorder.recordPeers(response.replicated); } if (req.restart === true) { @@ -613,21 +527,15 @@ async function deployComponent(req) { response.message = `Successfully deployed: ${application.name}, restarting Harper`; } else response.message = `Successfully deployed: ${application.name}`; - // Replication failures don't reject replicateOperation — they surface as 'failed' - // entries in peer_results. By default, treat any failed peer as an overall deploy - // failure so the operation returns a non-2xx status (and the CLI a non-zero exit - // code). The component is already deployed — and, if requested, restarted — on this - // origin node; the failure signals that one or more peers did not receive it. Pass - // ignore_replication_errors: true for best-effort deploys to partially-available clusters. + // Replication failures don't reject replicateOperation — they surface as 'failed' entries in + // peer_results. By default, treat any failed peer as an overall deploy failure so the operation + // returns a non-2xx status. Pass ignore_replication_errors: true for best-effort deploys. if (recorder && !req.ignore_replication_errors) { const failedPeers = recorder.getFailedPeers(); if (failedPeers.length > 0) { - const detail = failedPeers - .map((peer) => `${peer.node ?? 'unknown'} (${peer.error?.message ?? 'unknown error'})`) - .join(', '); throw new ServerError( `Component '${application.name}' was deployed on the origin node but failed to replicate to ` + - `${failedPeers.length} of ${recorder.row.peer_results.length} peer node(s): ${detail}. ` + + `${failedPeers.length} of ${recorder.row.peer_results.length} peer node(s): ${describePeers(failedPeers)}. ` + `See deployment ${recorder.deploymentId} (get_deployment) for details, or pass ` + `ignore_replication_errors: true to treat replication failures as non-fatal.` ); @@ -636,69 +544,541 @@ async function deployComponent(req) { if (recorder) { response.deployment_id = recorder.deploymentId; - // Reclaim the payload tarball for large deploys: every peer has now installed from - // the blob (replicateOperation resolved) and the origin no longer needs it. Dropping - // the reference before finish() folds the null into the single terminal write, which - // unlinks the file locally and replicates the null so peers drop their copies too. - // Metadata (size, hash, event_log) is retained for the audit trail. Two guards keep - // the tarball when it's still the artifact you'd debug or retry with: failed deploys - // don't reach this branch, and a deploy that reached here only because - // ignore_replication_errors masked failed peers keeps its payload for those peers. - const payloadSize = recorder.row.payload_size; - const retentionMaxSize = getPayloadRetentionMaxSize(); - if (typeof payloadSize === 'number' && payloadSize > retentionMaxSize && recorder.getFailedPeers().length === 0) { - const freed = recorder.dropPayload(); - if (freed > 0) emit('payload_dropped', { payload_size: freed, max_size: retentionMaxSize }); - } + maybeReclaimPayload(recorder, emit); emit('phase', { phase: 'success', status: 'done' }); await recorder.finish('success'); } return response; } catch (err) { - // Pack phase, install output tail, and deployment_id into http_resp_msg so the - // Fastify error handler forwards them verbatim (it does when http_resp_msg is an - // object). Non-SSE callers see structured failure detail; SSE callers already - // got the same data live via emit('error', ...) below. - const capture = installCapture.snapshot(); - const phase = recorder?.row.phase; - const baseMessage = err?.message ?? String(err); - const structured = { error: baseMessage }; - if (phase) structured.phase = phase; - if (capture.lines.length > 0) structured.install_output = capture; - if (recorder?.deploymentId) structured.deployment_id = recorder.deploymentId; - // Surface failed peer outcomes so callers see which nodes the deploy did not reach - // without a second get_deployment round-trip. Populated for replication failures (the - // throw above) and any other failure that occurred after peers reported. Carried on - // both the structured non-SSE body and the SSE 'error' event below so the two transports - // stay symmetric (the CLI uses SSE for deploy_component). - const failedPeers = recorder?.getFailedPeers() ?? []; - if (failedPeers.length > 0) structured.failed_peers = failedPeers; - - // Wrap as a ServerError so the Fastify error handler picks a 500 by default; preserve - // an upstream statusCode (e.g. a ClientError from payload validation) if present. - const outErr = new ServerError(baseMessage, err?.statusCode); - outErr.http_resp_msg = structured; - - emit('error', { - message: baseMessage, - code: outErr?.statusCode ?? err?.code, - phase, - install_output: capture.lines.length > 0 ? capture : undefined, - deployment_id: recorder?.deploymentId, - failed_peers: failedPeers.length > 0 ? failedPeers : undefined, + throw await finalizeDeployFailure({ err, recorder, installCapture, emit }); + } +} + +/** + * Two-phase deploy orchestrator (origin node). Builds the incoming version into staging on every + * node (phase 1, stage_component), gates on every node succeeding, then atomically swaps it live on + * every node (phase 2, activate_component). The live component on every node is untouched until the + * whole cluster has the bits in place, and the go-live window is just the swap + restart. + */ +async function deployComponentTwoPhase(req, credentialReferences) { + const { resolveCredentials } = require('./secretOperations.ts'); + // Fail fast on a protected core name before we create any state or touch the cluster. + if (req.package) assertNotProtectedCoreComponent(req.project, req.force); + + // The origin always records (a two-phase origin is never itself a replicated execution). + const emitter = req.progress ?? new ProgressEmitter(); + if (!req.progress) req.progress = emitter; + const recorder = await DeploymentRecorder.create({ + project: req.project, + package_identifier: req.package ?? null, + user: req.hdb_user?.username, + restart_mode: req.restart === 'rolling' ? 'rolling' : req.restart ? 'immediate' : null, + credentials: credentialReferences.length ? credentialReferences : null, + emitter, + }); + req._deploymentId = recorder.deploymentId; + const emit = (event, data) => emitter.emit(event, data); + const installCapture = createInstallCapture(); + const rollingRestart = req.restart === 'rolling'; + const recordPeer = (result) => { + recorder.recordPeer(result); + emit('peer', result); + }; + let application; + + try { + // Tee the payload into the row's blob (the replication channel peers read from) and re-source + // extraction from it. Two-phase requires systemReplicated, so peers always fetch from the row. + const extractionPayload = await sourceExtractionPayload({ req, recorder, isReplicatedExecution: false }); + const resolvedCredentials = await resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution: false }); + // stagingId = deployment id so peers (which build a fresh Application per sub-op) resolve the + // same staging path this deployment used. + application = buildDeployApplication({ + req, + extractionPayload, + resolvedCredentials, + stagingId: recorder.deploymentId, + installCapture, + emitter, + emit, + }); + // Strip tokens from req before any replication/log path; keep references (peers resolve those + // from their own hdb_secret copy). Strip the emitter and payload too — peers read the payload + // from the replicated row, keeping the sub-operation bodies small. + if (credentialReferences.length) req.credentials = credentialReferences; + else delete req.credentials; + delete req.progress; + delete req.payload; + + // ===== PHASE 1: STAGE — build on every node; nothing goes live. ===== + emit('phase', { phase: 'stage', status: 'start' }); + await stageApplication(application); + const stageOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.STAGE_COMPONENT); + const stageResp = await server.replication.replicateOperation(stageOp, { onPeerResult: recordPeer }); + if (stageResp?.replicated) recorder.recordPeers(stageResp.replicated); + emit('phase', { phase: 'stage', status: 'done' }); + + // ---- Cluster barrier: every node must have staged before ANY node activates. ---- + if (!req.ignore_replication_errors) { + const failed = recorder.getFailedPeers(); + if (failed.length > 0) { + await discardStagedApplication(application).catch(() => {}); + throw new ServerError( + `Component '${req.project}' failed to stage on ${failed.length} of ` + + `${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. No node was activated — ` + + `the live component is unchanged everywhere. See deployment ${recorder.deploymentId} (get_deployment), ` + + `or pass ignore_replication_errors: true to activate the nodes that did stage.` + ); + } + } + + // Validate the staged build before go-live (loads from the staging dir; see loadValidateComponent). + await loadValidateComponent({ dirPath: application.buildDirPath, emit }); + + // ===== PHASE 2: ACTIVATE — atomic swap + restart, now the bits are in place everywhere. ===== + // Persist root config now (not before staging) so a `package` config never points at a version + // that failed to stage. + if (req.package) await writeComponentRootConfig(req, credentialReferences); + // if doing a rolling restart set restart to false so peers don't also immediately restart. + req.restart = rollingRestart ? false : req.restart; + + emit('phase', { phase: 'activate', status: 'start' }); + await activateApplication(application); + const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, { + restart: req.restart, + deploymentId: recorder.deploymentId, + }); + // Seal before the activate replicate burst (same #1170 rationale as one-shot). + recorder.seal(); + const activateResp = await server.replication.replicateOperation(activateOp, { onPeerResult: recordPeer }); + emit('phase', { phase: 'activate', status: 'done' }); + let response = activateResp && typeof activateResp === 'object' ? activateResp : { message: '' }; + if (activateResp?.replicated) recorder.recordPeers(activateResp.replicated); + + // ---- Restart on the origin. ---- + if (req.restart === true) { + emit('phase', { phase: 'restart', status: 'start' }); + manageThreads.restartWorkers('http'); + emit('phase', { phase: 'restart', status: 'done' }); + response.message = `Successfully deployed: ${application.name}, restarting Harper`; + } else if (rollingRestart) { + const serverUtilities = require('../server/serverHelpers/serverUtilities.ts'); + emit('phase', { phase: 'restart', status: 'start' }); + const jobResponse = await serverUtilities.executeJob({ + operation: 'restart_service', + service: 'http', + replicated: true, + }); + emit('phase', { phase: 'restart', status: 'done' }); + response.restartJobId = jobResponse.job_id; + response.message = `Successfully deployed: ${application.name}, restarting Harper`; + } else response.message = `Successfully deployed: ${application.name}`; + + // ---- Activate gate: rare, but a node can stage OK and then fail the swap. ---- + if (!req.ignore_replication_errors) { + const failed = recorder.getFailedPeers(); + if (failed.length > 0) { + throw new ServerError( + `Component '${application.name}' was activated on the origin but failed to activate on ${failed.length} ` + + `of ${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. Those nodes have the staged ` + + `build but did not go live. See deployment ${recorder.deploymentId} (get_deployment), or pass ` + + `ignore_replication_errors: true.` + ); + } + } + + response.deployment_id = recorder.deploymentId; + maybeReclaimPayload(recorder, emit); + emit('phase', { phase: 'success', status: 'done' }); + await recorder.finish('success'); + return response; + } catch (err) { + // An aborted deploy leaves the live component untouched; drop any staged build so it can't leak. + if (application) await discardStagedApplication(application).catch(() => {}); + throw await finalizeDeployFailure({ err, recorder, installCapture, emit }); + } +} + +/** + * stage_component — phase 1 of a two-phase deploy, as a first-class operation. Builds the incoming + * version into the hidden staging directory on this node (and, when invoked directly rather than via + * replication, replicates the stage across the cluster). Never writes the live component directory, + * writes no root config, and never restarts — so it is safe to run cluster-wide and gate on. + * + * Reached three ways: directly by an operator (stage now, activate later), by a peer replaying a + * replicated stage (`_deploymentId` set), and indirectly — deploy_component drives staging itself, + * so it does not call this handler. + */ +async function stageComponent(req) { + if (req.project) { + req.project = path.parse(req.project).name; + } else if (req.package) { + req.project = getProjectNameFromPackage(req.package); + } + const validation = validator.stageComponentValidator(req); + if (validation) throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); + + const { ingestCredentials, resolveCredentials } = require('./secretOperations.ts'); + req.credentials = await ingestCredentials(req, req.credentials, req.project); + const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); + if (req.package) assertNotProtectedCoreComponent(req.project, req.force); + + const isReplicatedExecution = typeof req._deploymentId === 'string'; + const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); + if (emitter && !req.progress) req.progress = emitter; + // Standalone stage records so it has a deployment_id + payload row for peers to fetch; a peer + // replaying the stage skips recording (the origin owns the row). + const recorder = isReplicatedExecution + ? null + : await DeploymentRecorder.create({ + project: req.project, + package_identifier: req.package ?? null, + user: req.hdb_user?.username, + restart_mode: null, + credentials: credentialReferences.length ? credentialReferences : null, + emitter, + }); + if (recorder) req._deploymentId = recorder.deploymentId; + const emit = (event, data) => emitter?.emit(event, data); + const installCapture = createInstallCapture(); + let application; + + try { + const extractionPayload = await sourceExtractionPayload({ req, recorder, isReplicatedExecution }); + const resolvedCredentials = await resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution }); + application = buildDeployApplication({ + req, + extractionPayload, + resolvedCredentials, + stagingId: req._deploymentId, + installCapture, + emitter, + emit, }); - // Record the terminal failure, but never let a finish() write error (full disk, lock, - // dropped system table) mask the actual deploy failure — outErr carries the phase, - // install output, and failed_peers the caller needs. + if (credentialReferences.length) req.credentials = credentialReferences; + else delete req.credentials; + delete req.progress; + + emit('phase', { phase: 'stage', status: 'start' }); + await stageApplication(application); + emit('phase', { phase: 'stage', status: 'done' }); + + const response = { message: `Staged component: ${req.project}`, project: req.project, staged: true }; if (recorder) { - try { - await recorder.finish('failed', err); - } catch (finishErr) { - log.warn('Failed to record deployment failure row', finishErr); + response.deployment_id = recorder.deploymentId; + // Replicate staging to peers so the bits land cluster-wide. Keep the payload in the body only + // when the row can't carry it (system not replicated on this node). + if (isSystemDatabaseReplicated()) delete req.payload; + const stageOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.STAGE_COMPONENT, { + includePayload: !isSystemDatabaseReplicated(), + }); + recorder.seal(); + const rep = await server.replication.replicateOperation(stageOp, { + onPeerResult: (result) => { + recorder.recordPeer(result); + emit('peer', result); + }, + }); + if (rep?.replicated) recorder.recordPeers(rep.replicated); + if (!req.ignore_replication_errors) { + const failed = recorder.getFailedPeers(); + if (failed.length > 0) { + throw new ServerError( + `Component '${req.project}' failed to stage on ${failed.length} of ` + + `${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. ` + + `See deployment ${recorder.deploymentId} (get_deployment).` + ); + } + } + // Leave the row in a 'staged' resting state — the build exists cluster-wide but nothing is + // live yet; a subsequent activate_component (or the caller) takes it live. + emit('phase', { phase: 'staged', status: 'done' }); + await recorder.finish('staged'); + } + return response; + } catch (err) { + if (application) await discardStagedApplication(application).catch(() => {}); + throw await finalizeDeployFailure({ err, recorder, installCapture, emit }); + } +} + +/** + * activate_component — phase 2 of a two-phase deploy, as a first-class operation. Atomically swaps a + * previously-staged build (identified by `deployment_id`) into the live component directory, persists + * the component's root config for a `package` deploy, and restarts as requested. When invoked + * directly it also replicates the activation across the cluster. + * + * Reached two ways: directly by an operator to take a prior stage live, and by a peer replaying a + * replicated activate (`_deploymentId` set). deploy_component drives activation itself. + */ +async function activateComponent(req) { + if (req.project) req.project = path.parse(req.project).name; + const validation = validator.activateComponentValidator(req); + if (validation) throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); + + const isReplicatedExecution = typeof req._deploymentId === 'string'; + const stagingId = req._deploymentId ?? req.deployment_id; + if (!stagingId) { + throw handleHDBError( + new Error(), + `'deployment_id' is required to activate a staged component`, + HTTP_STATUS_CODES.BAD_REQUEST + ); + } + if (req.package) assertNotProtectedCoreComponent(req.project, req.force); + const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); + + const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); + const emit = (event, data) => emitter?.emit(event, data); + const rollingRestart = req.restart === 'rolling'; + // Reconstruct the Application against the staged build. Only name + stagingId are needed to locate + // and swap it — the staged directory already holds the fully-installed incoming version. + const application = new Application({ name: req.project, packageIdentifier: req.package, stagingId }); + + emit('phase', { phase: 'activate', status: 'start' }); + await activateApplication(application); + emit('phase', { phase: 'activate', status: 'done' }); + + // Persist root config now that the component is live (package deploys, on every node). + if (req.package) await writeComponentRootConfig(req, credentialReferences); + + const response = { message: `Activated component: ${req.project}`, project: req.project, activated: true }; + + // Replicate the activation to peers (direct invocation only; a peer replaying an activate must not + // re-fan it out). + if (!isReplicatedExecution) { + delete req.progress; + const restartForPeers = rollingRestart ? false : req.restart; + const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, { + restart: restartForPeers, + deploymentId: stagingId, + }); + const rep = await server.replication.replicateOperation(activateOp, {}); + if (rep?.replicated) response.replicated = rep.replicated; + } + + // Restart on this node. A peer replaying an immediate-restart activate restarts locally; the + // rolling path is driven only by the direct invoker via a replicated restart_service job. + if (req.restart === true) { + emit('phase', { phase: 'restart', status: 'start' }); + manageThreads.restartWorkers('http'); + emit('phase', { phase: 'restart', status: 'done' }); + response.message = `Activated component: ${req.project}, restarting Harper`; + } else if (rollingRestart && !isReplicatedExecution) { + const serverUtilities = require('../server/serverHelpers/serverUtilities.ts'); + emit('phase', { phase: 'restart', status: 'start' }); + const jobResponse = await serverUtilities.executeJob({ + operation: 'restart_service', + service: 'http', + replicated: true, + }); + emit('phase', { phase: 'restart', status: 'done' }); + response.restartJobId = jobResponse.job_id; + response.message = `Activated component: ${req.project}, restarting Harper`; + } + return response; +} + +// ———————————————————————————————————————————————————————————————————————————— +// Shared deploy-family helpers (used by deploy_component, stage_component, activate_component). +// ———————————————————————————————————————————————————————————————————————————— + +// Reject deploying over a protected core component name unless force is set. Lazy-loads +// componentLoader to avoid a circular dependency. +function assertNotProtectedCoreComponent(project, force) { + const { TRUSTED_RESOURCE_PLUGINS } = require('./componentLoader.ts'); + if (TRUSTED_RESOURCE_PLUGINS[project] && !force) { + throw handleHDBError( + new Error(), + `Cannot deploy component with name '${project}': this is a protected core component name. Use force: true to overwrite.`, + HTTP_STATUS_CODES.CONFLICT + ); + } +} + +// Persist a `package` deploy's entry into root config so every cold install (reboot, new peer, +// rollback) reinstalls it. In two-phase this runs at activation, once the bits are staged everywhere. +async function writeComponentRootConfig(req, credentialReferences) { + assertNotProtectedCoreComponent(req.project, req.force); + const applicationConfig = { package: req.package }; + // Avoid writing an empty `install:` block + if (req.install_command || req.install_timeout || req.install_allow_scripts !== undefined) { + applicationConfig.install = { + command: req.install_command, + timeout: req.install_timeout, + allowInstallScripts: req.install_allow_scripts, + }; + } + if (req.urlPath !== undefined) applicationConfig.urlPath = req.urlPath; + // Persist credential references (never tokens) so every cold install re-resolves from the store. + if (credentialReferences.length) applicationConfig.credentials = credentialReferences; + await configUtils.addConfig(req.project, applicationConfig); +} + +// Resolve the tarball to extract from. On the origin, tee req.payload into the row's blob (the +// channel peers read from) and re-source extraction from the persisted blob. On a peer replaying a +// deploy without a payload, read the tarball from the replicated row's blob (bounded wait). +async function sourceExtractionPayload({ req, recorder, isReplicatedExecution }) { + if (recorder && req.payload != null) { + await recorder.ingestPayload(req.payload); + return recorder.row.payload_blob.stream(); + } + if (isReplicatedExecution && req.payload == null && !req.package) { + // Blob.stream() blocks on in-flight BLOB_CHUNK writes until the chunks land. If the row never + // arrives within the timeout, the peer records a failure and the origin sees it in peer_results. + const row = await awaitDeploymentRow(req._deploymentId, { timeoutMs: req.deployment_timeout }); + return row.payload_blob.stream(); + } + return req.payload; +} + +// Resolve credential references into concrete tokens for this node's npm pack/install. On a peer, the +// referenced hdb_secret row may arrive just behind the deploy op, so allow a bounded grace period. +async function resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution }) { + let credentialsWaitMs = 0; + if (isReplicatedExecution) { + const requested = Number(req.deployment_timeout); + credentialsWaitMs = Number.isFinite(requested) && requested >= 0 ? requested : DEFAULT_AWAIT_ROW_TIMEOUT_MS; + } + return resolveCredentials(req.credentials, req.project, { waitMs: credentialsWaitMs }); +} + +// Construct the Application for a deploy, teeing each install line into the capture buffer (for the +// thrown-error tail) and the SSE channel (when a caller is streaming). +function buildDeployApplication({ + req, + extractionPayload, + resolvedCredentials, + stagingId, + installCapture, + emitter, + emit, +}) { + return new Application({ + name: req.project, + payload: extractionPayload, + packageIdentifier: req.package, + install: { + command: req.install_command, + timeout: req.install_timeout, + allowInstallScripts: req.install_allow_scripts, + }, + onInstallLine: (manager, stream, line) => { + installCapture.push(manager, stream, line); + if (emitter) emit('install', { manager, stream, line }); + }, + credentials: resolvedCredentials, + stagingId, + }); +} + +// Load a component directory to surface load-time errors early (throwaway scopes). No-op on the main +// thread or in safe mode. In two-phase this loads the STAGED directory before go-live; in one-shot it +// loads the live directory after in-place prepare. +async function loadValidateComponent({ dirPath, emit }) { + if (isMainThread || process.env.HARPER_SAFE_MODE) return; + const pseudoResources = new Resources(); + pseudoResources.isWorker = true; + + const componentLoader = require('./componentLoader.ts').default || require('./componentLoader.ts'); + const { trackScopeClose } = require('./scopeShutdown.ts'); + let lastError; + componentLoader.setErrorReporter((error) => (lastError = error)); + emit('phase', { phase: 'load', status: 'start' }); + // The Scopes this load creates are throwaway. Collect them (instead of registering for + // worker-shutdown auto-close) so we can close them here once validation completes — otherwise each + // deploy leaks the Scope's deploy-lifecycle listeners on this worker (#1462). + const validationScopes = new Set(); + const validation = (async () => { + try { + await componentLoader.loadComponent(dirPath, pseudoResources, undefined, { collectScopes: validationScopes }); + } finally { + const closeResults = await Promise.allSettled(Array.from(validationScopes, (scope) => scope.close())); + for (const result of closeResults) { + if (result.status === 'rejected') log.warn('Failed to close a deploy-validation Scope', result.reason); } } - throw outErr; + })(); + // Track the load+close so a concurrent worker shutdown waits for these scopes to finish disposing. + trackScopeClose(validation); + await validation; + emit('phase', { phase: 'load', status: 'done' }); + if (lastError) throw lastError; +} + +// Build a replicated sub-operation body (stage_component / activate_component) from the deploy +// request. Carries only what a peer needs: project, the deployment id (correlation + payload lookup + +// staging id), the build/config inputs, and credential REFERENCES (tokens are already stripped). +function buildReplicatedSubOp(req, operation, { includePayload = false, restart, deploymentId } = {}) { + const op = { operation, project: req.project, _deploymentId: deploymentId ?? req._deploymentId }; + if (req.package) op.package = req.package; + if (req.install_command != null) op.install_command = req.install_command; + if (req.install_timeout != null) op.install_timeout = req.install_timeout; + if (req.install_allow_scripts !== undefined) op.install_allow_scripts = req.install_allow_scripts; + if (req.deployment_timeout != null) op.deployment_timeout = req.deployment_timeout; + if (req.urlPath !== undefined) op.urlPath = req.urlPath; + if (req.force !== undefined) op.force = req.force; + if (req.ignore_replication_errors !== undefined) op.ignore_replication_errors = req.ignore_replication_errors; + if (Array.isArray(req.credentials) && req.credentials.length) op.credentials = req.credentials; + if (includePayload && req.payload != null) op.payload = req.payload; + if (restart !== undefined) op.restart = restart; + return op; +} + +// Render failed peer outcomes as "node (error)" for an operator-facing error message. +function describePeers(failedPeers) { + return failedPeers.map((peer) => `${peer.node ?? 'unknown'} (${peer.error?.message ?? 'unknown error'})`).join(', '); +} + +// Reclaim the payload tarball for large deploys once every peer has installed from the blob. Dropping +// the reference before finish() folds the null into the single terminal write, which unlinks the file +// locally and replicates the null so peers drop their copies too. Metadata is retained. Kept when a +// peer failed (still the retry artifact) or the payload is small. +function maybeReclaimPayload(recorder, emit) { + const payloadSize = recorder.row.payload_size; + const retentionMaxSize = getPayloadRetentionMaxSize(); + if (typeof payloadSize === 'number' && payloadSize > retentionMaxSize && recorder.getFailedPeers().length === 0) { + const freed = recorder.dropPayload(); + if (freed > 0) emit('payload_dropped', { payload_size: freed, max_size: retentionMaxSize }); + } +} + +// Build the structured failure (phase, install-output tail, deployment_id, failed peers) that the +// Fastify error handler forwards verbatim, emit the matching SSE 'error' event so the two transports +// stay symmetric, and record the terminal failure row. Returns the ServerError to throw. +async function finalizeDeployFailure({ err, recorder, installCapture, emit }) { + const capture = installCapture.snapshot(); + const phase = recorder?.row.phase; + const baseMessage = err?.message ?? String(err); + const structured = { error: baseMessage }; + if (phase) structured.phase = phase; + if (capture.lines.length > 0) structured.install_output = capture; + if (recorder?.deploymentId) structured.deployment_id = recorder.deploymentId; + const failedPeers = recorder?.getFailedPeers() ?? []; + if (failedPeers.length > 0) structured.failed_peers = failedPeers; + + // Wrap as a ServerError so the Fastify error handler picks a 500 by default; preserve an upstream + // statusCode (e.g. a ClientError from payload validation) if present. + const outErr = new ServerError(baseMessage, err?.statusCode); + outErr.http_resp_msg = structured; + + emit('error', { + message: baseMessage, + code: outErr?.statusCode ?? err?.code, + phase, + install_output: capture.lines.length > 0 ? capture : undefined, + deployment_id: recorder?.deploymentId, + failed_peers: failedPeers.length > 0 ? failedPeers : undefined, + }); + // Record the terminal failure, but never let a finish() write error mask the actual deploy failure. + if (recorder) { + try { + await recorder.finish('failed', err); + } catch (finishErr) { + log.warn('Failed to record deployment failure row', finishErr); + } } + return outErr; } // Ring buffer of install stdout/stderr lines, capped by both line count and bytes so @@ -796,7 +1176,7 @@ async function getComponents() { const list = await fs.readdir(dir, { withFileTypes: true }); for (let item of list) { const itemName = item.name; - if (itemName === 'node_modules' || itemName === ASIDE_STAGING_DIR) continue; + if (itemName === 'node_modules' || itemName === ASIDE_STAGING_DIR || itemName === DEPLOY_STAGING_DIR) continue; const itemPath = path.join(dir, itemName); if (item.isDirectory() || item.isSymbolicLink()) { let res = { @@ -1129,6 +1509,8 @@ exports.addComponent = addComponent; exports.dropCustomFunctionProject = dropCustomFunctionProject; exports.packageComponent = packageComponent; exports.deployComponent = deployComponent; +exports.stageComponent = stageComponent; +exports.activateComponent = activateComponent; exports.getComponents = getComponents; exports.getComponentFile = getComponentFile; exports.setComponentFile = setComponentFile; diff --git a/components/operationsValidation.js b/components/operationsValidation.js index b14321d0ed..8a865d37bc 100644 --- a/components/operationsValidation.js +++ b/components/operationsValidation.js @@ -25,6 +25,8 @@ module.exports = { dropCustomFunctionProjectValidator, packageComponentValidator, deployComponentValidator, + stageComponentValidator, + activateComponentValidator, setComponentFileValidator, getComponentFileValidator, dropComponentFileValidator, @@ -434,6 +436,39 @@ const GIT_CREDENTIAL_ENTRY = Joi.object({ .xor('token', 'secret') .unknown(false); +// The kind-heterogeneous deploy credentials array, shared by deploy_component and its two-phase +// sub-operations (stage_component / activate_component) so the three stay in lockstep. An entry's +// kind is implied by its identifying key (`registry` → npm registry auth, otherwise git host auth), +// dispatched here so a malformed entry reports what is actually wrong with it rather than a generic +// "no alternative matched". +const CREDENTIALS_ARRAY_SCHEMA = Joi.array() + .items( + Joi.alternatives().conditional('.registry', { + is: Joi.exist(), + then: REGISTRY_CREDENTIAL_ENTRY, + otherwise: GIT_CREDENTIAL_ENTRY, + }) + ) + .optional(); + +// A component's URL mount path. Rejects `..` so a deploy can't mount a component outside its intended +// path. Shared across deploy_component and the two-phase sub-operations that persist component config. +const URL_PATH_SCHEMA = Joi.string() + .min(1) + .custom((value, helpers) => { + if (value.includes('..')) return helpers.error('any.invalid'); + return value; + }) + .optional() + .messages({ 'any.invalid': 'urlPath must not contain ".."' }); + +// `registryAuth` was the credentials field's name on the 5.2 dev line. Rejected rather than ignored: +// validation allows unknown keys, so a caller still sending it would otherwise get a deploy that +// silently installs with no credentials. Shared so every deploy-family op rejects it identically. +const FORBIDDEN_REGISTRY_AUTH = Joi.any().forbidden().messages({ + 'any.unknown': `'registryAuth' has been renamed to 'credentials'`, +}); + /** * Validate deployComponent requests. * @param req @@ -453,44 +488,80 @@ function deployComponentValidator(req) { deployment_timeout: Joi.number().min(0).optional(), force: Joi.boolean().optional(), ignore_replication_errors: Joi.boolean().optional(), - urlPath: Joi.string() - .min(1) - .custom((value, helpers) => { - if (value.includes('..')) return helpers.error('any.invalid'); - return value; - }) - .optional() - .messages({ 'any.invalid': 'urlPath must not contain ".."' }), - // Deploy credentials. The array is kind-heterogeneous: an entry's kind is implied by its - // identifying key rather than a separate discriminator field, so a new kind is added as - // another item alternative here without reshaping the field. Today: npm registry auth - // (`registry`) and git host auth for a git-reference package (`host`, #1792). - // - // Every kind supplies its credential exactly one of two ways: - // - `token`: a literal token, used only for this node's install and never persisted or - // replicated (stripped from req before replicateOperation). - // - `secret`: the name of an hdb_secret row (#1550); the token is resolved by decrypting - // that row on this node at deploy time, so the credential lives in the secrets store - // (reference, not embed) instead of travelling in the operation body. - // Dispatched on the presence of `registry` rather than tried as alternatives, so a malformed - // entry reports what is actually wrong with it (a newline in the token, an invalid scope) rather - // than a generic "no alternative matched". - credentials: Joi.array() - .items( - Joi.alternatives().conditional('.registry', { - is: Joi.exist(), - then: REGISTRY_CREDENTIAL_ENTRY, - otherwise: GIT_CREDENTIAL_ENTRY, - }) - ) - .optional(), - // `registryAuth` was this field's name on the 5.2 dev line before it grew to carry other - // credential kinds. Rejected rather than ignored: validation allows unknown keys, so a caller - // still sending it would otherwise get a deploy that silently installs with no credentials. - registryAuth: Joi.any().forbidden().messages({ - 'any.unknown': `'registryAuth' has been renamed to 'credentials'`, - }), + // Opt out of the two-phase (stage-then-activate) deploy and use the legacy one-shot path + // instead. Provided as an escape hatch for mixed-version clusters where a peer predates the + // stage_component/activate_component operations. Defaults to two-phase. + two_phase: Joi.boolean().optional(), + urlPath: URL_PATH_SCHEMA, + // Deploy credentials. Each entry is npm registry auth (`registry`) or git host auth (`host`, + // #1792), and supplies its secret either as a literal `token` (used only for this node's + // install, never persisted/replicated) or a `secret` reference to an hdb_secret row (#1550). + credentials: CREDENTIALS_ARRAY_SCHEMA, + registryAuth: FORBIDDEN_REGISTRY_AUTH, }).with('urlPath', 'package'); return validator.validateBySchema(req, deployProjSchema); } + +/** + * Validate stage_component requests — phase 1 of a two-phase deploy. Accepts the same build-time + * inputs as deploy_component (package/payload, install options, credentials) but no go-live controls + * (`restart`), since staging never restarts. `restart` is intentionally absent; a stray one is + * ignored (operations validate with allowUnknown). + * @param req + * @returns {*} + */ +function stageComponentValidator(req) { + const stageSchema = Joi.object({ + project: Joi.string() + .pattern(PROJECT_FILE_NAME_REGEX) + .required() + .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), + package: Joi.string().optional(), + install_command: Joi.string().optional(), + install_timeout: Joi.number().optional(), + install_allow_scripts: Joi.boolean().optional(), + deployment_timeout: Joi.number().min(0).optional(), + force: Joi.boolean().optional(), + // urlPath is not applied at stage time (config is written at activate), but it is accepted and + // carried through so a single request body can flow stage → activate unchanged. + urlPath: URL_PATH_SCHEMA, + credentials: CREDENTIALS_ARRAY_SCHEMA, + registryAuth: FORBIDDEN_REGISTRY_AUTH, + }).with('urlPath', 'package'); + + return validator.validateBySchema(req, stageSchema); +} + +/** + * Validate activate_component requests — phase 2 of a two-phase deploy. Swaps an already-staged + * build (identified by `deployment_id`) into the live path and optionally restarts. Also carries the + * config-persistence inputs (package/install/credentials/urlPath) so a `package` deploy's root config + * is written at go-live rather than before the bits are in place. + * @param req + * @returns {*} + */ +function activateComponentValidator(req) { + const activateSchema = Joi.object({ + project: Joi.string() + .pattern(PROJECT_FILE_NAME_REGEX) + .required() + .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), + // Identifies which staged build to activate. Required for a standalone activate; the + // deploy_component orchestrator supplies it as the deployment id it staged under. + deployment_id: Joi.string().optional(), + package: Joi.string().optional(), + install_command: Joi.string().optional(), + install_timeout: Joi.number().optional(), + install_allow_scripts: Joi.boolean().optional(), + deployment_timeout: Joi.number().min(0).optional(), + restart: Joi.alternatives().try(Joi.boolean(), Joi.string().valid('rolling')).optional(), + force: Joi.boolean().optional(), + ignore_replication_errors: Joi.boolean().optional(), + urlPath: URL_PATH_SCHEMA, + credentials: CREDENTIALS_ARRAY_SCHEMA, + registryAuth: FORBIDDEN_REGISTRY_AUTH, + }).with('urlPath', 'package'); + + return validator.validateBySchema(req, activateSchema); +} diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index c0f132ba40..3b52339b51 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -33,6 +33,8 @@ const { ProgressEmitter, createSSEResponseStream } = require('./progressEmitter. // the client disconnects (the emitter's abort signal), so subscribers see new lines live. const SSE_PROGRESS_OPERATIONS = new Set([ terms.OPERATIONS_ENUM.DEPLOY_COMPONENT, + terms.OPERATIONS_ENUM.STAGE_COMPONENT, + terms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, terms.OPERATIONS_ENUM.GET_DEPLOYMENT, terms.OPERATIONS_ENUM.READ_LOG, ]); diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 694185478e..8bc2d83b6e 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -534,6 +534,14 @@ function initializeOperationFunctionMap(): Map assert.strictEqual(result, undefined, `expected valid, got: ${result && result.message}`); +const rejected = (result) => assert.ok(result, 'expected a validation error'); + +describe('stageComponentValidator', () => { + it('accepts a project-only request', () => { + ok(validator.stageComponentValidator({ project: 'my_app' })); + }); + + it('accepts a package deploy with install options', () => { + ok( + validator.stageComponentValidator({ + project: 'my_app', + package: 'npm:@org/thing', + install_command: 'npm ci', + install_timeout: 60000, + install_allow_scripts: false, + deployment_timeout: 120000, + }) + ); + }); + + it('requires a project', () => { + rejected(validator.stageComponentValidator({ package: 'npm:@org/thing' })); + }); + + it('rejects an invalid project name', () => { + rejected(validator.stageComponentValidator({ project: 'bad/name' })); + }); + + it('rejects a urlPath containing ".."', () => { + rejected(validator.stageComponentValidator({ project: 'my_app', package: 'npm:x', urlPath: '/a/../b' })); + }); +}); + +describe('activateComponentValidator', () => { + it('accepts a project + deployment_id', () => { + ok(validator.activateComponentValidator({ project: 'my_app', deployment_id: 'abc-123' })); + }); + + it('accepts a rolling restart', () => { + ok(validator.activateComponentValidator({ project: 'my_app', deployment_id: 'abc-123', restart: 'rolling' })); + }); + + it('accepts a boolean restart and ignore_replication_errors', () => { + ok( + validator.activateComponentValidator({ + project: 'my_app', + deployment_id: 'abc-123', + restart: true, + ignore_replication_errors: true, + }) + ); + }); + + it('requires a project', () => { + rejected(validator.activateComponentValidator({ deployment_id: 'abc-123' })); + }); + + it('rejects an invalid restart value', () => { + rejected(validator.activateComponentValidator({ project: 'my_app', restart: 'sideways' })); + }); +}); + +describe('deployComponentValidator two_phase flag', () => { + it('accepts two_phase: false (legacy opt-out)', () => { + ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: false })); + }); + + it('accepts two_phase: true', () => { + ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: true })); + }); + + it('rejects a non-boolean two_phase', () => { + rejected(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: 'yes' })); + }); +}); diff --git a/unitTests/components/deployStaging.test.js b/unitTests/components/deployStaging.test.js new file mode 100644 index 0000000000..0998d2e70c --- /dev/null +++ b/unitTests/components/deployStaging.test.js @@ -0,0 +1,171 @@ +'use strict'; + +// Unit tests for the two-phase deploy primitives in components/Application.ts: +// stageApplication (build the incoming version into a hidden staging dir, never touching the live +// path), activateApplication (atomically swap the staged copy into the live path), and +// discardStagedApplication (drop an aborted stage). These exercise the real filesystem — no +// componentLoader, no network — so they run without the private agent dependency. + +const assert = require('node:assert'); +const fs = require('node:fs/promises'); +const { existsSync } = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const zlib = require('node:zlib'); +const tarfs = require('tar-fs'); + +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const { + Application, + stageApplication, + activateApplication, + discardStagedApplication, + DEPLOY_STAGING_DIR, + ASIDE_STAGING_DIR, +} = require('#src/components/Application'); +const { getConfigPath } = require('#src/config/configUtils'); +const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); + +const COMPONENTS_ROOT = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); + +// Pack a directory's CONTENTS into a gzipped tar Buffer, the shape a deploy payload takes. +function packDirectory(dir) { + return new Promise((resolve, reject) => { + const chunks = []; + tarfs + .pack(dir) + .pipe(zlib.createGzip()) + .on('data', (c) => chunks.push(c)) + .on('end', () => resolve(Buffer.concat(chunks))) + .on('error', reject); + }); +} + +// A minimal component source that already contains node_modules, so installApplication short-circuits +// ("already has node_modules; skipping install") and the test needs no npm/network. +async function makeComponentPayload(marker) { + const src = await fs.mkdtemp(path.join(os.tmpdir(), 'harper-stage-src-')); + await fs.writeFile(path.join(src, 'package.json'), JSON.stringify({ name: 'stage-fixture', version: '1.0.0' })); + await fs.writeFile(path.join(src, 'index.js'), `module.exports = ${JSON.stringify(marker)};\n`); + await fs.mkdir(path.join(src, 'node_modules'), { recursive: true }); + await fs.writeFile(path.join(src, 'node_modules', '.marker'), marker); + const payload = await packDirectory(src); + await fs.rm(src, { recursive: true, force: true }); + return payload; +} + +async function readMarker(dir) { + return fs.readFile(path.join(dir, 'index.js'), 'utf8'); +} + +describe('two-phase deploy primitives (stage / activate / discard)', function () { + this.timeout(30_000); + + before(async () => { + await fs.mkdir(COMPONENTS_ROOT, { recursive: true }); + }); + + // Each case uses a unique component name so the tests are order-independent and don't collide. + let counter = 0; + function freshApp(payload) { + const name = `stage_test_${process.pid}_${counter++}`; + return new Application({ name, payload }); + } + + it('stageApplication builds into the hidden staging dir and does NOT touch the live path', async () => { + const app = freshApp(await makeComponentPayload('v1')); + + assert.ok(app.stagingDirPath.includes(DEPLOY_STAGING_DIR), 'staging path is under the staging dir'); + assert.strictEqual(app.buildDirPath, app.dirPath, 'build target defaults to the live dir before staging'); + + const stagedPath = await stageApplication(app); + + assert.strictEqual(stagedPath, app.stagingDirPath); + assert.ok(existsSync(path.join(app.stagingDirPath, 'index.js')), 'component extracted into staging'); + assert.ok(existsSync(path.join(app.stagingDirPath, 'node_modules')), 'node_modules present in staging'); + assert.strictEqual(existsSync(app.dirPath), false, 'live component dir was NOT created by staging'); + + await fs.rm(path.dirname(app.stagingDirPath), { recursive: true, force: true }); + }); + + it('activateApplication swaps the staged copy into the live path atomically', async () => { + const app = freshApp(await makeComponentPayload('v1')); + await stageApplication(app); + await activateApplication(app); + + assert.ok(existsSync(app.dirPath), 'live component dir now exists'); + assert.match(await readMarker(app.dirPath), /v1/, 'live dir holds the staged content'); + assert.strictEqual(existsSync(app.stagingDirPath), false, 'the staged copy was consumed by the swap'); + assert.strictEqual(app.buildDirPath, app.dirPath, 'build target reset to live after activation'); + + await fs.rm(app.dirPath, { recursive: true, force: true }); + }); + + it('stage → activate replaces an existing live version and moves the old one aside', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + const dirPath = path.join(COMPONENTS_ROOT, name); + // Seed an existing live version. + await fs.mkdir(dirPath, { recursive: true }); + await fs.writeFile(path.join(dirPath, 'index.js'), 'module.exports = "OLD";\n'); + await fs.writeFile(path.join(dirPath, 'leftover.txt'), 'from the old version'); + + const app = new Application({ name, payload: await makeComponentPayload('v2') }); + await stageApplication(app); + await activateApplication(app); + + assert.match(await readMarker(dirPath), /v2/, 'live dir now holds the new version'); + assert.strictEqual(existsSync(path.join(dirPath, 'leftover.txt')), false, 'old-version files are gone from live'); + + await fs.rm(dirPath, { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); + + it('discardStagedApplication removes the staging tree and leaves the live path untouched', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + const dirPath = path.join(COMPONENTS_ROOT, name); + await fs.mkdir(dirPath, { recursive: true }); + await fs.writeFile(path.join(dirPath, 'index.js'), 'module.exports = "LIVE";\n'); + + const app = new Application({ name, payload: await makeComponentPayload('v3') }); + await stageApplication(app); + assert.ok(existsSync(app.stagingDirPath), 'staged before discard'); + + await discardStagedApplication(app); + + assert.strictEqual(existsSync(app.stagingDirPath), false, 'staging tree removed'); + assert.match(await readMarker(dirPath), /LIVE/, 'live version untouched by discard'); + + await fs.rm(dirPath, { recursive: true, force: true }); + }); + + it('a failed stage leaves the live path untouched and cleans up its partial staging tree', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + const dirPath = path.join(COMPONENTS_ROOT, name); + await fs.mkdir(dirPath, { recursive: true }); + await fs.writeFile(path.join(dirPath, 'index.js'), 'module.exports = "LIVE";\n'); + + // No payload and no package identifier → extractApplication throws before anything is built. + const app = new Application({ name }); + await assert.rejects(() => stageApplication(app), /payload or package/i); + + assert.strictEqual(existsSync(app.stagingDirPath), false, 'partial staging tree removed on failure'); + assert.match(await readMarker(dirPath), /LIVE/, 'live version untouched by a failed stage'); + assert.strictEqual(app.buildDirPath, app.dirPath, 'build target reset to live after a failed stage'); + + await fs.rm(dirPath, { recursive: true, force: true }); + }); + + it('two independent components stage into non-colliding staging dirs', async () => { + const a = freshApp(await makeComponentPayload('A')); + const b = freshApp(await makeComponentPayload('B')); + await Promise.all([stageApplication(a), stageApplication(b)]); + + assert.notStrictEqual(a.stagingDirPath, b.stagingDirPath); + assert.match(await readMarker(a.stagingDirPath), /A/); + assert.match(await readMarker(b.stagingDirPath), /B/); + + await Promise.all([discardStagedApplication(a), discardStagedApplication(b)]); + }); +}); diff --git a/unitTests/server/fastifyRoutes/operations.test.js b/unitTests/server/fastifyRoutes/operations.test.js index 2643fc8e53..31075fa4bd 100644 --- a/unitTests/server/fastifyRoutes/operations.test.js +++ b/unitTests/server/fastifyRoutes/operations.test.js @@ -520,9 +520,11 @@ describe('Test custom functions operations', () => { // Mock addConfig to prevent actual file writes const addConfigStub = sandbox.stub(configUtils, 'addConfig').resolves(); - // Mock prepareApplication to prevent actual installation - const prepareApplicationStub = sandbox.stub(); - operations.__set__('prepareApplication', prepareApplicationStub); + // Mock the two-phase build/swap primitives to prevent actual install + filesystem work. + const stageApplicationStub = sandbox.stub().resolves('/tmp/staging'); + const activateApplicationStub = sandbox.stub().resolves(); + operations.__set__('stageApplication', stageApplicationStub); + operations.__set__('activateApplication', activateApplicationStub); // This should work - user components can be overwritten without force await operations.deployComponent({ @@ -530,13 +532,14 @@ describe('Test custom functions operations', () => { package: '@org/new-package', }); - // Verify addConfig was called + // Verify addConfig was called (at activation, once the bits are staged) expect(addConfigStub.calledOnce).to.be.true; expect(addConfigStub.firstCall.args[0]).to.equal('existing-component'); expect(addConfigStub.firstCall.args[1].package).to.equal('@org/new-package'); - // Verify prepareApplication was called - expect(prepareApplicationStub.calledOnce).to.be.true; + // Verify the component was staged then activated + expect(stageApplicationStub.calledOnce).to.be.true; + expect(activateApplicationStub.calledOnce).to.be.true; }); it('Test deployComponent allows deploying new component without force flag', async () => { @@ -546,9 +549,11 @@ describe('Test custom functions operations', () => { // Mock addConfig to prevent actual file writes const addConfigStub = sandbox.stub(configUtils, 'addConfig').resolves(); - // Mock prepareApplication to prevent actual installation - const prepareApplicationStub = sandbox.stub(); - operations.__set__('prepareApplication', prepareApplicationStub); + // Mock the two-phase build/swap primitives to prevent actual install + filesystem work. + const stageApplicationStub = sandbox.stub().resolves('/tmp/staging'); + const activateApplicationStub = sandbox.stub().resolves(); + operations.__set__('stageApplication', stageApplicationStub); + operations.__set__('activateApplication', activateApplicationStub); // This should work fine - no component exists yet await operations.deployComponent({ @@ -560,8 +565,9 @@ describe('Test custom functions operations', () => { expect(addConfigStub.calledOnce).to.be.true; expect(addConfigStub.firstCall.args[0]).to.equal('new-component'); - // Verify prepareApplication was called - expect(prepareApplicationStub.calledOnce).to.be.true; + // Verify the component was staged then activated + expect(stageApplicationStub.calledOnce).to.be.true; + expect(activateApplicationStub.calledOnce).to.be.true; }); it('Test deployComponent prevents overwriting core component without force flag', async () => { @@ -592,9 +598,11 @@ describe('Test custom functions operations', () => { // Mock addConfig to prevent actual file writes const addConfigStub = sandbox.stub(configUtils, 'addConfig').resolves(); - // Mock prepareApplication to prevent actual installation - const prepareApplicationStub = sandbox.stub(); - operations.__set__('prepareApplication', prepareApplicationStub); + // Mock the two-phase build/swap primitives to prevent actual install + filesystem work. + const stageApplicationStub = sandbox.stub().resolves('/tmp/staging'); + const activateApplicationStub = sandbox.stub().resolves(); + operations.__set__('stageApplication', stageApplicationStub); + operations.__set__('activateApplication', activateApplicationStub); // This should NOT throw an error because force is true await operations.deployComponent({ @@ -608,8 +616,9 @@ describe('Test custom functions operations', () => { expect(addConfigStub.firstCall.args[0]).to.equal('graphql'); expect(addConfigStub.firstCall.args[1].package).to.equal('@org/override-package'); - // Verify prepareApplication was called - expect(prepareApplicationStub.calledOnce).to.be.true; + // Verify the component was staged then activated + expect(stageApplicationStub.calledOnce).to.be.true; + expect(activateApplicationStub.calledOnce).to.be.true; }); it('Test deployComponent prevents overwriting multiple core component names', async () => { diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index cc708f15f7..c428264be1 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -291,6 +291,12 @@ export const OPERATIONS_ENUM = { DEPLOY_CUSTOM_FUNCTION_PROJECT: 'deploy_custom_function_project', PACKAGE_COMPONENT: 'package_component', DEPLOY_COMPONENT: 'deploy_component', + // Two-phase deploy sub-operations. stage_component builds the incoming version into a hidden + // staging directory cluster-wide (no go-live); activate_component atomically swaps the staged + // copy into the live path and restarts. deploy_component orchestrates the two so existing callers + // are unaffected. See components/Application.ts (stageApplication/activateApplication). + STAGE_COMPONENT: 'stage_component', + ACTIVATE_COMPONENT: 'activate_component', READ_TRANSACTION_LOG: 'read_transaction_log', DELETE_TRANSACTION_LOGS_BEFORE: 'delete_transaction_logs_before', INSTALL_NODE_MODULES: 'install_node_modules', diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index 112ab277e2..dcb70b6c26 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -286,6 +286,8 @@ requiredPermissions.set(functionsOperations.addComponent.name, new (permission a requiredPermissions.set(functionsOperations.dropCustomFunctionProject.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.packageComponent.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.deployComponent.name, new (permission as any)(true, [])); +requiredPermissions.set(functionsOperations.stageComponent.name, new (permission as any)(true, [])); +requiredPermissions.set(functionsOperations.activateComponent.name, new (permission as any)(true, [])); requiredPermissions.set( deploymentOperations.handleListDeployments.name, new (permission as any)(true, [], terms.OPERATIONS_ENUM.LIST_DEPLOYMENTS) From 909f384854d55b103234bcf07ac21855e5610384 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 17 Jul 2026 15:33:48 -0400 Subject: [PATCH 02/22] fix(deploy): staging parent for npm pack cwd, symlink-safe aside, sibling-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// (was /). 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 --- DESIGN.md | 10 +++- components/Application.ts | 44 ++++++++++++---- unitTests/components/deployStaging.test.js | 60 ++++++++++++++++++++++ 3 files changed, 101 insertions(+), 13 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 37fef08b92..eb21179be5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -321,7 +321,7 @@ could leave a peer half-installed after other peers had already restarted onto t request/response contract is unchanged; only the SSE phase names differ (`stage`/`activate` vs the old `prepare`/`replicate`). `two_phase: false` forces the legacy one-shot path. -The staging directory (`.deploy-staging//`) lives **under the components root**, +The staging directory (`.deploy-staging//`) lives **under the components root**, not in `os.tmpdir()`, even though its contents are transient. This is deliberate and load-bearing: the go-live step is `rename(stagingDir, liveDir)`, which is only atomic when both paths share a filesystem. `os.tmpdir()` is frequently a different mount (tmpfs, a separate volume); a cross-device @@ -335,7 +335,13 @@ scoped to `activateApplication`, the only phase that writes the live path. Stagi from the deployment id precisely so `activate_component` (a separate replicated operation, and on peers a separate invocation from `stage_component`) can reconstruct the same path the stage built — peers build a fresh `Application` per sub-operation, so there is no shared in-memory handle to rely -on. `extractApplication`/`installApplication` build into `application.buildDirPath`, which defaults +on. The deployment id sits ABOVE the component name (`…//`, not +`…//`) for two reasons: the leaf directory's basename is then the real component +name, which the pre-go-live validation load needs (`componentLoader` keys the `ApplicationScope` and +status registry off `basename(componentDirectory)`, so a UUID leaf would register the throwaway load +under a bogus name); and each deploy gets its own parent directory, so a parallel or queued deploy of +the same component can never share a directory or have its staged build swept by another's cleanup. +`extractApplication`/`installApplication` build into `application.buildDirPath`, which defaults to the live dir (`dirPath`) — this is what keeps the legacy one-shot path, boot-time `installApplications`, and the direct `extractApplication` callers unchanged — and is repointed at the staging dir only for the duration of a stage. diff --git a/components/Application.ts b/components/Application.ts index d5de3bfe69..dbeef9fdc6 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -18,12 +18,14 @@ import { access, constants, cp, + lstat, mkdir, mkdtemp, readdir, readFile, rename, rm, + rmdir, stat, symlink, writeFile, @@ -438,7 +440,11 @@ export const DEPLOY_STAGING_DIR = '.deploy-staging'; async function moveDirAside(targetDirPath: string): Promise { const asideStagingDir = join(dirname(targetDirPath), ASIDE_STAGING_DIR, basename(targetDirPath)); try { - await access(targetDirPath, constants.F_OK); + // lstat, not access(F_OK): access follows symlinks, so a DANGLING symlink at the path (left by a + // prior `file:`-directory deploy whose target was removed) would report ENOENT and be skipped + // here — then mkdir(targetDirPath) fails EEXIST because the dead link still occupies the path. + // lstat sees the link itself, so we move it aside like any other occupant. + await lstat(targetDirPath); } catch (err) { if (err.code === 'ENOENT') return null; // nothing there to move throw err; @@ -942,12 +948,19 @@ export class Application { return this.#buildDirPath ?? this.dirPath; } - // Hidden, per-deploy staging directory the incoming version is built into before it goes live. - // Deterministic from (component name, stagingId) so `activate_component` can find what - // `stage_component` built. Sits under the components root (dirname(dirPath)) so the go-live - // rename() into `dirPath` stays on one filesystem and is therefore atomic. See DEPLOY_STAGING_DIR. + // Hidden, per-deploy staging directory the incoming version is built into before it goes live: + // `/.deploy-staging//`. Deterministic from (stagingId, component + // name) so `activate_component` can find what `stage_component` built. Sits under the components + // root (dirname(dirPath)) so the go-live rename() into `dirPath` stays on one filesystem and is + // therefore atomic. Two properties fall out of putting the deployment id ABOVE the component name: + // - the leaf directory's basename IS the component name, so the pre-go-live validation load + // (componentLoader keys the ApplicationScope + status off basename) sees the real name, not a + // UUID; and + // - each deployment gets its OWN parent (.deploy-staging/), so a parallel or queued + // deploy of the same component never shares a directory — cleanup can't sweep a sibling. + // See DEPLOY_STAGING_DIR. get stagingDirPath(): string { - return join(dirname(this.dirPath), DEPLOY_STAGING_DIR, this.name, this.stagingId); + return join(dirname(this.dirPath), DEPLOY_STAGING_DIR, this.stagingId, this.name); } // Route extract/install into the staging directory. Called by stageApplication(). @@ -1124,6 +1137,11 @@ export async function stageApplication(application: Application): Promise) up front. extractApplication's + // own mkdir only covers the payload path — for a `package` deploy the FIRST filesystem touch is the + // `npm pack`/git-clone spawn, whose cwd is this parent directory; without it the spawn fails with + // ENOENT (posix_spawn) before any tarball is produced. + await mkdir(dirname(application.stagingDirPath), { recursive: true }); try { await application.writeTransientNpmrc(); try { @@ -1185,12 +1203,16 @@ export async function activateApplication(application: Application): Promise). Remove it with a NON-recursive rmdir, which succeeds only when it + // is empty — a belt-and-suspenders guard against ever recursively deleting a directory that could + // hold another deploy's build. ENOTEMPTY and ENOENT (already gone) are expected and ignored. cleanupAsideDir(asideStagingDir, application.name); - rm(dirname(stagingDirPath), { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }).catch((err) => - logger.trace?.(`Deferred cleanup of ${application.name} staging directory: ${err.message}`) - ); + rmdir(dirname(stagingDirPath)).catch((err) => { + if (err.code !== 'ENOTEMPTY' && err.code !== 'ENOENT') + logger.trace?.(`Deferred cleanup of ${application.name} staging directory: ${err.message}`); + }); } /** diff --git a/unitTests/components/deployStaging.test.js b/unitTests/components/deployStaging.test.js index 0998d2e70c..283fb2f0a0 100644 --- a/unitTests/components/deployStaging.test.js +++ b/unitTests/components/deployStaging.test.js @@ -168,4 +168,64 @@ describe('two-phase deploy primitives (stage / activate / discard)', function () await Promise.all([discardStagedApplication(a), discardStagedApplication(b)]); }); + + it('stages a `file:` tarball package identifier (the package path, no payload)', async () => { + // Regression for the staging parent dir: extractApplication's `file:`-tarball branch (and the + // npm-pack branch) resolve paths relative to dirname(stagingDirPath), which must exist before + // extraction. A payload-only test never exercises that branch. + const name = `stage_test_${process.pid}_${counter++}`; + const tgzDir = await fs.mkdtemp(path.join(os.tmpdir(), 'harper-stage-tgz-')); + const tgzPath = path.join(tgzDir, 'component.tgz'); + await fs.writeFile(tgzPath, await makeComponentPayload('from-tarball')); + + const app = new Application({ name, packageIdentifier: `file:${tgzPath}` }); + await stageApplication(app); + assert.match(await readMarker(app.stagingDirPath), /from-tarball/, 'tarball extracted into staging'); + assert.strictEqual(existsSync(app.dirPath), false, 'live dir untouched by staging a tarball'); + + await discardStagedApplication(app); + await fs.rm(tgzDir, { recursive: true, force: true }); + }); + + it('activate cleanup does NOT sweep a sibling staged build of the same component', async () => { + // Two deploys of the same component staged concurrently share the .deploy-staging/ parent. + // Activating one must not recursively delete that parent and destroy the other's staged build. + const name = `stage_test_${process.pid}_${counter++}`; + const first = new Application({ name, payload: await makeComponentPayload('first') }); + const second = new Application({ name, payload: await makeComponentPayload('second') }); + await stageApplication(first); + await stageApplication(second); + assert.notStrictEqual(first.stagingDirPath, second.stagingDirPath); + + await activateApplication(first); + + assert.match(await readMarker(first.dirPath), /first/, 'first went live'); + assert.ok(existsSync(second.stagingDirPath), 'the sibling staged build survived the activate cleanup'); + + await fs.rm(first.dirPath, { recursive: true, force: true }); + await discardStagedApplication(second); + await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); + + it('activate moves a DANGLING symlink at the live path aside instead of failing EEXIST', async () => { + // A prior `file:`-directory deploy leaves the live path as a symlink; if its target is later + // removed the link dangles. moveDirAside must detect it via lstat (access(F_OK) follows the link + // and reports ENOENT) so the swap replaces it cleanly. + const name = `stage_test_${process.pid}_${counter++}`; + const dirPath = path.join(COMPONENTS_ROOT, name); + await fs.symlink(path.join(os.tmpdir(), `does-not-exist-${process.pid}-${counter}`), dirPath); + assert.strictEqual(existsSync(dirPath), false, 'precondition: the symlink is dangling'); + + const app = new Application({ name, payload: await makeComponentPayload('replaced') }); + await stageApplication(app); + await activateApplication(app); + + const stat = await fs.lstat(dirPath); + assert.strictEqual(stat.isSymbolicLink(), false, 'live path is now a real directory, not the dead link'); + assert.match(await readMarker(dirPath), /replaced/); + + await fs.rm(dirPath, { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); }); From cb2020e60ddc36f04c25f538d50f5561e6c77279 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 17 Jul 2026 15:36:52 -0400 Subject: [PATCH 03/22] test(deploy): op-level coverage for stage_component, activate_component, 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 --- .../components/deployPhaseOperations.test.js | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 unitTests/components/deployPhaseOperations.test.js diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js new file mode 100644 index 0000000000..3c7dc09c96 --- /dev/null +++ b/unitTests/components/deployPhaseOperations.test.js @@ -0,0 +1,98 @@ +'use strict'; + +// Operation-level orchestration tests for the two-phase deploy handlers: stage_component, +// activate_component, and the legacy one-shot deploy_component (two_phase: false). The heavy, +// environment-dependent internals (the actual build/swap, payload blob ingest, credential resolution) +// are rewired to stubs so these assert the ORCHESTRATION — which primitive runs, whether replication +// and restart fire, the response shape — without needing npm/network or the full component loader. + +const assert = require('node:assert'); +const rewire = require('rewire'); +const sinon = require('sinon'); + +const operations = rewire('#js/components/operations'); +const manageThreads = require('#src/server/threads/manageThreads'); + +// Neutralize the parts that touch the filesystem / datastore / cluster, leaving the control flow. +function stubInternals(sandbox) { + const stage = sandbox.stub().resolves('/staging'); + const activate = sandbox.stub().resolves(); + const prepare = sandbox.stub().resolves(); + operations.__set__('stageApplication', stage); + operations.__set__('activateApplication', activate); + operations.__set__('prepareApplication', prepare); + // Skip payload-blob ingest and credential resolution — not what these tests exercise. + operations.__set__('sourceExtractionPayload', sandbox.stub().resolves(Buffer.from('tarball'))); + operations.__set__('resolveNodeCredentials', sandbox.stub().resolves([])); + // Don't actually bounce workers. + sandbox.stub(manageThreads, 'restartWorkers'); + return { stage, activate, prepare }; +} + +describe('stage_component / activate_component / one-shot deploy orchestration', () => { + let sandbox; + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + afterEach(() => { + sandbox.restore(); + }); + + it('stageComponent stages, replicates, and returns a staged marker + deployment_id — no restart, no config write', async () => { + const { stage, activate, prepare } = stubInternals(sandbox); + const addConfig = sandbox.stub(require('#src/config/configUtils'), 'addConfig').resolves(); + + const res = await operations.stageComponent({ project: 'my_app', payload: Buffer.from('x') }); + + assert.strictEqual(stage.calledOnce, true, 'stageApplication was called'); + assert.strictEqual(activate.called, false, 'activate never runs during a stage'); + assert.strictEqual(prepare.called, false, 'the one-shot prepare path is not used'); + assert.strictEqual(res.staged, true); + assert.strictEqual(res.project, 'my_app'); + assert.ok(res.deployment_id, 'a deployment_id is returned'); + assert.strictEqual(addConfig.called, false, 'staging never writes root config'); + assert.strictEqual(manageThreads.restartWorkers.called, false, 'staging never restarts'); + }); + + it('activateComponent swaps live and restarts when restart:true; returns an activated marker', async () => { + const { activate } = stubInternals(sandbox); + + const res = await operations.activateComponent({ + project: 'my_app', + deployment_id: 'dep-123', + restart: true, + }); + + assert.strictEqual(activate.calledOnce, true, 'activateApplication was called'); + assert.strictEqual(res.activated, true); + assert.strictEqual(res.project, 'my_app'); + assert.strictEqual(manageThreads.restartWorkers.calledWith('http'), true, 'restarted the http workers'); + }); + + it('activateComponent does not restart when restart is omitted', async () => { + const { activate } = stubInternals(sandbox); + await operations.activateComponent({ project: 'my_app', deployment_id: 'dep-123' }); + assert.strictEqual(activate.calledOnce, true); + assert.strictEqual(manageThreads.restartWorkers.called, false); + }); + + it('activateComponent rejects when no deployment_id is supplied', async () => { + stubInternals(sandbox); + await assert.rejects(() => operations.activateComponent({ project: 'my_app' }), /deployment_id.*required/i); + }); + + it('deploy_component with two_phase:false takes the legacy one-shot path (prepareApplication, not stage/activate)', async () => { + const { stage, activate, prepare } = stubInternals(sandbox); + + const res = await operations.deployComponent({ + project: 'my_app', + payload: Buffer.from('x'), + two_phase: false, + }); + + assert.strictEqual(prepare.calledOnce, true, 'one-shot prepareApplication was called'); + assert.strictEqual(stage.called, false, 'two-phase stage not used on the one-shot path'); + assert.strictEqual(activate.called, false, 'two-phase activate not used on the one-shot path'); + assert.match(res.message, /Successfully deployed/); + }); +}); From 52718f6fde5dabfc434a54c4a6677179c23cd463 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 17 Jul 2026 15:44:42 -0400 Subject: [PATCH 04/22] test(deploy): update integration phase assertions to two-phase names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- integrationTests/deploy/deploy-tracking-events.test.ts | 7 ++++--- integrationTests/deploy/deploy-tracking.test.ts | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/integrationTests/deploy/deploy-tracking-events.test.ts b/integrationTests/deploy/deploy-tracking-events.test.ts index 0d80f2f600..49a290fc65 100644 --- a/integrationTests/deploy/deploy-tracking-events.test.ts +++ b/integrationTests/deploy/deploy-tracking-events.test.ts @@ -132,9 +132,10 @@ suite('Deployment tracking — events + SSE', (ctx: ContextWithHarper) => { ok(Array.isArray(row.event_log), 'event_log should be an array'); ok(row.event_log.length >= 2, `expected at least 2 events, got ${row.event_log.length}`); const phases = row.event_log.filter((e: any) => e.event === 'phase').map((e: any) => e.data?.phase); - // We emit prepare → (load) → replicate → success in the lifecycle. Verify the spine. - ok(phases.includes('prepare'), `event_log should include a prepare phase: ${phases.join(',')}`); - ok(phases.includes('replicate'), `event_log should include a replicate phase: ${phases.join(',')}`); + // A two-phase deploy emits stage → (load) → activate → success. Verify the spine (load is only + // emitted off the main thread, so it isn't asserted here). + ok(phases.includes('stage'), `event_log should include a stage phase: ${phases.join(',')}`); + ok(phases.includes('activate'), `event_log should include an activate phase: ${phases.join(',')}`); }); test('get_deployment with Accept: text/event-stream replays event_log and closes cleanly', async () => { diff --git a/integrationTests/deploy/deploy-tracking.test.ts b/integrationTests/deploy/deploy-tracking.test.ts index 57bc623c40..255e6d28b2 100644 --- a/integrationTests/deploy/deploy-tracking.test.ts +++ b/integrationTests/deploy/deploy-tracking.test.ts @@ -196,7 +196,9 @@ suite('Deployment tracking', (ctx: ContextWithHarper) => { // match what the install command actually printed; an empty array would pass the // pre-fix code, so assert on the line content too. const body = JSON.parse(response.body); - strictEqual(body.phase, 'prepare'); + // The install runs during the stage phase of a two-phase deploy, so a failed install is + // recorded against 'stage' (was 'prepare' in the one-shot flow). + strictEqual(body.phase, 'stage'); ok(Array.isArray(body.install_output?.lines) && body.install_output.lines.length > 0); ok( body.install_output.lines.some( From 64ad02b8f7b24629141dfeb4b8350d5dd5dd5ab6 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 17 Jul 2026 15:51:44 -0400 Subject: [PATCH 05/22] test(deploy): rewrite op-level tests as real-module tests (no sinon/rewire) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../components/deployPhaseOperations.test.js | 181 +++++++++++------- 1 file changed, 112 insertions(+), 69 deletions(-) diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index 3c7dc09c96..76945c031e 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -1,98 +1,141 @@ 'use strict'; -// Operation-level orchestration tests for the two-phase deploy handlers: stage_component, -// activate_component, and the legacy one-shot deploy_component (two_phase: false). The heavy, -// environment-dependent internals (the actual build/swap, payload blob ingest, credential resolution) -// are rewired to stubs so these assert the ORCHESTRATION — which primitive runs, whether replication -// and restart fire, the response shape — without needing npm/network or the full component loader. +// Operation-level tests for the two-phase deploy handlers: stage_component, activate_component, and +// deploy_component (both the two-phase default and the two_phase:false one-shot fallback). These run +// the real handlers against a real temp filesystem with a real tarball payload — no stubbing — in the +// deployStaging.test.js style (AGENTS.md: new tests use plain `assert` against real modules, no +// sinon/rewire). Payload deploys are used throughout so nothing reaches the component loader (a +// `package` deploy's protected-name guard would), and no test requests a restart. const assert = require('node:assert'); -const rewire = require('rewire'); -const sinon = require('sinon'); - -const operations = rewire('#js/components/operations'); -const manageThreads = require('#src/server/threads/manageThreads'); - -// Neutralize the parts that touch the filesystem / datastore / cluster, leaving the control flow. -function stubInternals(sandbox) { - const stage = sandbox.stub().resolves('/staging'); - const activate = sandbox.stub().resolves(); - const prepare = sandbox.stub().resolves(); - operations.__set__('stageApplication', stage); - operations.__set__('activateApplication', activate); - operations.__set__('prepareApplication', prepare); - // Skip payload-blob ingest and credential resolution — not what these tests exercise. - operations.__set__('sourceExtractionPayload', sandbox.stub().resolves(Buffer.from('tarball'))); - operations.__set__('resolveNodeCredentials', sandbox.stub().resolves([])); - // Don't actually bounce workers. - sandbox.stub(manageThreads, 'restartWorkers'); - return { stage, activate, prepare }; +const fs = require('node:fs/promises'); +const { existsSync } = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const zlib = require('node:zlib'); +const tarfs = require('tar-fs'); + +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const operations = require('#src/components/operations'); +const { DEPLOY_STAGING_DIR } = require('#src/components/Application'); +const { getConfigPath } = require('#src/config/configUtils'); +const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); + +const COMPONENTS_ROOT = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); + +// Pack a directory's CONTENTS into a gzipped tar Buffer, the shape a deploy payload takes. +function packDirectory(dir) { + return new Promise((resolve, reject) => { + const chunks = []; + tarfs + .pack(dir) + .pipe(zlib.createGzip()) + .on('data', (c) => chunks.push(c)) + .on('end', () => resolve(Buffer.concat(chunks))) + .on('error', reject); + }); } -describe('stage_component / activate_component / one-shot deploy orchestration', () => { - let sandbox; - beforeEach(() => { - sandbox = sinon.createSandbox(); - }); - afterEach(() => { - sandbox.restore(); +// A component source that already contains node_modules, so installApplication short-circuits and no +// npm/network is needed. +async function makeComponentPayload(marker) { + const src = await fs.mkdtemp(path.join(os.tmpdir(), 'harper-op-src-')); + await fs.writeFile(path.join(src, 'package.json'), JSON.stringify({ name: 'op-fixture', version: '1.0.0' })); + await fs.writeFile(path.join(src, 'index.js'), `module.exports = ${JSON.stringify(marker)};\n`); + await fs.mkdir(path.join(src, 'node_modules'), { recursive: true }); + await fs.writeFile(path.join(src, 'node_modules', '.marker'), marker); + const payload = await packDirectory(src); + await fs.rm(src, { recursive: true, force: true }); + return payload; +} + +const readIndex = (dir) => fs.readFile(path.join(dir, 'index.js'), 'utf8'); + +describe('deploy operations: stage_component / activate_component / deploy_component', function () { + this.timeout(30_000); + + before(async () => { + await fs.mkdir(COMPONENTS_ROOT, { recursive: true }); }); - it('stageComponent stages, replicates, and returns a staged marker + deployment_id — no restart, no config write', async () => { - const { stage, activate, prepare } = stubInternals(sandbox); - const addConfig = sandbox.stub(require('#src/config/configUtils'), 'addConfig').resolves(); + let counter = 0; + const names = []; + function freshName() { + const name = `op_test_${process.pid}_${counter++}`; + names.push(name); + return name; + } + + // Sweep any live dirs, staging, and aside created by the suite. + after(async () => { + for (const name of names) await fs.rm(path.join(COMPONENTS_ROOT, name), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, '.deploy-aside'), { recursive: true, force: true }); + }); - const res = await operations.stageComponent({ project: 'my_app', payload: Buffer.from('x') }); + it('stage_component builds the incoming version into staging without going live, and returns a deployment_id', async () => { + const name = freshName(); + const res = await operations.stageComponent({ project: name, payload: await makeComponentPayload('op-staged') }); - assert.strictEqual(stage.calledOnce, true, 'stageApplication was called'); - assert.strictEqual(activate.called, false, 'activate never runs during a stage'); - assert.strictEqual(prepare.called, false, 'the one-shot prepare path is not used'); assert.strictEqual(res.staged, true); - assert.strictEqual(res.project, 'my_app'); - assert.ok(res.deployment_id, 'a deployment_id is returned'); - assert.strictEqual(addConfig.called, false, 'staging never writes root config'); - assert.strictEqual(manageThreads.restartWorkers.called, false, 'staging never restarts'); - }); + assert.strictEqual(res.project, name); + assert.strictEqual(typeof res.deployment_id, 'string'); - it('activateComponent swaps live and restarts when restart:true; returns an activated marker', async () => { - const { activate } = stubInternals(sandbox); + const stagedDir = path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR, res.deployment_id, name); + assert.ok(existsSync(path.join(stagedDir, 'index.js')), 'component was built into the staging dir'); + assert.strictEqual(existsSync(path.join(COMPONENTS_ROOT, name)), false, 'staging did not touch the live path'); + }); - const res = await operations.activateComponent({ - project: 'my_app', - deployment_id: 'dep-123', - restart: true, + it('activate_component takes a prior stage live', async () => { + const name = freshName(); + const staged = await operations.stageComponent({ + project: name, + payload: await makeComponentPayload('op-activated'), }); + const res = await operations.activateComponent({ project: name, deployment_id: staged.deployment_id }); - assert.strictEqual(activate.calledOnce, true, 'activateApplication was called'); assert.strictEqual(res.activated, true); - assert.strictEqual(res.project, 'my_app'); - assert.strictEqual(manageThreads.restartWorkers.calledWith('http'), true, 'restarted the http workers'); + assert.strictEqual(res.project, name); + const liveDir = path.join(COMPONENTS_ROOT, name); + assert.ok(existsSync(liveDir), 'live component dir now exists'); + assert.match(await readIndex(liveDir), /op-activated/); + // A restart was not requested, so the message must not claim one. + assert.doesNotMatch(res.message, /restart/i); }); - it('activateComponent does not restart when restart is omitted', async () => { - const { activate } = stubInternals(sandbox); - await operations.activateComponent({ project: 'my_app', deployment_id: 'dep-123' }); - assert.strictEqual(activate.calledOnce, true); - assert.strictEqual(manageThreads.restartWorkers.called, false); + it('activate_component rejects when no deployment_id is supplied', async () => { + const name = freshName(); + await assert.rejects(() => operations.activateComponent({ project: name }), /deployment_id.*required/i); }); - it('activateComponent rejects when no deployment_id is supplied', async () => { - stubInternals(sandbox); - await assert.rejects(() => operations.activateComponent({ project: 'my_app' }), /deployment_id.*required/i); - }); + it('deploy_component (two-phase default) stages then activates end-to-end', async () => { + const name = freshName(); + const res = await operations.deployComponent({ project: name, payload: await makeComponentPayload('op-deployed') }); - it('deploy_component with two_phase:false takes the legacy one-shot path (prepareApplication, not stage/activate)', async () => { - const { stage, activate, prepare } = stubInternals(sandbox); + assert.match(res.message, /Successfully deployed/); + assert.strictEqual(typeof res.deployment_id, 'string'); + const liveDir = path.join(COMPONENTS_ROOT, name); + assert.match(await readIndex(liveDir), /op-deployed/, 'component is live after a two-phase deploy'); + // The staged copy was consumed by the swap; its per-deploy staging parent is cleaned up. + assert.strictEqual( + existsSync(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR, res.deployment_id)), + false, + 'per-deploy staging parent removed after activation' + ); + }); + it('deploy_component with two_phase:false runs the legacy one-shot path', async () => { + const name = freshName(); const res = await operations.deployComponent({ - project: 'my_app', - payload: Buffer.from('x'), + project: name, + payload: await makeComponentPayload('op-oneshot'), two_phase: false, }); - assert.strictEqual(prepare.calledOnce, true, 'one-shot prepareApplication was called'); - assert.strictEqual(stage.called, false, 'two-phase stage not used on the one-shot path'); - assert.strictEqual(activate.called, false, 'two-phase activate not used on the one-shot path'); assert.match(res.message, /Successfully deployed/); + const liveDir = path.join(COMPONENTS_ROOT, name); + assert.match(await readIndex(liveDir), /op-oneshot/, 'component is live after a one-shot deploy'); }); }); From 26de01cac9a84f1b03e92da46f5224eb0d093dd4 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 19 Jul 2026 23:23:53 -0400 Subject: [PATCH 06/22] fix(types): cast commitResolution to Promise at recordCommitLatency call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | void` (the abort() branch reassigns it), but at this call site it is the `commit()` promise already cast to `Promise` 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 --- resources/DatabaseTransaction.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index fdc0bb2363..33626d19c0 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -364,7 +364,10 @@ export class DatabaseTransaction implements Transaction { // "Outstanding write transactions have too long of queue" (503) rejection. A transient- // conflict retry rejects this promise and issues a fresh commit(), and outstandingCommit // re-arms per attempt, so recording per attempt matches the overload semantics. - recordCommitLatency(commitResolution, performance.now()); + // `commitResolution` is declared with the wider `Promise | void` + // (the abort() branch below), but in this branch it is the `commit()` promise cast to + // `Promise` just above; recordCommitLatency only awaits it for timing. + recordCommitLatency(commitResolution as Promise, performance.now()); } else { try { commitResolution = transaction.abort(); From 6e4269c451a4745d2747a6469171f7899d8f6f75 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 20 Jul 2026 10:44:09 -0400 Subject: [PATCH 07/22] feat(deploy): revert_component + retained previous version, CLI commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the draft PR's open questions: - Reversibility (Q3): activate now RETAINS the outgoing live version as .deploy-previous/ (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 --- DESIGN.md | 32 +++- bin/cliOperations.ts | 84 ++++++----- components/Application.ts | 125 +++++++++++++++- components/deploymentRecorder.ts | 4 + components/operations.js | 138 +++++++++++++++++- components/operationsValidation.js | 33 ++++- server/serverHelpers/serverHandlers.js | 1 + server/serverHelpers/serverUtilities.ts | 4 + .../components/deployPhaseOperations.test.js | 27 +++- .../components/deployPhaseValidators.test.js | 31 +++- unitTests/components/deployStaging.test.js | 70 +++++++++ utility/hdbTerms.ts | 4 + utility/operation_authorization.ts | 1 + 13 files changed, 504 insertions(+), 50 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 5b24f5264b..4674553135 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -351,9 +351,35 @@ since the `hdb_deployment` row's `payload_blob` is how peers fetch the tarball a phases by deployment id. When `system` is excluded from a narrow `REPLICATION_DATABASES`, or the caller passes `two_phase: false`, or the invocation is a peer replaying a one-shot deploy, `deploy_component` falls back to `deployComponentOneShot` (the previous behavior, preserved verbatim). -Known gap for a rolling upgrade window: an origin on this version replicating `stage_component` to a -peer that predates these operations will see that peer fail the op; `two_phase: false` or -`ignore_replication_errors` is the escape hatch until capability negotiation lands. +Cross-version skew is a non-issue by policy — a cluster stays in lockstep on its Harper version, so +every node understands `stage_component`/`activate_component` — which is why there is no capability +negotiation on the fan-out. + +**Replicator contract this rides on (`harper-pro/replication/replicator.ts`).** +`server.replication.replicateOperation(op, {onPeerResult})` fans `op` to every node in `server.nodes` +in parallel, setting `op.replicated = false` on the copy it sends so a peer never re-fans (the deploy +handlers instead detect a replicated execution by the presence of `_deploymentId`, which is always set +on the sub-operations). Per-peer failures never throw — `sendOperationToNode` rejections are caught and +surface as `{status:'failed', reason, node}` entries in the returned `replicated[]` array and via +`onPeerResult`, which is exactly the shape `DeploymentRecorder.normalizePeerResult` consumes. Peers +authenticate node-to-node by TLS certificate, and the receive side runs the op via +`server.operation(data, {user}, !isAuthorizedNode)` — for a trusted cluster node the authorize flag is +`false`, so a replicated super-user op skips the permission gate. That is why `stage_component` / +`activate_component` / `revert_component` (registered with the same `permission(true, [])` as +`deploy_component`, dispatched by `operation` name) replicate without an `hdb_user`, identically to the +long-proven `deploy_component` fan-out. + +**Reversibility: retained previous + `revert_component`.** `activateApplication` no longer discards the +outgoing live version — it retains it as `.deploy-previous/` (`retainAsPrevious`, evicting the +older one so exactly one previous is kept per component). `revert_component` swaps the live directory +with that retained previous via three same-filesystem renames through a hidden holding path, cluster- +wide and replicated like activate. The swap is bidirectional, so reverting a revert rolls forward +again. This backs two things: a customer can deploy, run their own health checks against the live +version, and `revert` if unhappy even when the cluster looks healthy; and `deploy_component`'s opt-in +`revert_on_failure` rolls the whole cluster back to the previous version when the activate phase leaves +some nodes live and some not, so the cluster reconverges on one version. The previous copy is retained +per-node (each node retains its own outgoing version during its own activate), so a replicated revert +has a local rollback source on every node. ## Scheduler: cluster-once execution without a consensus primitive (`resources/scheduler/`) diff --git a/bin/cliOperations.ts b/bin/cliOperations.ts index 019fa5ff20..c0f8daa67f 100644 --- a/bin/cliOperations.ts +++ b/bin/cliOperations.ts @@ -18,7 +18,16 @@ import { DeployRenderer } from './deployRenderer.ts'; import { getHdbPid } from '../utility/processManagement/processManagement.js'; import { initConfig, getConfigPath } from '../config/configUtils.ts'; -const OP_ALIASES = { deploy: 'deploy_component', package: 'package_component' }; +const OP_ALIASES = { + deploy: 'deploy_component', + package: 'package_component', + // Two-phase deploy: `harper stage` packages + uploads the incoming version to a hidden staging + // dir cluster-wide (no go-live); `harper activate` swaps a staged deployment live; `harper revert` + // swaps the live version back to its retained previous version. + stage: 'stage_component', + activate: 'activate_component', + revert: 'revert_component', +}; // Shown for any local-instance connection failure (missing pid, missing/stale domain // socket, or a refused/ENOENT connect against it) — they're all the same user-facing @@ -31,7 +40,7 @@ const LOCAL_NOT_RUNNING_MESSAGE = 'Harper is not running. Use `harperdb run` (or // deploy completes. Add an operation here only after wiring its server-side // SSE_PROGRESS_OPERATIONS entry — otherwise the server returns the buffered JSON path and // the SSE parser sees no events. -const SSE_OPERATIONS = new Set(['deploy_component']); +const SSE_OPERATIONS = new Set(['deploy_component', 'stage_component', 'activate_component', 'revert_component']); // Properties on `req` that the CLI itself uses for transport/UX, not the operations API. // They never get serialized into the request body. @@ -151,38 +160,47 @@ function operationFields(req: any): any { } export { cliOperations, buildRequest }; -const PREPARE_OPERATION: any = { - deploy_component: async (req) => { - if (req.package) { - return; - } - const projectPath = process.cwd(); - if (!req.project) req.project = path.basename(projectPath); - const packageOptions = { - skip_node_modules: req.skip_node_modules !== false, - skip_symlinks: req.skip_symlinks === true, - }; - // Store path + options for deferred stream creation after the renderer is set up, - // so the pre-gzip onBytes callback can be wired directly to renderer.countUploadBytes. - req._projectPath = projectPath; - req._packageOptions = packageOptions; - // Pre-walk the directory once for both the uncompressed-size estimate (progress bar - // total) and the dangling-symlink list — a dangling symlink would otherwise silently - // truncate the tarball (tar-fs finalizes early on the broken target). Packaging skips - // them; the list is reused below (no second walk) and warns the user which links were - // skipped so the omission is visible. - const scan = await scanPackageDirectory(projectPath, packageOptions); - req._uploadSizeEstimate = scan.totalSize; - req._danglingSymlinks = scan.danglingSymlinks; - if (scan.danglingSymlinks.length) { - process.stderr.write( - `warning: skipping ${scan.danglingSymlinks.length} broken symlink(s) — their linked content will NOT be deployed:\n` + - scan.danglingSymlinks.map((p) => ` ${p}\n`).join('') - ); - } - req._multipart = true; - }, +// Package the current working directory into a multipart tarball upload. Shared by `deploy` and +// `stage` — both send the incoming component version as a `payload` (unless a `package` identifier is +// given, in which case the server fetches it and there is nothing to upload). +const packageCwdForUpload = async (req) => { + if (req.package) { + return; + } + + const projectPath = process.cwd(); + if (!req.project) req.project = path.basename(projectPath); + const packageOptions = { + skip_node_modules: req.skip_node_modules !== false, + skip_symlinks: req.skip_symlinks === true, + }; + // Store path + options for deferred stream creation after the renderer is set up, + // so the pre-gzip onBytes callback can be wired directly to renderer.countUploadBytes. + req._projectPath = projectPath; + req._packageOptions = packageOptions; + // Pre-walk the directory once for both the uncompressed-size estimate (progress bar + // total) and the dangling-symlink list — a dangling symlink would otherwise silently + // truncate the tarball (tar-fs finalizes early on the broken target). Packaging skips + // them; the list is reused below (no second walk) and warns the user which links were + // skipped so the omission is visible. + const scan = await scanPackageDirectory(projectPath, packageOptions); + req._uploadSizeEstimate = scan.totalSize; + req._danglingSymlinks = scan.danglingSymlinks; + if (scan.danglingSymlinks.length) { + process.stderr.write( + `warning: skipping ${scan.danglingSymlinks.length} broken symlink(s) — their linked content will NOT be deployed:\n` + + scan.danglingSymlinks.map((p) => ` ${p}\n`).join('') + ); + } + req._multipart = true; +}; + +const PREPARE_OPERATION: any = { + deploy_component: packageCwdForUpload, + // `harper stage` uploads the same tarball as `harper deploy`. activate/revert take no payload + // (they operate on an already-staged / already-retained version), so they need no prep step. + stage_component: packageCwdForUpload, }; /** diff --git a/components/Application.ts b/components/Application.ts index dbeef9fdc6..8721ab91ff 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -424,6 +424,20 @@ export const ASIDE_STAGING_DIR = '.deploy-aside'; // restart-on-change storm and needs no deploy:start watcher suppression. export const DEPLOY_STAGING_DIR = '.deploy-staging'; +// Hidden directory under the components root that RETAINS the immediately-previous live version of a +// component (`.deploy-previous/`) after an activate swap, so it can be swapped back by +// revert_component. Exactly one previous version is kept per component — each activate evicts the +// older one — so the retention cost is bounded at one extra copy. Same filesystem as the live path +// (atomic swap on revert), leading-dot-hidden (loader/watchers ignore it). This is what turns the +// deploy swap into something reversible: a customer can activate, run their own health checks, and +// revert if unhappy; and a partially-failed activate can be swapped back cluster-wide. +export const DEPLOY_PREVIOUS_DIR = '.deploy-previous'; + +// Absolute path of the retained-previous copy for a component's live directory. +function previousDirPathFor(liveDirPath: string): string { + return join(dirname(liveDirPath), DEPLOY_PREVIOUS_DIR, basename(liveDirPath)); +} + /** * Atomically move `targetDirPath` aside into a hidden, per-component staging directory if it * exists, returning the aside staging directory (for best-effort cleanup) or null when there was @@ -465,6 +479,34 @@ function cleanupAsideDir(asideStagingDir: string | null, componentName: string): ); } +/** + * Retain the current live version of a component as its rollback source: rename it to + * `.deploy-previous/`, evicting any older retained-previous first. Returns the aside directory + * holding the evicted older-previous (for best-effort cleanup), or null when there was no live + * version to retain (a first-ever deploy). + * + * The eviction moves the older-previous ASIDE (an atomic rename that can't fail on a directory a + * lingering worker still holds open) rather than an in-place rm, so the subsequent rename onto + * `.deploy-previous/` never races an incomplete delete (ENOTEMPTY). Like the aside swap, the + * still-running worker of the version being retained keeps writing into the renamed inode harmlessly + * until it exits on restart. + */ +async function retainAsPrevious(liveDirPath: string): Promise { + const previousPath = previousDirPathFor(liveDirPath); + try { + await lstat(liveDirPath); // lstat, not access: see moveDirAside — a dangling symlink must still move + } catch (err) { + if (err.code === 'ENOENT') return null; // no live version yet — nothing to retain + throw err; + } + // Evict the older retained-previous (2 deploys ago; its worker exited on the last restart) by moving + // it aside atomically, clearing the target for the rename below. + const evictedAside = await moveDirAside(previousPath); + await mkdir(dirname(previousPath), { recursive: true }); + await rename(liveDirPath, previousPath); + return evictedAside; +} + // The credential helper git executes for a private git-reference deploy. It ships alongside this // module (both in source and in dist), holds no secret, and is inert without a live session. export const GIT_CREDENTIAL_HELPER_PATH = join(__dirname, 'gitCredentialHelper.js'); @@ -963,6 +1005,12 @@ export class Application { return join(dirname(this.dirPath), DEPLOY_STAGING_DIR, this.stagingId, this.name); } + // The retained-previous copy this component would revert to (`.deploy-previous/`). See + // DEPLOY_PREVIOUS_DIR / activateApplication / revertApplication. + get previousDirPath(): string { + return previousDirPathFor(this.dirPath); + } + // Route extract/install into the staging directory. Called by stageApplication(). useStagingBuildDir(): void { this.#buildDirPath = this.stagingDirPath; @@ -1175,6 +1223,9 @@ export async function stageApplication(application: Application): Promise` so + * revert_component can swap it back. See retainAsPrevious. + * * This method should only be called from the main thread. */ export async function activateApplication(application: Application): Promise { @@ -1191,30 +1242,90 @@ export async function activateApplication(application: Application): Promise), then rename // the staged copy into place. Both live under the components root, so the rename is same-fs and - // atomic — there is no interval where `dirPath` is a partially populated directory. - asideStagingDir = await moveDirAside(application.dirPath); + // atomic — there is no interval where `dirPath` is a partially populated directory. retainAsPrevious + // tolerates a still-writing worker exactly as the old aside swap did. + evictedAside = await retainAsPrevious(application.dirPath); await mkdir(dirname(application.dirPath), { recursive: true }); await rename(stagingDirPath, application.dirPath); application.useLiveBuildDir(); } finally { broadcastDeployEnd(application.name); } - // Best-effort cleanup of the outgoing version (see cleanupAsideDir). The rename already consumed - // stagingDirPath, so all that remains is this deploy's now-empty staging parent + // Best-effort cleanup of the EVICTED older-previous (the version from two deploys ago; see + // cleanupAsideDir) — NOT the retained previous, which is kept for revert. The rename already consumed + // stagingDirPath, so all that remains of staging is this deploy's now-empty parent // (.deploy-staging/). Remove it with a NON-recursive rmdir, which succeeds only when it // is empty — a belt-and-suspenders guard against ever recursively deleting a directory that could // hold another deploy's build. ENOTEMPTY and ENOENT (already gone) are expected and ignored. - cleanupAsideDir(asideStagingDir, application.name); + cleanupAsideDir(evictedAside, application.name); rmdir(dirname(stagingDirPath)).catch((err) => { if (err.code !== 'ENOTEMPTY' && err.code !== 'ENOENT') logger.trace?.(`Deferred cleanup of ${application.name} staging directory: ${err.message}`); }); } +/** + * Swap a component's live version with its retained previous version (`.deploy-previous/`), + * atomically, then let watchers restart onto it. This is what backs revert_component: a customer can + * activate a deploy, run their own health checks, and swap back if unhappy; and a partially-failed + * activate can be rolled back cluster-wide. + * + * The swap is bidirectional: the outgoing live becomes the new retained previous, so a second revert + * toggles forward again. Three same-filesystem renames via a hidden holding path — the only window + * where `dirPath` is momentarily absent is between two atomic renames, and deploy:start suppresses + * watchers across it (same as activate). + * + * Throws if there is no retained previous version (a component deployed only once, or never). + * + * This method should only be called from the main thread. + */ +export async function revertApplication(application: Application): Promise { + const liveDirPath = application.dirPath; + const previousPath = previousDirPathFor(liveDirPath); + try { + await lstat(previousPath); + } catch (err) { + if (err.code === 'ENOENT') { + throw new Error( + `Cannot revert ${application.name}: no previous version is retained. A component must have ` + + `been deployed over a prior version (which activate retains as .deploy-previous) to be reverted.` + ); + } + throw err; + } + await broadcastDeployStart(application.name); + try { + // Does a live version currently exist? (It always should after a deploy, but guard so a missing + // live dir degrades to "restore previous" rather than throwing mid-swap.) + let liveExists = true; + try { + await lstat(liveDirPath); + } catch (err) { + if (err.code === 'ENOENT') liveExists = false; + else throw err; + } + await mkdir(dirname(previousPath), { recursive: true }); + if (liveExists) { + // Three-way atomic swap: live → holding, previous → live, holding(old live) → previous. + const holding = join(dirname(previousPath), `.reverting-${basename(liveDirPath)}-${randomUUID()}`); + await rename(liveDirPath, holding); + await rename(previousPath, liveDirPath); + await rename(holding, previousPath); + } else { + // No live version to preserve; just restore the previous into place (nothing becomes the new + // previous, so the component can't be re-reverted until its next deploy). + await rename(previousPath, liveDirPath); + } + application.useLiveBuildDir(); + } finally { + broadcastDeployEnd(application.name); + } +} + /** * Discard a staged-but-not-activated build (an aborted two-phase deploy). Best-effort: removes the * staging tree and tears down any transient credential state. The live component directory is never diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index 73f499bbf1..fb09d4107d 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -57,6 +57,8 @@ type DeploymentStatus = | 'replicating' // Two-phase deploy: swapping the staged build into the live path cluster-wide (activate phase). | 'activating' + // revert_component: swapping the live version back to its retained previous version. + | 'reverting' | 'restarting' | 'success' | 'failed' @@ -578,6 +580,8 @@ function startStatusFor(phase: string | undefined): DeploymentStatus | null { return 'replicating'; case 'activate': return 'activating'; + case 'revert': + return 'reverting'; case 'restart': return 'restarting'; default: diff --git a/components/operations.js b/components/operations.js index 303512a445..5d47e7462f 100644 --- a/components/operations.js +++ b/components/operations.js @@ -29,9 +29,11 @@ const { prepareApplication, stageApplication, activateApplication, + revertApplication, discardStagedApplication, ASIDE_STAGING_DIR, DEPLOY_STAGING_DIR, + DEPLOY_PREVIOUS_DIR, } = require('./Application.ts'); const { server } = require('../server/Server.ts'); const { DeploymentRecorder, awaitDeploymentRow, DEFAULT_AWAIT_ROW_TIMEOUT_MS } = require('./deploymentRecorder.ts'); @@ -678,10 +680,31 @@ async function deployComponentTwoPhase(req, credentialReferences) { if (!req.ignore_replication_errors) { const failed = recorder.getFailedPeers(); if (failed.length > 0) { + let revertNote = ''; + // Opt-in swap-back: some nodes went live and some didn't, leaving the cluster split across + // versions. When revert_on_failure is set, roll the whole cluster (incl. this origin) back to + // the retained previous version so it converges on one version again. Best-effort — a revert + // failure must not mask the original activate failure. + if (req.revert_on_failure) { + try { + emit('phase', { phase: 'revert', status: 'start' }); + await revertApplication(application); + const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { + restart: req.restart, + deploymentId: recorder.deploymentId, + }); + await server.replication.replicateOperation(revertOp, {}); + emit('phase', { phase: 'revert', status: 'done' }); + revertNote = ` The cluster was rolled back to the previous version (revert_on_failure); verify with get_components.`; + } catch (revertErr) { + log.warn('revert_on_failure rollback failed', revertErr); + revertNote = ` An automatic rollback (revert_on_failure) was attempted but also failed: ${revertErr?.message ?? revertErr}.`; + } + } throw new ServerError( `Component '${application.name}' was activated on the origin but failed to activate on ${failed.length} ` + `of ${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. Those nodes have the staged ` + - `build but did not go live. See deployment ${recorder.deploymentId} (get_deployment), or pass ` + + `build but did not go live.${revertNote} See deployment ${recorder.deploymentId} (get_deployment), or pass ` + `ignore_replication_errors: true.` ); } @@ -879,6 +902,110 @@ async function activateComponent(req) { return response; } +/** + * revert_component — swap a component's live version back to its retained previous version + * (`.deploy-previous/`, kept by the last activate), cluster-wide, then restart. Backs + * customer-driven rollback (deploy → run your own health checks → revert if unhappy) and a + * swap-back after a partially-failed activate. The swap is bidirectional, so reverting a revert + * rolls forward again. + * + * Reached two ways: directly by an operator, and by a peer replaying a replicated revert + * (`_deploymentId` set). deploy_component's `revert_on_failure` path drives it internally. + */ +async function revertComponent(req) { + if (req.project) req.project = path.parse(req.project).name; + const validation = validator.revertComponentValidator(req); + if (validation) throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); + + const isReplicatedExecution = typeof req._deploymentId === 'string'; + const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); + if (emitter && !req.progress) req.progress = emitter; + // The origin records a rollback row for observability; a peer replaying the revert does not. + const recorder = isReplicatedExecution + ? null + : await DeploymentRecorder.create({ + project: req.project, + package_identifier: null, + user: req.hdb_user?.username, + restart_mode: req.restart === 'rolling' ? 'rolling' : req.restart ? 'immediate' : null, + rollback_of: req.deployment_id ?? null, + emitter, + }); + if (recorder) req._deploymentId = recorder.deploymentId; + const emit = (event, data) => emitter?.emit(event, data); + const installCapture = createInstallCapture(); // revert has no install output, but finalizeDeployFailure expects one + const rollingRestart = req.restart === 'rolling'; + + try { + const application = new Application({ name: req.project }); + emit('phase', { phase: 'revert', status: 'start' }); + await revertApplication(application); + emit('phase', { phase: 'revert', status: 'done' }); + + const response = { message: `Reverted component: ${req.project}`, project: req.project, reverted: true }; + if (recorder) response.deployment_id = recorder.deploymentId; + + // Replicate the revert to peers (direct invocation only; a peer replaying must not re-fan). + req.restart = rollingRestart ? false : req.restart; + if (!isReplicatedExecution) { + delete req.progress; + const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { + restart: req.restart, + deploymentId: recorder?.deploymentId, + }); + recorder?.seal(); + const rep = await server.replication.replicateOperation(revertOp, { + onPeerResult: recorder + ? (result) => { + recorder.recordPeer(result); + emit('peer', result); + } + : undefined, + }); + if (recorder && rep?.replicated) recorder.recordPeers(rep.replicated); + } + + // Restart on this node (peers replaying an immediate-restart revert restart locally; the rolling + // path is driven only by the direct invoker via a replicated restart_service job). + if (req.restart === true) { + emit('phase', { phase: 'restart', status: 'start' }); + manageThreads.restartWorkers('http'); + emit('phase', { phase: 'restart', status: 'done' }); + response.message = `Reverted component: ${req.project}, restarting Harper`; + } else if (rollingRestart && !isReplicatedExecution) { + const serverUtilities = require('../server/serverHelpers/serverUtilities.ts'); + emit('phase', { phase: 'restart', status: 'start' }); + const jobResponse = await serverUtilities.executeJob({ + operation: 'restart_service', + service: 'http', + replicated: true, + }); + emit('phase', { phase: 'restart', status: 'done' }); + response.restartJobId = jobResponse.job_id; + response.message = `Reverted component: ${req.project}, restarting Harper`; + } + + if (recorder && !req.ignore_replication_errors) { + const failed = recorder.getFailedPeers(); + if (failed.length > 0) { + throw new ServerError( + `Component '${req.project}' was reverted on the origin but failed to revert on ${failed.length} ` + + `of ${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. ` + + `See deployment ${recorder.deploymentId} (get_deployment), or pass ignore_replication_errors: true.` + ); + } + } + + if (recorder) { + emit('phase', { phase: 'success', status: 'done' }); + await recorder.finish('rolled_back'); + } + return response; + } catch (err) { + throw await finalizeDeployFailure({ err, recorder, installCapture, emit }); + } +} + // ———————————————————————————————————————————————————————————————————————————— // Shared deploy-family helpers (used by deploy_component, stage_component, activate_component). // ———————————————————————————————————————————————————————————————————————————— @@ -1176,7 +1303,13 @@ async function getComponents() { const list = await fs.readdir(dir, { withFileTypes: true }); for (let item of list) { const itemName = item.name; - if (itemName === 'node_modules' || itemName === ASIDE_STAGING_DIR || itemName === DEPLOY_STAGING_DIR) continue; + if ( + itemName === 'node_modules' || + itemName === ASIDE_STAGING_DIR || + itemName === DEPLOY_STAGING_DIR || + itemName === DEPLOY_PREVIOUS_DIR + ) + continue; const itemPath = path.join(dir, itemName); if (item.isDirectory() || item.isSymbolicLink()) { let res = { @@ -1511,6 +1644,7 @@ exports.packageComponent = packageComponent; exports.deployComponent = deployComponent; exports.stageComponent = stageComponent; exports.activateComponent = activateComponent; +exports.revertComponent = revertComponent; exports.getComponents = getComponents; exports.getComponentFile = getComponentFile; exports.setComponentFile = setComponentFile; diff --git a/components/operationsValidation.js b/components/operationsValidation.js index 8a865d37bc..b50f0304fc 100644 --- a/components/operationsValidation.js +++ b/components/operationsValidation.js @@ -27,6 +27,7 @@ module.exports = { deployComponentValidator, stageComponentValidator, activateComponentValidator, + revertComponentValidator, setComponentFileValidator, getComponentFileValidator, dropComponentFileValidator, @@ -488,9 +489,11 @@ function deployComponentValidator(req) { deployment_timeout: Joi.number().min(0).optional(), force: Joi.boolean().optional(), ignore_replication_errors: Joi.boolean().optional(), - // Opt out of the two-phase (stage-then-activate) deploy and use the legacy one-shot path - // instead. Provided as an escape hatch for mixed-version clusters where a peer predates the - // stage_component/activate_component operations. Defaults to two-phase. + // If the activate phase fails on some nodes (leaving the cluster split across versions), swap the + // whole cluster back to the retained previous version before reporting the failure. Off by default. + revert_on_failure: Joi.boolean().optional(), + // Opt out of the two-phase (stage-then-activate) deploy and use the legacy one-shot path instead. + // Defaults to two-phase. two_phase: Joi.boolean().optional(), urlPath: URL_PATH_SCHEMA, // Deploy credentials. Each entry is npm registry auth (`registry`) or git host auth (`host`, @@ -565,3 +568,27 @@ function activateComponentValidator(req) { return validator.validateBySchema(req, activateSchema); } + +/** + * Validate revert_component requests — swap a component's live version back to its retained previous + * version. No build inputs (nothing is fetched or installed); just the project, an optional restart, + * and the replication controls. + * @param req + * @returns {*} + */ +function revertComponentValidator(req) { + const revertSchema = Joi.object({ + project: Joi.string() + .pattern(PROJECT_FILE_NAME_REGEX) + .required() + .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), + // The deployment being reverted, recorded as the rollback's `rollback_of` for the audit trail. + // Optional — revert operates on whatever version is currently live regardless. + deployment_id: Joi.string().optional(), + restart: Joi.alternatives().try(Joi.boolean(), Joi.string().valid('rolling')).optional(), + deployment_timeout: Joi.number().min(0).optional(), + ignore_replication_errors: Joi.boolean().optional(), + }); + + return validator.validateBySchema(req, revertSchema); +} diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index 3b52339b51..9acac09fb1 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -35,6 +35,7 @@ const SSE_PROGRESS_OPERATIONS = new Set([ terms.OPERATIONS_ENUM.DEPLOY_COMPONENT, terms.OPERATIONS_ENUM.STAGE_COMPONENT, terms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, + terms.OPERATIONS_ENUM.REVERT_COMPONENT, terms.OPERATIONS_ENUM.GET_DEPLOYMENT, terms.OPERATIONS_ENUM.READ_LOG, ]); diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 8bc2d83b6e..b0c6557949 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -542,6 +542,10 @@ function initializeOperationFunctionMap(): Map { for (const name of names) await fs.rm(path.join(COMPONENTS_ROOT, name), { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, '.deploy-previous'), { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, '.deploy-aside'), { recursive: true, force: true }); }); @@ -138,4 +139,28 @@ describe('deploy operations: stage_component / activate_component / deploy_compo const liveDir = path.join(COMPONENTS_ROOT, name); assert.match(await readIndex(liveDir), /op-oneshot/, 'component is live after a one-shot deploy'); }); + + it('revert_component swaps the live version back to the previous deployment', async () => { + const name = freshName(); + const liveDir = path.join(COMPONENTS_ROOT, name); + await operations.deployComponent({ project: name, payload: await makeComponentPayload('rev-v1') }); + await operations.deployComponent({ project: name, payload: await makeComponentPayload('rev-v2') }); + assert.match(await readIndex(liveDir), /rev-v2/, 'v2 is live before revert'); + + const res = await operations.revertComponent({ project: name }); + + assert.strictEqual(res.reverted, true); + assert.strictEqual(res.project, name); + assert.match(await readIndex(liveDir), /rev-v1/, 'revert restored v1 to live'); + }); + + it('revert_component rejects a component with no retained previous version', async () => { + const name = freshName(); + await operations.deployComponent({ project: name, payload: await makeComponentPayload('rev-once') }); + await assert.rejects(() => operations.revertComponent({ project: name }), /no previous version is retained/i); + }); + + it('revert_component requires a project', async () => { + await assert.rejects(() => operations.revertComponent({}), /project/i); + }); }); diff --git a/unitTests/components/deployPhaseValidators.test.js b/unitTests/components/deployPhaseValidators.test.js index d0ac511363..bc08a57bad 100644 --- a/unitTests/components/deployPhaseValidators.test.js +++ b/unitTests/components/deployPhaseValidators.test.js @@ -71,7 +71,36 @@ describe('activateComponentValidator', () => { }); }); -describe('deployComponentValidator two_phase flag', () => { +describe('revertComponentValidator', () => { + it('accepts a project-only revert', () => { + ok(validator.revertComponentValidator({ project: 'my_app' })); + }); + + it('accepts a deployment_id, restart, and ignore_replication_errors', () => { + ok( + validator.revertComponentValidator({ + project: 'my_app', + deployment_id: 'abc-123', + restart: 'rolling', + ignore_replication_errors: true, + }) + ); + }); + + it('requires a project', () => { + rejected(validator.revertComponentValidator({ deployment_id: 'abc-123' })); + }); + + it('rejects an invalid restart value', () => { + rejected(validator.revertComponentValidator({ project: 'my_app', restart: 'sideways' })); + }); +}); + +describe('deployComponentValidator two_phase + revert_on_failure flags', () => { + it('accepts revert_on_failure: true', () => { + ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', revert_on_failure: true })); + }); + it('accepts two_phase: false (legacy opt-out)', () => { ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: false })); }); diff --git a/unitTests/components/deployStaging.test.js b/unitTests/components/deployStaging.test.js index 283fb2f0a0..a9b3f36a8a 100644 --- a/unitTests/components/deployStaging.test.js +++ b/unitTests/components/deployStaging.test.js @@ -21,8 +21,10 @@ const { Application, stageApplication, activateApplication, + revertApplication, discardStagedApplication, DEPLOY_STAGING_DIR, + DEPLOY_PREVIOUS_DIR, ASIDE_STAGING_DIR, } = require('#src/components/Application'); const { getConfigPath } = require('#src/config/configUtils'); @@ -228,4 +230,72 @@ describe('two-phase deploy primitives (stage / activate / discard)', function () await fs.rm(dirPath, { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); }); + + // Deploy a version (stage + activate) through the primitives, returning the app. + async function deployVersion(name, marker) { + const app = new Application({ name, payload: await makeComponentPayload(marker) }); + await stageApplication(app); + await activateApplication(app); + return app; + } + + it('activate retains the outgoing version as .deploy-previous/', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + await deployVersion(name, 'v1'); + await deployVersion(name, 'v2'); + + const liveDir = path.join(COMPONENTS_ROOT, name); + const previousDir = path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR, name); + assert.match(await readMarker(liveDir), /v2/, 'live is the newest version'); + assert.match(await readMarker(previousDir), /v1/, 'the outgoing version is retained as previous'); + + await fs.rm(liveDir, { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); + + it('revertApplication swaps live <-> previous, and a second revert rolls forward again', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + await deployVersion(name, 'v1'); + const app = await deployVersion(name, 'v2'); + const liveDir = path.join(COMPONENTS_ROOT, name); + const previousDir = path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR, name); + + await revertApplication(app); + assert.match(await readMarker(liveDir), /v1/, 'reverted live back to v1'); + assert.match(await readMarker(previousDir), /v2/, 'the reverted-away v2 is now the previous'); + + await revertApplication(app); + assert.match(await readMarker(liveDir), /v2/, 'reverting the revert rolls forward to v2'); + assert.match(await readMarker(previousDir), /v1/, 'v1 is the previous again'); + + await fs.rm(liveDir, { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); + + it('revertApplication throws when there is no retained previous (deployed only once)', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + const app = await deployVersion(name, 'only'); // first-ever deploy: nothing retained as previous + + await assert.rejects(() => revertApplication(app), /no previous version is retained/i); + + await fs.rm(path.join(COMPONENTS_ROOT, name), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); + + it('only one previous is retained across three deploys (older previous evicted)', async () => { + const name = `stage_test_${process.pid}_${counter++}`; + await deployVersion(name, 'v1'); + await deployVersion(name, 'v2'); + await deployVersion(name, 'v3'); + + const previousDir = path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR, name); + assert.match(await readMarker(path.join(COMPONENTS_ROOT, name)), /v3/, 'live is v3'); + assert.match(await readMarker(previousDir), /v2/, 'previous is v2; v1 was evicted'); + + await fs.rm(path.join(COMPONENTS_ROOT, name), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_PREVIOUS_DIR), { recursive: true, force: true }); + await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); + }); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index c428264be1..1b47d74f51 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -297,6 +297,10 @@ export const OPERATIONS_ENUM = { // are unaffected. See components/Application.ts (stageApplication/activateApplication). STAGE_COMPONENT: 'stage_component', ACTIVATE_COMPONENT: 'activate_component', + // Swap a component's live version back to its retained previous version, cluster-wide. Backs + // customer-driven rollback (activate → test → revert) and swap-back on a partially-failed activate. + // See components/Application.ts (revertApplication). + REVERT_COMPONENT: 'revert_component', READ_TRANSACTION_LOG: 'read_transaction_log', DELETE_TRANSACTION_LOGS_BEFORE: 'delete_transaction_logs_before', INSTALL_NODE_MODULES: 'install_node_modules', diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index dcb70b6c26..c48a4a6c3c 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -288,6 +288,7 @@ requiredPermissions.set(functionsOperations.packageComponent.name, new (permissi requiredPermissions.set(functionsOperations.deployComponent.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.stageComponent.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.activateComponent.name, new (permission as any)(true, [])); +requiredPermissions.set(functionsOperations.revertComponent.name, new (permission as any)(true, [])); requiredPermissions.set( deploymentOperations.handleListDeployments.name, new (permission as any)(true, [], terms.OPERATIONS_ENUM.LIST_DEPLOYMENTS) From 722158b8ee66f5fefc9a63180223d8646c04d878 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 20 Jul 2026 10:54:40 -0400 Subject: [PATCH 08/22] fix(deploy): revert_on_failure must skip peers that never activated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- components/operations.js | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/components/operations.js b/components/operations.js index 5d47e7462f..aa0ec75efc 100644 --- a/components/operations.js +++ b/components/operations.js @@ -682,20 +682,40 @@ async function deployComponentTwoPhase(req, credentialReferences) { if (failed.length > 0) { let revertNote = ''; // Opt-in swap-back: some nodes went live and some didn't, leaving the cluster split across - // versions. When revert_on_failure is set, roll the whole cluster (incl. this origin) back to - // the retained previous version so it converges on one version again. Best-effort — a revert - // failure must not mask the original activate failure. + // versions. When revert_on_failure is set, roll the nodes that DID activate back to the + // retained previous version so the cluster reconverges. Best-effort — a revert failure must + // not mask the original activate failure. if (req.revert_on_failure) { try { emit('phase', { phase: 'revert', status: 'start' }); + // The origin activated, so revert it. await revertApplication(application); + // Revert ONLY the peers that successfully activated. A peer that FAILED to activate never + // swapped in the new version (validation/timeout/transport failures fire before + // activateApplication runs retainAsPrevious) — its live directory is still the correct + // pre-deploy version, and its `.deploy-previous` holds a copy from TWO deploys ago. Reverting + // it would roll it back an EXTRA version, splitting the cluster across three versions instead + // of reconverging on one. replicateOperation has no subset targeting (it fans to every + // server.node), so send point-to-point to the activated peers via sendOperationToNode, + // skipping recorder.getFailedPeers(). + const failedNodeNames = new Set(failed.map((peer) => peer.node).filter(Boolean)); + const activatedPeers = (server.nodes ?? []).filter((node) => !failedNodeNames.has(node.name)); const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { restart: req.restart, deploymentId: recorder.deploymentId, }); - await server.replication.replicateOperation(revertOp, {}); + revertOp.replicated = false; // point-to-point; the peer must not re-fan the revert + const revertResults = await Promise.allSettled( + activatedPeers.map((node) => server.replication.sendOperationToNode(node, revertOp)) + ); + const revertFailures = revertResults.filter((result) => result.status === 'rejected').length; emit('phase', { phase: 'revert', status: 'done' }); - revertNote = ` The cluster was rolled back to the previous version (revert_on_failure); verify with get_components.`; + revertNote = + ` Rolled the origin and ${activatedPeers.length - revertFailures} of ${activatedPeers.length} ` + + `activated peer(s) back to the previous version (revert_on_failure); the ${failed.length} peer(s) ` + + `that never activated were left on their current (correct) version.` + + (revertFailures > 0 ? ` ${revertFailures} peer revert(s) also failed.` : '') + + ` Verify with get_components.`; } catch (revertErr) { log.warn('revert_on_failure rollback failed', revertErr); revertNote = ` An automatic rollback (revert_on_failure) was attempted but also failed: ${revertErr?.message ?? revertErr}.`; From 2ac508f5ae80e6685b6cfe634ef908fc04847249 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 20 Jul 2026 11:01:59 -0400 Subject: [PATCH 09/22] fix(deploy): exclude this node from revert_on_failure point-to-point fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- components/operations.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/components/operations.js b/components/operations.js index aa0ec75efc..88844d0967 100644 --- a/components/operations.js +++ b/components/operations.js @@ -697,9 +697,17 @@ async function deployComponentTwoPhase(req, credentialReferences) { // it would roll it back an EXTRA version, splitting the cluster across three versions instead // of reconverging on one. replicateOperation has no subset targeting (it fans to every // server.node), so send point-to-point to the activated peers via sendOperationToNode, - // skipping recorder.getFailedPeers(). + // skipping recorder.getFailedPeers() — and skipping THIS node, which was already reverted + // directly above and would otherwise be reverted a second time (a bidirectional swap that + // flips it back to the just-activated version). server.nodes normally excludes self, but a + // not-yet-named node can slip in (knownNodes) and every point-to-point fan-out in the code + // base guards self anyway (bin/restart.ts). + const { getThisNodeName } = require('../server/nodeName.ts'); + const thisNode = getThisNodeName(); const failedNodeNames = new Set(failed.map((peer) => peer.node).filter(Boolean)); - const activatedPeers = (server.nodes ?? []).filter((node) => !failedNodeNames.has(node.name)); + const activatedPeers = (server.nodes ?? []).filter( + (node) => node.name !== thisNode && !failedNodeNames.has(node.name) + ); const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { restart: req.restart, deploymentId: recorder.deploymentId, From baca6b7165c8427d772d6a0c2a6b2dad55d30586 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 20 Jul 2026 11:11:49 -0400 Subject: [PATCH 10/22] test(deploy): extract + cover revert_on_failure node targeting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- components/operations.js | 38 ++++++++++--------- .../components/deployPhaseOperations.test.js | 35 +++++++++++++++++ 2 files changed, 56 insertions(+), 17 deletions(-) diff --git a/components/operations.js b/components/operations.js index 88844d0967..92673d3491 100644 --- a/components/operations.js +++ b/components/operations.js @@ -690,24 +690,12 @@ async function deployComponentTwoPhase(req, credentialReferences) { emit('phase', { phase: 'revert', status: 'start' }); // The origin activated, so revert it. await revertApplication(application); - // Revert ONLY the peers that successfully activated. A peer that FAILED to activate never - // swapped in the new version (validation/timeout/transport failures fire before - // activateApplication runs retainAsPrevious) — its live directory is still the correct - // pre-deploy version, and its `.deploy-previous` holds a copy from TWO deploys ago. Reverting - // it would roll it back an EXTRA version, splitting the cluster across three versions instead - // of reconverging on one. replicateOperation has no subset targeting (it fans to every - // server.node), so send point-to-point to the activated peers via sendOperationToNode, - // skipping recorder.getFailedPeers() — and skipping THIS node, which was already reverted - // directly above and would otherwise be reverted a second time (a bidirectional swap that - // flips it back to the just-activated version). server.nodes normally excludes self, but a - // not-yet-named node can slip in (knownNodes) and every point-to-point fan-out in the code - // base guards self anyway (bin/restart.ts). + // Revert ONLY the peers that successfully activated (see selectRevertTargets): every known + // node minus the ones that failed to activate (still on the correct version) and minus this + // node (already reverted directly above; a second bidirectional revert would flip it back). + // replicateOperation has no subset targeting, so send point-to-point via sendOperationToNode. const { getThisNodeName } = require('../server/nodeName.ts'); - const thisNode = getThisNodeName(); - const failedNodeNames = new Set(failed.map((peer) => peer.node).filter(Boolean)); - const activatedPeers = (server.nodes ?? []).filter( - (node) => node.name !== thisNode && !failedNodeNames.has(node.name) - ); + const activatedPeers = selectRevertTargets(server.nodes, failed, getThisNodeName()); const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { restart: req.restart, deploymentId: recorder.deploymentId, @@ -1185,6 +1173,21 @@ function describePeers(failedPeers) { return failedPeers.map((peer) => `${peer.node ?? 'unknown'} (${peer.error?.message ?? 'unknown error'})`).join(', '); } +// Choose which peers a revert_on_failure swap-back should target: every known node EXCEPT +// - `thisNodeName`: the origin, already reverted directly by the caller — a second (bidirectional) +// revert would flip it back to the just-activated version; and +// - any node in `failedPeers`: it never activated (its failure fired before activateApplication ran +// retainAsPrevious), so its live directory is still the correct pre-deploy version and reverting it +// would roll it back an EXTRA version onto a two-deploys-ago copy. +// Pure and exported so the node-targeting logic (which had two review-caught bugs — the failed-peer +// skip and the self-skip) is unit-testable without a live cluster. `nodes` is `server.nodes`, which +// normally already excludes self, but a not-yet-named node can slip in (knownNodes) so self is guarded +// here regardless — matching every other point-to-point fan-out in the code base (bin/restart.ts). +function selectRevertTargets(nodes, failedPeers, thisNodeName) { + const failedNodeNames = new Set((failedPeers ?? []).map((peer) => peer.node).filter(Boolean)); + return (nodes ?? []).filter((node) => node?.name !== thisNodeName && !failedNodeNames.has(node?.name)); +} + // Reclaim the payload tarball for large deploys once every peer has installed from the blob. Dropping // the reference before finish() folds the null into the single terminal write, which unlinks the file // locally and replicates the null so peers drop their copies too. Metadata is retained. Kept when a @@ -1673,6 +1676,7 @@ exports.deployComponent = deployComponent; exports.stageComponent = stageComponent; exports.activateComponent = activateComponent; exports.revertComponent = revertComponent; +exports.selectRevertTargets = selectRevertTargets; // exported for unit testing the revert_on_failure node-targeting exports.getComponents = getComponents; exports.getComponentFile = getComponentFile; exports.setComponentFile = setComponentFile; diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index 757f658209..fd93a84e13 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -163,4 +163,39 @@ describe('deploy operations: stage_component / activate_component / deploy_compo it('revert_component requires a project', async () => { await assert.rejects(() => operations.revertComponent({}), /project/i); }); + + // The revert_on_failure fan-out itself needs a live multi-node cluster (harper-pro's replicator) to + // run end-to-end, but its node-targeting is a pure function — and the exact spot that had two + // review-caught bugs (skip failed peers, skip self). Exercise it directly. + describe('selectRevertTargets (revert_on_failure node targeting)', () => { + const nodes = [{ name: 'origin' }, { name: 'peerA' }, { name: 'peerB' }, { name: 'peerC' }]; + + it('returns activated peers, excluding this node and failed peers', () => { + const failed = [{ node: 'peerB', status: 'failed' }]; + const targets = operations.selectRevertTargets(nodes, failed, 'origin').map((n) => n.name); + assert.deepStrictEqual(targets.sort(), ['peerA', 'peerC'], 'peerB (failed) and origin (self) excluded'); + }); + + it('excludes THIS node even when it is present in server.nodes (bidirectional double-revert guard)', () => { + const targets = operations.selectRevertTargets(nodes, [], 'origin').map((n) => n.name); + assert.ok(!targets.includes('origin'), 'self must never receive a self-directed revert'); + assert.deepStrictEqual(targets.sort(), ['peerA', 'peerB', 'peerC']); + }); + + it('excludes every failed peer (they never activated and are on the correct version)', () => { + const failed = [ + { node: 'peerA', status: 'failed' }, + { node: 'peerC', status: 'failed' }, + ]; + const targets = operations.selectRevertTargets(nodes, failed, 'origin').map((n) => n.name); + assert.deepStrictEqual(targets, ['peerB'], 'only the one activated peer is a revert target'); + }); + + it('is safe with empty/undefined nodes and failed lists, and ignores failed entries with no node name', () => { + assert.deepStrictEqual(operations.selectRevertTargets(undefined, undefined, 'origin'), []); + assert.deepStrictEqual(operations.selectRevertTargets([], [{ node: null }], 'origin'), []); + const targets = operations.selectRevertTargets(nodes, [{ node: null }], 'origin').map((n) => n.name); + assert.deepStrictEqual(targets.sort(), ['peerA', 'peerB', 'peerC'], 'a null-node failed entry drops nobody'); + }); + }); }); From 488d172047506fb67c341448b5bbee0e95e9c645 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 20 Jul 2026 11:35:08 -0400 Subject: [PATCH 11/22] fix(deploy): validate deployment_id charset; validate staged build during stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings from kriszyp's review: - deployment_id becomes a staging-dir path segment (.deploy-staging//), 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 --- DESIGN.md | 11 +++++++++++ components/operations.js | 7 +++++++ components/operationsValidation.js | 16 ++++++++++++---- .../components/deployPhaseValidators.test.js | 12 ++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 4674553135..cb16318c20 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -321,6 +321,17 @@ could leave a peer half-installed after other peers had already restarted onto t request/response contract is unchanged; only the SSE phase names differ (`stage`/`activate` vs the old `prepare`/`replicate`). `two_phase: false` forces the legacy one-shot path. +**Scope of the barrier's guarantee: fetch + install, not load.** The cluster-wide "nobody activates +until everybody staged" guarantee covers the download/`npm pack` and `npm install` steps — the slow, +failure-prone work. The pre-go-live component _load_ check (`loadValidateComponent`, which surfaces a +component that installs cleanly but throws at load) runs during stage on the origin and on any node +whose stage executes on a worker (the op-API worker for a standalone `stage_component`), but it is a +no-op on the main thread — and replicated peer executions of `stage_component` run on the main thread +(`replicateOperation` → `sendOperationToNode` execute there), where app code deliberately isn't +loaded. So a load-time-only fault on a peer is not caught by the barrier; it surfaces at +activate/restart like any other. Gating load-time faults cluster-wide would require dispatching the +throwaway load to a worker on each peer during stage — a possible follow-up, not done here. + The staging directory (`.deploy-staging//`) lives **under the components root**, not in `os.tmpdir()`, even though its contents are transient. This is deliberate and load-bearing: the go-live step is `rename(stagingDir, liveDir)`, which is only atomic when both paths share a diff --git a/components/operations.js b/components/operations.js index 92673d3491..15cb44b498 100644 --- a/components/operations.js +++ b/components/operations.js @@ -800,6 +800,13 @@ async function stageComponent(req) { emit('phase', { phase: 'stage', status: 'start' }); await stageApplication(application); + // Surface load-time errors on the staged build before it can be activated, matching what + // deployComponentTwoPhase does on the origin (and closing the gap where a standalone + // stage_component never validated at all). Loads the STAGED dir (application.buildDirPath). This + // is a no-op on the main thread — where replicated peer executions run — because app code must + // not load there; so load-time faults are gated where the stage runs on a worker (origin op-API + // worker, standalone stage), while fetch + install remain gated cluster-wide by the barrier. + await loadValidateComponent({ dirPath: application.buildDirPath, emit }); emit('phase', { phase: 'stage', status: 'done' }); const response = { message: `Staged component: ${req.project}`, project: req.project, staged: true }; diff --git a/components/operationsValidation.js b/components/operationsValidation.js index b50f0304fc..a726f41b8d 100644 --- a/components/operationsValidation.js +++ b/components/operationsValidation.js @@ -551,8 +551,13 @@ function activateComponentValidator(req) { .required() .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), // Identifies which staged build to activate. Required for a standalone activate; the - // deploy_component orchestrator supplies it as the deployment id it staged under. - deployment_id: Joi.string().optional(), + // deploy_component orchestrator supplies it as the deployment id it staged under. Constrained to + // the same safe charset as `project` because it becomes a path segment of the staging directory + // (`.deploy-staging//`) — without this a `../` value would resolve the staging + // source outside `.deploy-staging`. Defense-in-depth (the op is super_user-only). + deployment_id: Joi.string().pattern(PROJECT_FILE_NAME_REGEX).optional().messages({ + 'string.pattern.base': `'deployment_id' must only contain letters, numbers, dashes, and underscores`, + }), package: Joi.string().optional(), install_command: Joi.string().optional(), install_timeout: Joi.number().optional(), @@ -583,8 +588,11 @@ function revertComponentValidator(req) { .required() .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), // The deployment being reverted, recorded as the rollback's `rollback_of` for the audit trail. - // Optional — revert operates on whatever version is currently live regardless. - deployment_id: Joi.string().optional(), + // Optional — revert operates on whatever version is currently live regardless. Same safe charset + // as elsewhere (it is an id, and this keeps the deploy family's `deployment_id` consistent). + deployment_id: Joi.string().pattern(PROJECT_FILE_NAME_REGEX).optional().messages({ + 'string.pattern.base': `'deployment_id' must only contain letters, numbers, dashes, and underscores`, + }), restart: Joi.alternatives().try(Joi.boolean(), Joi.string().valid('rolling')).optional(), deployment_timeout: Joi.number().min(0).optional(), ignore_replication_errors: Joi.boolean().optional(), diff --git a/unitTests/components/deployPhaseValidators.test.js b/unitTests/components/deployPhaseValidators.test.js index bc08a57bad..daa85573e6 100644 --- a/unitTests/components/deployPhaseValidators.test.js +++ b/unitTests/components/deployPhaseValidators.test.js @@ -69,6 +69,18 @@ describe('activateComponentValidator', () => { it('rejects an invalid restart value', () => { rejected(validator.activateComponentValidator({ project: 'my_app', restart: 'sideways' })); }); + + it('rejects a path-traversal deployment_id (it becomes a staging-dir path segment)', () => { + for (const bad of ['../evil', 'a/b', 'dep/../..', '.', '..']) { + rejected(validator.activateComponentValidator({ project: 'my_app', deployment_id: bad })); + } + }); + + it('accepts a normal UUID-shaped deployment_id', () => { + ok( + validator.activateComponentValidator({ project: 'my_app', deployment_id: '41faded8-6cf5-4a2a-95f8-863e7ea498fa' }) + ); + }); }); describe('revertComponentValidator', () => { From d80e695316ebbdac3fb3661d117c92cf7e52dc43 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 10:03:53 -0400 Subject: [PATCH 12/22] refactor(deploy): fold stage/activate into deploy_component (internal _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=` -> deploy_component deployment_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 --- DESIGN.md | 63 +++-- bin/cliOperations.ts | 35 ++- components/deploymentRecorder.ts | 20 ++ components/operations.js | 261 ++++++++---------- components/operationsValidation.js | 83 +----- server/serverHelpers/serverHandlers.js | 2 - server/serverHelpers/serverUtilities.ts | 8 - .../components/deployPhaseOperations.test.js | 21 +- .../components/deployPhaseValidators.test.js | 95 ++----- utility/hdbTerms.ts | 14 +- utility/operation_authorization.ts | 2 - 11 files changed, 241 insertions(+), 363 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index cb16318c20..75a2921ad6 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -308,25 +308,38 @@ other future cause would still report success silently; that's a deferred, separ ## Two-phase deploy: stage then activate (`components/Application.ts`, `components/operations.js`) -`deploy_component` splits into two replicated phases so a cluster deploy is all-or-nothing at the -point of go-live. **Phase 1 (`stage_component`)** builds the incoming version — download/`npm pack` -(incl. a git clone), extract, `npm install` — into a hidden staging directory on every node. -**Phase 2 (`activate_component`)** atomically renames the staged copy into the live component path and -restarts. `deploy_component` orchestrates the two: it stages on the origin, replicates -`stage_component` to peers, **waits for every node to report a successful stage before any node -activates** (`ignore_replication_errors` opts out of the barrier), then replicates -`activate_component`. If a node can't fetch the package or fails `npm install`, it fails during -staging while the live component is still untouched _on every node_ — where the old one-shot path -could leave a peer half-installed after other peers had already restarted onto the new code. The -request/response contract is unchanged; only the SSE phase names differ (`stage`/`activate` vs the -old `prepare`/`replicate`). `two_phase: false` forces the legacy one-shot path. +`deploy_component` runs internally as two replicated phases so a cluster deploy is all-or-nothing at +the point of go-live. **Phase 1 (stage)** builds the incoming version — download/`npm pack` (incl. a +git clone), extract, `npm install` — into a hidden staging directory on every node. **Phase 2 +(activate)** atomically renames the staged copy into the live component path and restarts. The origin +stages locally, **waits for every node to report a successful stage before any node activates** +(`ignore_replication_errors` opts out of the barrier), then activates. If a node can't fetch the +package or fails `npm install`, it fails during staging while the live component is still untouched _on +every node_ — where the old one-shot path could leave a peer half-installed after other peers had +already restarted onto the new code. The request/response contract is unchanged; only the SSE phase +names differ (`stage`/`activate` vs the old `prepare`/`replicate`). `two_phase: false` forces the +legacy one-shot path. + +**There is only one public operation — `deploy_component`.** The two phases are NOT separate public +operations; the peer fan-out is `deploy_component` itself tagged with an internal `_phase: 'stage' | +'activate'` marker (the same `_`-prefixed internal-field convention peers already branch on, alongside +`_deploymentId`). `deployComponent` dispatches: a replicated execution with `_phase` runs the peer +stage/activate work (`deployPhaseStage` / `deployPhaseActivate`) and never re-fans; a public call runs +the origin orchestrator. Two public properties expose the phases when an operator wants them separated +(e.g. pre-stage the cluster now, flip later — or a CI-stages / approver-activates split): `activate: +false` stages cluster-wide and stops, returning the `deployment_id` in a `staged` state; passing that +`deployment_id` back to `deploy_component` (with no new payload) activates the already-staged build. +This was a deliberate API-surface choice (harper#1849 review): peer fan-out needs a wire format, not +two extra public ops, and folding the phases into `deploy_component` keeps the surface at one op while +the convergence properties cover the stage-now/activate-later use case. (`revert_component` stays a +distinct public op — it is a rollback, not a deploy phase.) **Scope of the barrier's guarantee: fetch + install, not load.** The cluster-wide "nobody activates until everybody staged" guarantee covers the download/`npm pack` and `npm install` steps — the slow, failure-prone work. The pre-go-live component _load_ check (`loadValidateComponent`, which surfaces a component that installs cleanly but throws at load) runs during stage on the origin and on any node -whose stage executes on a worker (the op-API worker for a standalone `stage_component`), but it is a -no-op on the main thread — and replicated peer executions of `stage_component` run on the main thread +whose stage executes on a worker (e.g. the op-API worker for an `activate: false` stage), but it is a +no-op on the main thread — and replicated peer stage executions run on the main thread (`replicateOperation` → `sendOperationToNode` execute there), where app code deliberately isn't loaded. So a load-time-only fault on a peer is not caught by the barrier; it surfaces at activate/restart like any other. Gating load-time faults cluster-wide would require dispatching the @@ -343,9 +356,9 @@ component, and it is **not** the watched base of any component's file watcher (t each live component dir, `EntryHandler`/`deriveCommonPatternBase`) — so building here fires no restart-on-change events and needs no `deploy:start` watcher suppression. That suppression is now scoped to `activateApplication`, the only phase that writes the live path. Staging is deterministic -from the deployment id precisely so `activate_component` (a separate replicated operation, and on -peers a separate invocation from `stage_component`) can reconstruct the same path the stage built — -peers build a fresh `Application` per sub-operation, so there is no shared in-memory handle to rely +from the deployment id precisely so the activate phase (a separate replicated `deploy_component` +invocation on peers, tagged `_phase: 'activate'`) can reconstruct the same path the stage built — +peers build a fresh `Application` per phase invocation, so there is no shared in-memory handle to rely on. The deployment id sits ABOVE the component name (`…//`, not `…//`) for two reasons: the leaf directory's basename is then the real component name, which the pre-go-live validation load needs (`componentLoader` keys the `ApplicationScope` and @@ -363,22 +376,22 @@ phases by deployment id. When `system` is excluded from a narrow `REPLICATION_DA caller passes `two_phase: false`, or the invocation is a peer replaying a one-shot deploy, `deploy_component` falls back to `deployComponentOneShot` (the previous behavior, preserved verbatim). Cross-version skew is a non-issue by policy — a cluster stays in lockstep on its Harper version, so -every node understands `stage_component`/`activate_component` — which is why there is no capability -negotiation on the fan-out. +every node understands the `_phase`-tagged `deploy_component` fan-out — which is why there is no +capability negotiation on it. **Replicator contract this rides on (`harper-pro/replication/replicator.ts`).** `server.replication.replicateOperation(op, {onPeerResult})` fans `op` to every node in `server.nodes` in parallel, setting `op.replicated = false` on the copy it sends so a peer never re-fans (the deploy -handlers instead detect a replicated execution by the presence of `_deploymentId`, which is always set -on the sub-operations). Per-peer failures never throw — `sendOperationToNode` rejections are caught and +handlers additionally detect a replicated execution by the presence of `_deploymentId` — always set on +the sub-operations — and run the peer stage/activate work off the `_phase` marker without re-fanning). Per-peer failures never throw — `sendOperationToNode` rejections are caught and surface as `{status:'failed', reason, node}` entries in the returned `replicated[]` array and via `onPeerResult`, which is exactly the shape `DeploymentRecorder.normalizePeerResult` consumes. Peers authenticate node-to-node by TLS certificate, and the receive side runs the op via `server.operation(data, {user}, !isAuthorizedNode)` — for a trusted cluster node the authorize flag is -`false`, so a replicated super-user op skips the permission gate. That is why `stage_component` / -`activate_component` / `revert_component` (registered with the same `permission(true, [])` as -`deploy_component`, dispatched by `operation` name) replicate without an `hdb_user`, identically to the -long-proven `deploy_component` fan-out. +`false`, so a replicated super-user op skips the permission gate. That is why the `_phase`-tagged +`deploy_component` fan-out and `revert_component` (registered with the same `permission(true, [])`, +dispatched by `operation` name) replicate without an `hdb_user`, identically to the long-proven +one-shot `deploy_component` fan-out. **Reversibility: retained previous + `revert_component`.** `activateApplication` no longer discards the outgoing live version — it retains it as `.deploy-previous/` (`retainAsPrevious`, evicting the diff --git a/bin/cliOperations.ts b/bin/cliOperations.ts index c0f8daa67f..51e022f0d9 100644 --- a/bin/cliOperations.ts +++ b/bin/cliOperations.ts @@ -21,14 +21,19 @@ import { initConfig, getConfigPath } from '../config/configUtils.ts'; const OP_ALIASES = { deploy: 'deploy_component', package: 'package_component', - // Two-phase deploy: `harper stage` packages + uploads the incoming version to a hidden staging - // dir cluster-wide (no go-live); `harper activate` swaps a staged deployment live; `harper revert` - // swaps the live version back to its retained previous version. - stage: 'stage_component', - activate: 'activate_component', revert: 'revert_component', }; +// CLI verbs that are sugar over `deploy_component` with preset properties (the stage/activate phases +// are folded into deploy_component; there are no separate stage/activate operations). `harper stage` +// packages + uploads the incoming version to a hidden staging dir cluster-wide and stops before +// go-live (`activate: false`), printing the staged deployment_id; `harper activate deployment_id=` +// takes that staged deployment live (no upload). +const OP_VERB_PROPS: Record> = { + stage: { operation: 'deploy_component', activate: false }, + activate: { operation: 'deploy_component' }, +}; + // Shown for any local-instance connection failure (missing pid, missing/stale domain // socket, or a refused/ENOENT connect against it) — they're all the same user-facing // scenario: Harper isn't running. Remote-target failures keep the detailed error instead, @@ -40,7 +45,7 @@ const LOCAL_NOT_RUNNING_MESSAGE = 'Harper is not running. Use `harperdb run` (or // deploy completes. Add an operation here only after wiring its server-side // SSE_PROGRESS_OPERATIONS entry — otherwise the server returns the buffered JSON path and // the SSE parser sees no events. -const SSE_OPERATIONS = new Set(['deploy_component', 'stage_component', 'activate_component', 'revert_component']); +const SSE_OPERATIONS = new Set(['deploy_component', 'revert_component']); // Properties on `req` that the CLI itself uses for transport/UX, not the operations API. // They never get serialized into the request body. @@ -161,11 +166,12 @@ function operationFields(req: any): any { export { cliOperations, buildRequest }; -// Package the current working directory into a multipart tarball upload. Shared by `deploy` and -// `stage` — both send the incoming component version as a `payload` (unless a `package` identifier is -// given, in which case the server fetches it and there is nothing to upload). +// Package the current working directory into a multipart tarball upload for deploy_component. Covers +// `harper deploy` and `harper stage` (deploy_component with activate:false) — both upload the incoming +// version. Nothing to package when a `package` identifier is given (the server fetches it) or when +// activating a previously-staged deployment (`deployment_id`, i.e. `harper activate`). const packageCwdForUpload = async (req) => { - if (req.package) { + if (req.package || req.deployment_id) { return; } @@ -197,10 +203,10 @@ const packageCwdForUpload = async (req) => { }; const PREPARE_OPERATION: any = { + // deploy_component covers `harper deploy` and `harper stage` (activate:false); packageCwdForUpload + // itself skips the upload for a `package` identifier or a `deployment_id` activate. revert takes no + // payload, so it needs no prep step. deploy_component: packageCwdForUpload, - // `harper stage` uploads the same tarball as `harper deploy`. activate/revert take no payload - // (they operate on an already-staged / already-retained version), so they need no prep step. - stage_component: packageCwdForUpload, }; /** @@ -211,6 +217,9 @@ function buildRequest(): any { for (const arg of process.argv.slice(2)) { if (OP_ALIASES.hasOwnProperty(arg)) { req.operation = OP_ALIASES[arg]; + } else if (OP_VERB_PROPS.hasOwnProperty(arg)) { + // Sugar verb (stage/activate) → deploy_component + preset props (e.g. activate:false). + Object.assign(req, OP_VERB_PROPS[arg]); } else if (arg.includes('=')) { let [first, ...rest] = arg.split('='); let restStr: any = rest.join('='); diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index fb09d4107d..92460cdc04 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -455,6 +455,26 @@ export class DeploymentRecorder { } } +/** + * Best-effort terminal-status update for an existing deployment row by id, used when a later operation + * finishes a deployment the current process didn't record with a live DeploymentRecorder — e.g. + * `deploy_component({ deployment_id })` activating a build that an earlier stage-and-stop left in the + * `staged` state. No-op when the row (or the table) is absent; observability only, so callers treat a + * failure here as non-fatal. + */ +export async function markDeploymentTerminal( + deploymentId: string, + status: 'success' | 'failed' | 'rolled_back' | 'staged' +): Promise { + const table = (databases as any).system?.[terms.SYSTEM_TABLE_NAMES.DEPLOYMENT_TABLE_NAME]; + if (!table) return; + const row = await table.get(deploymentId); + if (!row) return; + row.status = status; + row.completed_at = Date.now(); + await table.put(row); +} + // Default peer-wait budget for the hdb_deployment row to replicate. A deploy is a rare, // heavyweight, user-initiated operation, and the `system`-table replication channel can be // backlogged behind unrelated writes when several deploys land in succession, so the row can diff --git a/components/operations.js b/components/operations.js index 15cb44b498..f5ca5fabdf 100644 --- a/components/operations.js +++ b/components/operations.js @@ -36,7 +36,12 @@ const { DEPLOY_PREVIOUS_DIR, } = require('./Application.ts'); const { server } = require('../server/Server.ts'); -const { DeploymentRecorder, awaitDeploymentRow, DEFAULT_AWAIT_ROW_TIMEOUT_MS } = require('./deploymentRecorder.ts'); +const { + DeploymentRecorder, + awaitDeploymentRow, + markDeploymentTerminal, + DEFAULT_AWAIT_ROW_TIMEOUT_MS, +} = require('./deploymentRecorder.ts'); const { ProgressEmitter } = require('../server/serverHelpers/progressEmitter.ts'); /** @@ -392,6 +397,14 @@ async function deployComponent(req) { throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); } + const isReplicatedExecution = typeof req._deploymentId === 'string'; + + // Internal peer-phase execution. The two-phase cluster fan-out rides deploy_component itself, tagged + // with an internal `_phase` marker (`stage`/`activate`) rather than separate public operations — so + // the wire format is deploy_component + `_phase`, and only deploy_component is publicly exposed. + if (isReplicatedExecution && req._phase === 'stage') return deployPhaseStage(req); + if (isReplicatedExecution && req._phase === 'activate') return deployPhaseActivate(req); + // Ingest any provided credential token into the secrets store so the credential lives as // replicated ciphertext (reference, not embed); already-reference entries pass through, and with // no custody a literal token stays as a transient, this-node-only fallback (#1158). Peers @@ -402,18 +415,18 @@ async function deployComponent(req) { // token is not — it is used only for this node's install, then stripped before replication. const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); - // A peer replaying a replicated ONE-SHOT deploy_component arrives with `_deploymentId` set. In - // two-phase mode the origin never sends deploy_component to peers (it sends stage_component / - // activate_component), so `_deploymentId` here always means "legacy peer". - const isReplicatedExecution = typeof req._deploymentId === 'string'; - // Two-phase is the default, but it leans on the system table's replication channel to carry the // payload and correlate the stage/activate steps across the cluster. Fall back to the legacy - // one-shot deploy when: the caller opted out (`two_phase: false`); this is a peer replaying a - // one-shot deploy; or `system` isn't replicated on this node (a narrow REPLICATION_DATABASES). + // one-shot deploy when: the caller opted out (`two_phase: false`); this is a legacy peer replaying a + // one-shot deploy (replicated, no `_phase`); or `system` isn't replicated on this node. if (req.two_phase === false || isReplicatedExecution || !isSystemDatabaseReplicated()) { return deployComponentOneShot(req, credentialReferences, isReplicatedExecution); } + + // `deployment_id` with no fresh payload → activate a previously-staged deployment (the second half of + // a stage-then-activate-later flow). Otherwise run the full stage+activate (which itself honors + // `activate: false` to stop after the cluster-wide staged barrier). + if (req.deployment_id) return deployComponentActivateExisting(req, credentialReferences); return deployComponentTwoPhase(req, credentialReferences); } @@ -615,7 +628,7 @@ async function deployComponentTwoPhase(req, credentialReferences) { // ===== PHASE 1: STAGE — build on every node; nothing goes live. ===== emit('phase', { phase: 'stage', status: 'start' }); await stageApplication(application); - const stageOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.STAGE_COMPONENT); + const stageOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.DEPLOY_COMPONENT, { phase: 'stage' }); const stageResp = await server.replication.replicateOperation(stageOp, { onPeerResult: recordPeer }); if (stageResp?.replicated) recorder.recordPeers(stageResp.replicated); emit('phase', { phase: 'stage', status: 'done' }); @@ -637,6 +650,20 @@ async function deployComponentTwoPhase(req, credentialReferences) { // Validate the staged build before go-live (loads from the staging dir; see loadValidateComponent). await loadValidateComponent({ dirPath: application.buildDirPath, emit }); + // `activate: false` — stage-and-stop. The build is verified on every node; leave the row in a + // `staged` state and return its deployment_id so a later deploy_component({deployment_id}) can + // take it live. Nothing has gone live anywhere. + if (req.activate === false) { + emit('phase', { phase: 'staged', status: 'done' }); + await recorder.finish('staged'); + return { + message: `Staged component: ${application.name}`, + project: application.name, + staged: true, + deployment_id: recorder.deploymentId, + }; + } + // ===== PHASE 2: ACTIVATE — atomic swap + restart, now the bits are in place everywhere. ===== // Persist root config now (not before staging) so a `package` config never points at a version // that failed to stage. @@ -646,7 +673,8 @@ async function deployComponentTwoPhase(req, credentialReferences) { emit('phase', { phase: 'activate', status: 'start' }); await activateApplication(application); - const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, { + const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.DEPLOY_COMPONENT, { + phase: 'activate', restart: req.restart, deploymentId: recorder.deploymentId, }); @@ -739,52 +767,21 @@ async function deployComponentTwoPhase(req, credentialReferences) { } /** - * stage_component — phase 1 of a two-phase deploy, as a first-class operation. Builds the incoming - * version into the hidden staging directory on this node (and, when invoked directly rather than via - * replication, replicates the stage across the cluster). Never writes the live component directory, - * writes no root config, and never restarts — so it is safe to run cluster-wide and gate on. - * - * Reached three ways: directly by an operator (stage now, activate later), by a peer replaying a - * replicated stage (`_deploymentId` set), and indirectly — deploy_component drives staging itself, - * so it does not call this handler. + * Peer stage phase (internal — NOT a public operation). Runs on a peer when the origin fans out + * deploy_component tagged `_phase: 'stage'`: fetch the tarball from the replicated hdb_deployment row, + * build + `npm install` into the hidden staging directory, and load-validate — never touching the live + * path, writing config, or restarting. A failure here fails this peer's stage, which the origin's + * barrier catches. No recorder (the origin owns the row) and no re-replication. */ -async function stageComponent(req) { - if (req.project) { - req.project = path.parse(req.project).name; - } else if (req.package) { - req.project = getProjectNameFromPackage(req.package); - } - const validation = validator.stageComponentValidator(req); - if (validation) throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); - - const { ingestCredentials, resolveCredentials } = require('./secretOperations.ts'); - req.credentials = await ingestCredentials(req, req.credentials, req.project); - const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); - if (req.package) assertNotProtectedCoreComponent(req.project, req.force); - - const isReplicatedExecution = typeof req._deploymentId === 'string'; - const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); - if (emitter && !req.progress) req.progress = emitter; - // Standalone stage records so it has a deployment_id + payload row for peers to fetch; a peer - // replaying the stage skips recording (the origin owns the row). - const recorder = isReplicatedExecution - ? null - : await DeploymentRecorder.create({ - project: req.project, - package_identifier: req.package ?? null, - user: req.hdb_user?.username, - restart_mode: null, - credentials: credentialReferences.length ? credentialReferences : null, - emitter, - }); - if (recorder) req._deploymentId = recorder.deploymentId; - const emit = (event, data) => emitter?.emit(event, data); +async function deployPhaseStage(req) { + const { resolveCredentials } = require('./secretOperations.ts'); + const emitter = null; // peers stream nothing back; the origin owns the emitter/recorder + const emit = () => {}; const installCapture = createInstallCapture(); let application; - try { - const extractionPayload = await sourceExtractionPayload({ req, recorder, isReplicatedExecution }); - const resolvedCredentials = await resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution }); + const extractionPayload = await sourceExtractionPayload({ req, recorder: null, isReplicatedExecution: true }); + const resolvedCredentials = await resolveNodeCredentials({ req, resolveCredentials, isReplicatedExecution: true }); application = buildDeployApplication({ req, extractionPayload, @@ -794,123 +791,84 @@ async function stageComponent(req) { emitter, emit, }); - if (credentialReferences.length) req.credentials = credentialReferences; - else delete req.credentials; - delete req.progress; - - emit('phase', { phase: 'stage', status: 'start' }); await stageApplication(application); - // Surface load-time errors on the staged build before it can be activated, matching what - // deployComponentTwoPhase does on the origin (and closing the gap where a standalone - // stage_component never validated at all). Loads the STAGED dir (application.buildDirPath). This - // is a no-op on the main thread — where replicated peer executions run — because app code must - // not load there; so load-time faults are gated where the stage runs on a worker (origin op-API - // worker, standalone stage), while fetch + install remain gated cluster-wide by the barrier. + // Surface load-time errors on the staged build (no-op on the main thread, where replicated peer + // executions run — app code must not load there; see loadValidateComponent + DESIGN.md). await loadValidateComponent({ dirPath: application.buildDirPath, emit }); - emit('phase', { phase: 'stage', status: 'done' }); - - const response = { message: `Staged component: ${req.project}`, project: req.project, staged: true }; - if (recorder) { - response.deployment_id = recorder.deploymentId; - // Replicate staging to peers so the bits land cluster-wide. Keep the payload in the body only - // when the row can't carry it (system not replicated on this node). - if (isSystemDatabaseReplicated()) delete req.payload; - const stageOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.STAGE_COMPONENT, { - includePayload: !isSystemDatabaseReplicated(), - }); - recorder.seal(); - const rep = await server.replication.replicateOperation(stageOp, { - onPeerResult: (result) => { - recorder.recordPeer(result); - emit('peer', result); - }, - }); - if (rep?.replicated) recorder.recordPeers(rep.replicated); - if (!req.ignore_replication_errors) { - const failed = recorder.getFailedPeers(); - if (failed.length > 0) { - throw new ServerError( - `Component '${req.project}' failed to stage on ${failed.length} of ` + - `${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. ` + - `See deployment ${recorder.deploymentId} (get_deployment).` - ); - } - } - // Leave the row in a 'staged' resting state — the build exists cluster-wide but nothing is - // live yet; a subsequent activate_component (or the caller) takes it live. - emit('phase', { phase: 'staged', status: 'done' }); - await recorder.finish('staged'); - } - return response; + return { message: `Staged component: ${req.project}`, project: req.project, staged: true }; } catch (err) { if (application) await discardStagedApplication(application).catch(() => {}); - throw await finalizeDeployFailure({ err, recorder, installCapture, emit }); + throw await finalizeDeployFailure({ err, recorder: null, installCapture, emit }); } } /** - * activate_component — phase 2 of a two-phase deploy, as a first-class operation. Atomically swaps a - * previously-staged build (identified by `deployment_id`) into the live component directory, persists - * the component's root config for a `package` deploy, and restarts as requested. When invoked - * directly it also replicates the activation across the cluster. - * - * Reached two ways: directly by an operator to take a prior stage live, and by a peer replaying a - * replicated activate (`_deploymentId` set). deploy_component drives activation itself. + * Peer activate phase (internal — NOT a public operation). Runs on a peer when the origin fans out + * deploy_component tagged `_phase: 'activate'`: atomically swap the already-staged build (by deployment + * id) into the live path, persist root config for a package deploy, and restart if the origin asked + * for an immediate restart. No recorder, no re-replication. */ -async function activateComponent(req) { - if (req.project) req.project = path.parse(req.project).name; - const validation = validator.activateComponentValidator(req); - if (validation) throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); - - const isReplicatedExecution = typeof req._deploymentId === 'string'; - const stagingId = req._deploymentId ?? req.deployment_id; - if (!stagingId) { - throw handleHDBError( - new Error(), - `'deployment_id' is required to activate a staged component`, - HTTP_STATUS_CODES.BAD_REQUEST - ); - } +async function deployPhaseActivate(req) { if (req.package) assertNotProtectedCoreComponent(req.project, req.force); const credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); + const application = new Application({ + name: req.project, + packageIdentifier: req.package, + stagingId: req._deploymentId, + }); + await activateApplication(application); + if (req.package) await writeComponentRootConfig(req, credentialReferences); + // The origin sets restart=true on the sub-op only for an immediate restart; rolling restarts are + // driven separately by the origin via a replicated restart_service job. + if (req.restart === true) manageThreads.restartWorkers('http'); + return { message: `Activated component: ${req.project}`, project: req.project, activated: true }; +} - const emitter = isReplicatedExecution ? null : (req.progress ?? new ProgressEmitter()); - const emit = (event, data) => emitter?.emit(event, data); +/** + * Activate a previously-staged deployment cluster-wide — the second half of a stage-then-activate + * flow, reached as `deploy_component({ deployment_id })` with no fresh payload. Swaps the staged build + * into the live path on the origin, replicates the activate phase to peers (each activates its own + * staged copy of the same deployment id), restarts, and marks the deployment row success. + */ +async function deployComponentActivateExisting(req, credentialReferences) { + const stagingId = req.deployment_id; + if (req.package) assertNotProtectedCoreComponent(req.project, req.force); + const emitter = req.progress ?? new ProgressEmitter(); + if (!req.progress) req.progress = emitter; + const emit = (event, data) => emitter.emit(event, data); const rollingRestart = req.restart === 'rolling'; - // Reconstruct the Application against the staged build. Only name + stagingId are needed to locate - // and swap it — the staged directory already holds the fully-installed incoming version. const application = new Application({ name: req.project, packageIdentifier: req.package, stagingId }); emit('phase', { phase: 'activate', status: 'start' }); await activateApplication(application); emit('phase', { phase: 'activate', status: 'done' }); - - // Persist root config now that the component is live (package deploys, on every node). + // Persist root config now that the component is live (package deploys). if (req.package) await writeComponentRootConfig(req, credentialReferences); - const response = { message: `Activated component: ${req.project}`, project: req.project, activated: true }; + // Replicate the activate phase to peers (each activates its own staged copy of this deployment id). + req._deploymentId = stagingId; + delete req.progress; + const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.DEPLOY_COMPONENT, { + phase: 'activate', + restart: rollingRestart ? false : req.restart, + deploymentId: stagingId, + }); + const rep = await server.replication.replicateOperation(activateOp, {}); - // Replicate the activation to peers (direct invocation only; a peer replaying an activate must not - // re-fan it out). - if (!isReplicatedExecution) { - delete req.progress; - const restartForPeers = rollingRestart ? false : req.restart; - const activateOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, { - restart: restartForPeers, - deploymentId: stagingId, - }); - const rep = await server.replication.replicateOperation(activateOp, {}); - if (rep?.replicated) response.replicated = rep.replicated; - } + const response = { + message: `Activated component: ${req.project}`, + project: req.project, + activated: true, + deployment_id: stagingId, + }; + if (rep?.replicated) response.replicated = rep.replicated; - // Restart on this node. A peer replaying an immediate-restart activate restarts locally; the - // rolling path is driven only by the direct invoker via a replicated restart_service job. if (req.restart === true) { emit('phase', { phase: 'restart', status: 'start' }); manageThreads.restartWorkers('http'); emit('phase', { phase: 'restart', status: 'done' }); response.message = `Activated component: ${req.project}, restarting Harper`; - } else if (rollingRestart && !isReplicatedExecution) { + } else if (rollingRestart) { const serverUtilities = require('../server/serverHelpers/serverUtilities.ts'); emit('phase', { phase: 'restart', status: 'start' }); const jobResponse = await serverUtilities.executeJob({ @@ -922,6 +880,12 @@ async function activateComponent(req) { response.restartJobId = jobResponse.job_id; response.message = `Activated component: ${req.project}, restarting Harper`; } + + // Best-effort: flip the staged deployment row (left 'staged' by the stage-and-stop) to success now + // that it is live. Observability only — a tracking-write failure must not fail the activate. + await markDeploymentTerminal(stagingId, 'success').catch((err) => + log.warn('Failed to mark staged deployment as activated', err) + ); return response; } @@ -1156,11 +1120,14 @@ async function loadValidateComponent({ dirPath, emit }) { if (lastError) throw lastError; } -// Build a replicated sub-operation body (stage_component / activate_component) from the deploy -// request. Carries only what a peer needs: project, the deployment id (correlation + payload lookup + -// staging id), the build/config inputs, and credential REFERENCES (tokens are already stripped). -function buildReplicatedSubOp(req, operation, { includePayload = false, restart, deploymentId } = {}) { +// Build a replicated sub-operation body for the peer fan-out. For the two-phase peer phases this is +// `deploy_component` tagged with an internal `_phase` marker (`stage`/`activate`) — the wire format — +// so no separate public op is exposed; revert uses operation `revert_component`. Carries only what a +// peer needs: project, the deployment id (correlation + payload lookup + staging id), the internal +// `_phase`, the build/config inputs, and credential REFERENCES (tokens are already stripped). +function buildReplicatedSubOp(req, operation, { includePayload = false, restart, deploymentId, phase } = {}) { const op = { operation, project: req.project, _deploymentId: deploymentId ?? req._deploymentId }; + if (phase) op._phase = phase; if (req.package) op.package = req.package; if (req.install_command != null) op.install_command = req.install_command; if (req.install_timeout != null) op.install_timeout = req.install_timeout; @@ -1680,8 +1647,6 @@ exports.addComponent = addComponent; exports.dropCustomFunctionProject = dropCustomFunctionProject; exports.packageComponent = packageComponent; exports.deployComponent = deployComponent; -exports.stageComponent = stageComponent; -exports.activateComponent = activateComponent; exports.revertComponent = revertComponent; exports.selectRevertTargets = selectRevertTargets; // exported for unit testing the revert_on_failure node-targeting exports.getComponents = getComponents; diff --git a/components/operationsValidation.js b/components/operationsValidation.js index a726f41b8d..8964caba1d 100644 --- a/components/operationsValidation.js +++ b/components/operationsValidation.js @@ -25,8 +25,6 @@ module.exports = { dropCustomFunctionProjectValidator, packageComponentValidator, deployComponentValidator, - stageComponentValidator, - activateComponentValidator, revertComponentValidator, setComponentFileValidator, getComponentFileValidator, @@ -489,8 +487,19 @@ function deployComponentValidator(req) { deployment_timeout: Joi.number().min(0).optional(), force: Joi.boolean().optional(), ignore_replication_errors: Joi.boolean().optional(), + // Stop after the incoming version is staged and verified cluster-wide, without going live. Returns + // the staged deployment_id; a later deploy_component with that deployment_id activates it. Defaults + // to true (full stage + activate). + activate: Joi.boolean().optional(), + // Activate a previously-staged deployment (from an `activate: false` stage) cluster-wide. Same safe + // charset as `project` because it becomes a staging-dir path segment (`.deploy-staging//`) + // — a `../` value would otherwise resolve the staging source outside `.deploy-staging`. + deployment_id: Joi.string().pattern(PROJECT_FILE_NAME_REGEX).optional().messages({ + 'string.pattern.base': `'deployment_id' must only contain letters, numbers, dashes, and underscores`, + }), // If the activate phase fails on some nodes (leaving the cluster split across versions), swap the - // whole cluster back to the retained previous version before reporting the failure. Off by default. + // nodes that did activate back to the retained previous version before reporting the failure. Off + // by default. revert_on_failure: Joi.boolean().optional(), // Opt out of the two-phase (stage-then-activate) deploy and use the legacy one-shot path instead. // Defaults to two-phase. @@ -506,74 +515,6 @@ function deployComponentValidator(req) { return validator.validateBySchema(req, deployProjSchema); } -/** - * Validate stage_component requests — phase 1 of a two-phase deploy. Accepts the same build-time - * inputs as deploy_component (package/payload, install options, credentials) but no go-live controls - * (`restart`), since staging never restarts. `restart` is intentionally absent; a stray one is - * ignored (operations validate with allowUnknown). - * @param req - * @returns {*} - */ -function stageComponentValidator(req) { - const stageSchema = Joi.object({ - project: Joi.string() - .pattern(PROJECT_FILE_NAME_REGEX) - .required() - .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), - package: Joi.string().optional(), - install_command: Joi.string().optional(), - install_timeout: Joi.number().optional(), - install_allow_scripts: Joi.boolean().optional(), - deployment_timeout: Joi.number().min(0).optional(), - force: Joi.boolean().optional(), - // urlPath is not applied at stage time (config is written at activate), but it is accepted and - // carried through so a single request body can flow stage → activate unchanged. - urlPath: URL_PATH_SCHEMA, - credentials: CREDENTIALS_ARRAY_SCHEMA, - registryAuth: FORBIDDEN_REGISTRY_AUTH, - }).with('urlPath', 'package'); - - return validator.validateBySchema(req, stageSchema); -} - -/** - * Validate activate_component requests — phase 2 of a two-phase deploy. Swaps an already-staged - * build (identified by `deployment_id`) into the live path and optionally restarts. Also carries the - * config-persistence inputs (package/install/credentials/urlPath) so a `package` deploy's root config - * is written at go-live rather than before the bits are in place. - * @param req - * @returns {*} - */ -function activateComponentValidator(req) { - const activateSchema = Joi.object({ - project: Joi.string() - .pattern(PROJECT_FILE_NAME_REGEX) - .required() - .messages({ 'string.pattern.base': HDB_ERROR_MSGS.BAD_PROJECT_NAME }), - // Identifies which staged build to activate. Required for a standalone activate; the - // deploy_component orchestrator supplies it as the deployment id it staged under. Constrained to - // the same safe charset as `project` because it becomes a path segment of the staging directory - // (`.deploy-staging//`) — without this a `../` value would resolve the staging - // source outside `.deploy-staging`. Defense-in-depth (the op is super_user-only). - deployment_id: Joi.string().pattern(PROJECT_FILE_NAME_REGEX).optional().messages({ - 'string.pattern.base': `'deployment_id' must only contain letters, numbers, dashes, and underscores`, - }), - package: Joi.string().optional(), - install_command: Joi.string().optional(), - install_timeout: Joi.number().optional(), - install_allow_scripts: Joi.boolean().optional(), - deployment_timeout: Joi.number().min(0).optional(), - restart: Joi.alternatives().try(Joi.boolean(), Joi.string().valid('rolling')).optional(), - force: Joi.boolean().optional(), - ignore_replication_errors: Joi.boolean().optional(), - urlPath: URL_PATH_SCHEMA, - credentials: CREDENTIALS_ARRAY_SCHEMA, - registryAuth: FORBIDDEN_REGISTRY_AUTH, - }).with('urlPath', 'package'); - - return validator.validateBySchema(req, activateSchema); -} - /** * Validate revert_component requests — swap a component's live version back to its retained previous * version. No build inputs (nothing is fetched or installed); just the project, an optional restart, diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index 9acac09fb1..fa71d7c140 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -33,8 +33,6 @@ const { ProgressEmitter, createSSEResponseStream } = require('./progressEmitter. // the client disconnects (the emitter's abort signal), so subscribers see new lines live. const SSE_PROGRESS_OPERATIONS = new Set([ terms.OPERATIONS_ENUM.DEPLOY_COMPONENT, - terms.OPERATIONS_ENUM.STAGE_COMPONENT, - terms.OPERATIONS_ENUM.ACTIVATE_COMPONENT, terms.OPERATIONS_ENUM.REVERT_COMPONENT, terms.OPERATIONS_ENUM.GET_DEPLOYMENT, terms.OPERATIONS_ENUM.READ_LOG, diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index b0c6557949..a4b631094a 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -534,14 +534,6 @@ function initializeOperationFunctionMap(): Map { + it('deploy_component({activate:false}) stages into the hidden dir without going live, and returns a deployment_id', async () => { const name = freshName(); - const res = await operations.stageComponent({ project: name, payload: await makeComponentPayload('op-staged') }); + const res = await operations.deployComponent({ + project: name, + payload: await makeComponentPayload('op-staged'), + activate: false, + }); assert.strictEqual(res.staged, true); assert.strictEqual(res.project, name); @@ -89,16 +93,18 @@ describe('deploy operations: stage_component / activate_component / deploy_compo assert.strictEqual(existsSync(path.join(COMPONENTS_ROOT, name)), false, 'staging did not touch the live path'); }); - it('activate_component takes a prior stage live', async () => { + it('deploy_component({deployment_id}) takes a prior stage live', async () => { const name = freshName(); - const staged = await operations.stageComponent({ + const staged = await operations.deployComponent({ project: name, payload: await makeComponentPayload('op-activated'), + activate: false, }); - const res = await operations.activateComponent({ project: name, deployment_id: staged.deployment_id }); + const res = await operations.deployComponent({ project: name, deployment_id: staged.deployment_id }); assert.strictEqual(res.activated, true); assert.strictEqual(res.project, name); + assert.strictEqual(res.deployment_id, staged.deployment_id); const liveDir = path.join(COMPONENTS_ROOT, name); assert.ok(existsSync(liveDir), 'live component dir now exists'); assert.match(await readIndex(liveDir), /op-activated/); @@ -106,11 +112,6 @@ describe('deploy operations: stage_component / activate_component / deploy_compo assert.doesNotMatch(res.message, /restart/i); }); - it('activate_component rejects when no deployment_id is supplied', async () => { - const name = freshName(); - await assert.rejects(() => operations.activateComponent({ project: name }), /deployment_id.*required/i); - }); - it('deploy_component (two-phase default) stages then activates end-to-end', async () => { const name = freshName(); const res = await operations.deployComponent({ project: name, payload: await makeComponentPayload('op-deployed') }); diff --git a/unitTests/components/deployPhaseValidators.test.js b/unitTests/components/deployPhaseValidators.test.js index daa85573e6..779cbfa32a 100644 --- a/unitTests/components/deployPhaseValidators.test.js +++ b/unitTests/components/deployPhaseValidators.test.js @@ -11,78 +11,6 @@ const validator = require('#js/components/operationsValidation'); const ok = (result) => assert.strictEqual(result, undefined, `expected valid, got: ${result && result.message}`); const rejected = (result) => assert.ok(result, 'expected a validation error'); -describe('stageComponentValidator', () => { - it('accepts a project-only request', () => { - ok(validator.stageComponentValidator({ project: 'my_app' })); - }); - - it('accepts a package deploy with install options', () => { - ok( - validator.stageComponentValidator({ - project: 'my_app', - package: 'npm:@org/thing', - install_command: 'npm ci', - install_timeout: 60000, - install_allow_scripts: false, - deployment_timeout: 120000, - }) - ); - }); - - it('requires a project', () => { - rejected(validator.stageComponentValidator({ package: 'npm:@org/thing' })); - }); - - it('rejects an invalid project name', () => { - rejected(validator.stageComponentValidator({ project: 'bad/name' })); - }); - - it('rejects a urlPath containing ".."', () => { - rejected(validator.stageComponentValidator({ project: 'my_app', package: 'npm:x', urlPath: '/a/../b' })); - }); -}); - -describe('activateComponentValidator', () => { - it('accepts a project + deployment_id', () => { - ok(validator.activateComponentValidator({ project: 'my_app', deployment_id: 'abc-123' })); - }); - - it('accepts a rolling restart', () => { - ok(validator.activateComponentValidator({ project: 'my_app', deployment_id: 'abc-123', restart: 'rolling' })); - }); - - it('accepts a boolean restart and ignore_replication_errors', () => { - ok( - validator.activateComponentValidator({ - project: 'my_app', - deployment_id: 'abc-123', - restart: true, - ignore_replication_errors: true, - }) - ); - }); - - it('requires a project', () => { - rejected(validator.activateComponentValidator({ deployment_id: 'abc-123' })); - }); - - it('rejects an invalid restart value', () => { - rejected(validator.activateComponentValidator({ project: 'my_app', restart: 'sideways' })); - }); - - it('rejects a path-traversal deployment_id (it becomes a staging-dir path segment)', () => { - for (const bad of ['../evil', 'a/b', 'dep/../..', '.', '..']) { - rejected(validator.activateComponentValidator({ project: 'my_app', deployment_id: bad })); - } - }); - - it('accepts a normal UUID-shaped deployment_id', () => { - ok( - validator.activateComponentValidator({ project: 'my_app', deployment_id: '41faded8-6cf5-4a2a-95f8-863e7ea498fa' }) - ); - }); -}); - describe('revertComponentValidator', () => { it('accepts a project-only revert', () => { ok(validator.revertComponentValidator({ project: 'my_app' })); @@ -108,16 +36,29 @@ describe('revertComponentValidator', () => { }); }); -describe('deployComponentValidator two_phase + revert_on_failure flags', () => { +describe('deployComponentValidator two-phase props (activate / deployment_id / flags)', () => { + it('accepts activate: false (stage-and-stop)', () => { + ok(validator.deployComponentValidator({ project: 'my_app', activate: false })); + }); + + it('accepts a deployment_id (activate an existing stage)', () => { + ok( + validator.deployComponentValidator({ project: 'my_app', deployment_id: '41faded8-6cf5-4a2a-95f8-863e7ea498fa' }) + ); + }); + + it('rejects a path-traversal deployment_id (it becomes a staging-dir path segment)', () => { + for (const bad of ['../evil', 'a/b', 'dep/../..', '.', '..']) { + rejected(validator.deployComponentValidator({ project: 'my_app', deployment_id: bad })); + } + }); + it('accepts revert_on_failure: true', () => { ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', revert_on_failure: true })); }); - it('accepts two_phase: false (legacy opt-out)', () => { + it('accepts two_phase: false (legacy opt-out) and true', () => { ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: false })); - }); - - it('accepts two_phase: true', () => { ok(validator.deployComponentValidator({ project: 'my_app', package: 'npm:x', two_phase: true })); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index 1b47d74f51..16e532aaa3 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -290,15 +290,15 @@ export const OPERATIONS_ENUM = { PACKAGE_CUSTOM_FUNCTION_PROJECT: 'package_custom_function_project', DEPLOY_CUSTOM_FUNCTION_PROJECT: 'deploy_custom_function_project', PACKAGE_COMPONENT: 'package_component', + // deploy_component runs a two-phase deploy internally (stage the incoming version into a hidden + // dir cluster-wide, gate on every node, then atomically swap it live). The two phases are fanned + // out to peers as deploy_component tagged with an internal `_phase` marker rather than separate + // public operations. Public knobs: `activate: false` (stage-and-stop, returns a staged + // deployment_id) and `deployment_id` (activate a previously-staged deployment). See + // components/Application.ts (stageApplication/activateApplication). DEPLOY_COMPONENT: 'deploy_component', - // Two-phase deploy sub-operations. stage_component builds the incoming version into a hidden - // staging directory cluster-wide (no go-live); activate_component atomically swaps the staged - // copy into the live path and restarts. deploy_component orchestrates the two so existing callers - // are unaffected. See components/Application.ts (stageApplication/activateApplication). - STAGE_COMPONENT: 'stage_component', - ACTIVATE_COMPONENT: 'activate_component', // Swap a component's live version back to its retained previous version, cluster-wide. Backs - // customer-driven rollback (activate → test → revert) and swap-back on a partially-failed activate. + // customer-driven rollback (deploy → test → revert) and swap-back on a partially-failed activate. // See components/Application.ts (revertApplication). REVERT_COMPONENT: 'revert_component', READ_TRANSACTION_LOG: 'read_transaction_log', diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index c48a4a6c3c..b24eb95786 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -286,8 +286,6 @@ requiredPermissions.set(functionsOperations.addComponent.name, new (permission a requiredPermissions.set(functionsOperations.dropCustomFunctionProject.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.packageComponent.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.deployComponent.name, new (permission as any)(true, [])); -requiredPermissions.set(functionsOperations.stageComponent.name, new (permission as any)(true, [])); -requiredPermissions.set(functionsOperations.activateComponent.name, new (permission as any)(true, [])); requiredPermissions.set(functionsOperations.revertComponent.name, new (permission as any)(true, [])); requiredPermissions.set( deploymentOperations.handleListDeployments.name, From 4197f1c285d9dcaeca52b9da4577097e79345883 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 10:20:36 -0400 Subject: [PATCH 13/22] feat(deploy): bound staged-build retention per component (evict-on-stage) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With deploy_component({ activate: false }) leaving staged builds around for a later deploy_component({ deployment_id }), those not-yet-activated builds could accumulate without limit (a full deploy consumes its build on activate, so only stage-and-stops pile up). stageApplication now evicts the oldest such builds for a component beyond deployment_stagingRetention_maxCount (default 5) after each successful stage — always keeping the just-staged one plus the newest N-1 by mtime. Eviction is best-effort but awaited so the count is settled when the stage returns. Per the harper#1849 decision: retention is count-only and fully automatic (no new op); hdb_deployment rows stay as the audit trail (payload blobs already self-reclaim by size). Consequence: activating a deployment_id that has aged out of the window fails "no staged build found", as expected. DESIGN.md + a retention test added. Co-Authored-By: Claude Opus 4.8 --- DESIGN.md | 14 +++++ components/Application.ts | 71 ++++++++++++++++++++++ unitTests/components/deployStaging.test.js | 22 +++++++ utility/hdbTerms.ts | 3 + 4 files changed, 110 insertions(+) diff --git a/DESIGN.md b/DESIGN.md index 75a2921ad6..6523650681 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -405,6 +405,20 @@ some nodes live and some not, so the cluster reconverges on one version. The pre per-node (each node retains its own outgoing version during its own activate), so a replicated revert has a local rollback source on every node. +**Staged-build retention.** A full deploy consumes its staged build immediately (activate renames it +live), so the only builds that accumulate are `activate: false` stage-and-stops that are never +activated — each leaves `.deploy-staging//` in place so a later +`deploy_component({deployment_id})` can activate it. `stageApplication` bounds this: after a successful +stage it evicts the oldest not-yet-activated staged builds for that component beyond +`deployment_stagingRetention_maxCount` (default 5, `pruneStagedBuilds`), always keeping the just-staged +one and the newest N−1 by mtime. Eviction is best-effort (`allSettled`, trace-logged) but awaited so +the count is settled when the stage returns. Retention is deliberately count-only and automatic: +per the harper#1849 discussion, `hdb_deployment` rows stay as the audit trail (payload blobs already +self-reclaim by size, `deployment_payloadRetention_maxSize`), and no `delete_deployment` op was added — +eviction-on-stage keeps the surface at zero new operations. Consequence: activating a `deployment_id` +that has already aged out of the window fails with "no staged build found" — expected once more than +`maxCount` newer stages have landed for that component. + ## Scheduler: cluster-once execution without a consensus primitive (`resources/scheduler/`) The built-in `scheduler` plugin (#951) runs config-declared jobs "exactly once per cluster." The diff --git a/components/Application.ts b/components/Application.ts index 8721ab91ff..2ddaabfa40 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -438,6 +438,73 @@ function previousDirPathFor(liveDirPath: string): string { return join(dirname(liveDirPath), DEPLOY_PREVIOUS_DIR, basename(liveDirPath)); } +// Max not-yet-activated staged builds kept per component before the oldest are evicted on the next +// stage. A full deploy consumes its staged build immediately (activate renames it live), so this only +// bounds `activate: false` stage-and-stops that are never activated. Configurable via +// deployment_stagingRetention_maxCount. +export const DEFAULT_STAGING_RETENTION_MAX_COUNT = 5; + +function getStagingRetentionMaxCount(): number { + const configured = getConfigValue(CONFIG_PARAMS.DEPLOYMENT_STAGINGRETENTION_MAXCOUNT); + // Only a number or numeric string is a valid count; reject everything else (unset, boolean, array, + // blank) and fall back to the default — mirroring getPayloadRetentionMaxSize's defensive coercion. + if (typeof configured !== 'number' && typeof configured !== 'string') return DEFAULT_STAGING_RETENTION_MAX_COUNT; + if (typeof configured === 'string' && configured.trim() === '') return DEFAULT_STAGING_RETENTION_MAX_COUNT; + const parsed = Number(configured); + return Number.isFinite(parsed) && parsed >= 1 ? Math.floor(parsed) : DEFAULT_STAGING_RETENTION_MAX_COUNT; +} + +/** + * Prune the oldest not-yet-activated staged builds for a component, keeping at most `maxCount` (newest + * by mtime) and ALWAYS retaining the just-built one (`keepStagingId`). Staged builds live at + * `.deploy-staging//`, and each stagingId parent holds exactly one component's build, + * so an evicted build's whole parent directory is removed. Entirely best-effort: any error (a racing + * concurrent stage, a busy dir) is logged at trace and never fails the stage that triggered it. + */ +async function pruneStagedBuilds(componentName: string, keepStagingId: string, maxCount: number): Promise { + try { + if (!(maxCount >= 1)) return; + const componentsRoot = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); + if (!componentsRoot) return; + const stagingRoot = join(componentsRoot, DEPLOY_STAGING_DIR); + let parents: import('node:fs').Dirent[]; + try { + parents = await readdir(stagingRoot, { withFileTypes: true }); + } catch (err) { + if ((err as any).code === 'ENOENT') return; // nothing staged yet + throw err; + } + // Collect this component's staged builds: // that still exist. + const builds: Array<{ stagingId: string; parentPath: string; mtime: number }> = []; + for (const parent of parents) { + if (!parent.isDirectory()) continue; + const parentPath = join(stagingRoot, parent.name); + try { + const st = await stat(join(parentPath, componentName)); + builds.push({ stagingId: parent.name, parentPath, mtime: st.mtimeMs }); + } catch (err) { + if ((err as any).code !== 'ENOENT') throw err; // parent holds a different component; skip + } + } + // Always keep the build we just made, plus the newest (maxCount - 1) of the OTHERS by mtime; evict + // the rest. Computing it as "keep keepStagingId + top-(N-1) others" (rather than "evict everything + // past the top N") keeps the count exact even when mtimes tie and the just-built one would + // otherwise sort into the eviction window. Await the evictions (best-effort via allSettled) so the + // retention count is settled by the time the stage returns. + const others = builds.filter((build) => build.stagingId !== keepStagingId).sort((a, b) => b.mtime - a.mtime); + const evictions = others + .slice(Math.max(0, maxCount - 1)) + .map((build) => + rm(build.parentPath, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }).catch((err) => + logger.trace?.(`Deferred prune of staged ${componentName} build ${build.stagingId}: ${err.message}`) + ) + ); + await Promise.allSettled(evictions); + } catch (err) { + logger.trace?.(`Staged-build prune for ${componentName} skipped: ${(err as Error).message}`); + } +} + /** * Atomically move `targetDirPath` aside into a hidden, per-component staging directory if it * exists, returning the aside staging directory (for best-effort cleanup) or null when there was @@ -1208,6 +1275,10 @@ export async function stageApplication(application: Application): Promise { + // Simulate repeated `activate: false` stage-and-stops of the same component (distinct stagingIds, + // never activated). Each stage should prune older ones down to the retention count. + const name = `stage_test_${process.pid}_${counter++}`; + const stagingRoot = path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR); + const stagingIds = []; + for (let i = 0; i < 7; i++) { + const app = new Application({ name, payload: await makeComponentPayload(`v${i}`) }); + stagingIds.push(app.stagingId); + await stageApplication(app); + } + + const remaining = stagingIds.filter((id) => existsSync(path.join(stagingRoot, id, name))); + // Contract: at most `maxCount` staged builds are retained per component, and the just-staged one is + // always kept. (Which older builds are evicted is ordered by mtime — reliable in real use where + // stages are time-separated, but this tight loop can create ties, so it isn't asserted here.) + assert.strictEqual(remaining.length, 5, `expected 5 staged builds retained, got ${remaining.length}`); + assert.ok(existsSync(path.join(stagingRoot, stagingIds[6], name)), 'the most recent stage is always retained'); + + await fs.rm(stagingRoot, { recursive: true, force: true }); + }); + it('only one previous is retained across three deploys (older previous evicted)', async () => { const name = `stage_test_${process.pid}_${counter++}`; await deployVersion(name, 'v1'); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index 16e532aaa3..e1e2dd81f9 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -555,6 +555,9 @@ export const CONFIG_PARAMS = { OPERATIONSAPI_NETWORK_MAXREQUESTBODYSIZE: 'operationsApi_network_maxRequestBodySize', OPERATIONSAPI_COMPONENTFILE_MAXSIZE: 'operationsApi_componentFile_maxSize', DEPLOYMENT_PAYLOADRETENTION_MAXSIZE: 'deployment_payloadRetention_maxSize', + // Max not-yet-activated staged builds kept per component (`activate: false` stage-and-stops). When a + // new stage lands, the oldest beyond this count are evicted. See components/Application.ts. + DEPLOYMENT_STAGINGRETENTION_MAXCOUNT: 'deployment_stagingRetention_maxCount', OPERATIONSAPI_TLS: 'operationsApi_tls', OPERATIONSAPI_TLS_CERTIFICATE: 'operationsApi_tls_certificate', OPERATIONSAPI_TLS_PRIVATEKEY: 'operationsApi_tls_privateKey', From 9f645bc29a490e817233505042c21e53b55d691a Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 10:25:12 -0400 Subject: [PATCH 14/22] fix(cli): `harper activate` must carry a deployment_id (fail fast, not full deploy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After folding activate into deploy_component, `harper activate` mapped to deploy_component with no marker. deployComponent only routes to activate-existing when deployment_id is truthy — otherwise it falls through to a full two-phase deploy, and packageCwdForUpload (skips only for package || deployment_id) would tar + upload the CWD. So `harper activate project=x` with a missing/mistyped deployment_id silently ran a brand-new deploy from local files. The `activate` verb now carries a CLI-internal `_verb` marker, and verbRequirementError (pure, exported) rejects it when deployment_id is absent — checked BEFORE packaging, with a clear message, so it fails fast instead of deploying. The marker is stripped before the request body. Adds CLI verb tests covering stage/activate mapping and the missing-deployment_id guard. (Review catch.) Co-Authored-By: Claude Opus 4.8 --- bin/cliOperations.ts | 25 +++++++++++++++++-- unitTests/bin/cliOperations.test.js | 37 +++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/bin/cliOperations.ts b/bin/cliOperations.ts index 51e022f0d9..c353bd9f57 100644 --- a/bin/cliOperations.ts +++ b/bin/cliOperations.ts @@ -31,9 +31,22 @@ const OP_ALIASES = { // takes that staged deployment live (no upload). const OP_VERB_PROPS: Record> = { stage: { operation: 'deploy_component', activate: false }, - activate: { operation: 'deploy_component' }, + // `_verb` is a CLI-internal marker (stripped before the request) so verbRequirementError can enforce + // that `harper activate` carries a deployment_id — without it, deploy_component's generic + // "no deployment_id → full deploy" fallback would silently build a brand-new deploy from the CWD. + activate: { operation: 'deploy_component', _verb: 'activate' }, }; +// Guard CLI-verb requirements that the operation itself can't enforce (the op has no notion of which +// verb invoked it). Returns an error message, or null when the request is fine. Pure + exported so it +// is unit-testable without the network/process-exit machinery in cliOperations. +function verbRequirementError(req: any): string | null { + if (req._verb === 'activate' && !req.deployment_id) { + return '`harper activate` requires a deployment_id from a prior `harper stage` — usage: harper activate project= deployment_id='; + } + return null; +} + // Shown for any local-instance connection failure (missing pid, missing/stale domain // socket, or a refused/ENOENT connect against it) — they're all the same user-facing // scenario: Harper isn't running. Remote-target failures keep the detailed error instead, @@ -164,7 +177,7 @@ function operationFields(req: any): any { return fields; } -export { cliOperations, buildRequest }; +export { cliOperations, buildRequest, verbRequirementError }; // Package the current working directory into a multipart tarball upload for deploy_component. Covers // `harper deploy` and `harper stage` (deploy_component with activate:false) — both upload the incoming @@ -303,6 +316,14 @@ async function cliOperations(req: any, skipResponseLog = false) { process.exit(1); } } + // Enforce CLI-verb requirements (e.g. `harper activate` needs a deployment_id) BEFORE packaging so a + // mistake fails fast instead of building + uploading a fresh deploy from the CWD. + const verbError = verbRequirementError(req); + if (verbError) { + console.error(verbError); + process.exit(1); + } + delete req._verb; // CLI-internal marker; never send it in the request body await PREPARE_OPERATION[req.operation]?.(req); try { let options = target ?? { diff --git a/unitTests/bin/cliOperations.test.js b/unitTests/bin/cliOperations.test.js index 69a5ae67ef..da0c76c92f 100644 --- a/unitTests/bin/cliOperations.test.js +++ b/unitTests/bin/cliOperations.test.js @@ -766,3 +766,40 @@ describe('cliOperations', () => { }); }); }); + +describe('deploy CLI verbs (stage / activate fold into deploy_component)', () => { + const { buildRequest, verbRequirementError } = cliOperationsModule; + let savedArgv; + beforeEach(() => { + savedArgv = process.argv; + }); + afterEach(() => { + process.argv = savedArgv; + }); + + it('`stage` maps to deploy_component with activate:false', () => { + process.argv = ['node', 'harper', 'stage', 'project=my_app']; + const req = buildRequest(); + assert.strictEqual(req.operation, 'deploy_component'); + assert.strictEqual(req.activate, false); + }); + + it('`activate` with a deployment_id maps to deploy_component and passes the verb guard', () => { + process.argv = ['node', 'harper', 'activate', 'project=my_app', 'deployment_id=abc-123']; + const req = buildRequest(); + assert.strictEqual(req.operation, 'deploy_component'); + assert.strictEqual(req.deployment_id, 'abc-123'); + assert.strictEqual(verbRequirementError(req), null); + }); + + it('`activate` WITHOUT a deployment_id is rejected (would otherwise become a full deploy from the CWD)', () => { + process.argv = ['node', 'harper', 'activate', 'project=my_app']; + const req = buildRequest(); + assert.match(verbRequirementError(req), /deployment_id/); + }); + + it('verbRequirementError ignores non-activate deploys', () => { + assert.strictEqual(verbRequirementError({ operation: 'deploy_component' }), null); + assert.strictEqual(verbRequirementError({ operation: 'deploy_component', activate: false }), null); + }); +}); From b0e33e3af601efcf6fe46c29b3570e3ae379458b Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 19:22:04 -0400 Subject: [PATCH 15/22] test(deploy): reset process-wide restart buffer after two-phase op tests deployPhaseOperations deploys genuinely-new components, which post-merge trips deployComponent's requestRestart() (harper#674 new-component scoping) and sets the process-wide restart-needed shared buffer. That leaked into requestRestart.test.js's "pristine buffer" assertion, failing it on suite ordering alone. Restore the buffer in the existing after() hook. Co-Authored-By: Claude Opus 4.8 --- unitTests/components/deployPhaseOperations.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index 8979ca1bb4..5d26f95b2e 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -20,6 +20,7 @@ testUtils.preTestPrep(); const operations = require('#src/components/operations'); const { DEPLOY_STAGING_DIR } = require('#src/components/Application'); +const { resetRestartNeeded } = require('#src/components/requestRestart'); const { getConfigPath } = require('#src/config/configUtils'); const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); @@ -74,6 +75,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo await fs.rm(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR), { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, '.deploy-previous'), { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, '.deploy-aside'), { recursive: true, force: true }); + // Deploying a genuinely-new component flips the process-wide restart-needed buffer on + // (harper#674, via deployComponent's requestRestart() scoping). That buffer is shared across + // the whole mocha process, so restore it or a later test asserting a pristine buffer + // (e.g. requestRestart.test.js) fails on ordering alone. + resetRestartNeeded(); }); it('deploy_component({activate:false}) stages into the hidden dir without going live, and returns a deployment_id', async () => { From c907eb2a7432e08df26d94b3b213217a139c4192 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 19:32:05 -0400 Subject: [PATCH 16/22] fix(deploy): two-phase deploy marks restart-required for new components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge with main brought harper#674/#1806: a deploy_component with restart:false must set get_status restartRequired for a genuinely-new (never-loaded) component, scoped so a redeploy of an already-active component stays quiet. Main implemented this only in the one-shot deploy path (requestRestart() gated on Application.isNewComponent). Two-phase deploy — the default whenever the system db is replicated — never carried that behavior, so inactive-component-404.test.ts failed: a fresh deploy left restartRequired false and the actionable 404 never surfaced. Two-phase also can't set isNewComponent the way the one-shot path does: stageApplication extracts into a fresh staging dir, so extractApplication never sees the live directory and isNewComponent stayed default-true for everything (which would wrongly mark a restart on a redeploy). Fix both: - activateApplication now sets isNewComponent from whether the live dir existed BEFORE the swap (the two-phase equivalent of extract's in-place check) — true for a first deploy, false for a redeploy. - Extract markRestartRequiredForNewComponent() and call it on every no-restart path: one-shot (unchanged behavior), two-phase origin, activate-existing, and the per-node peer activate — so a new component deployed cluster-wide with restart:false reports restartRequired on every node, matching the one-shot peer behavior. Unit coverage: deployStaging asserts activate sets isNewComponent true for a first-ever activate and false when replacing an existing live version. Co-Authored-By: Claude Opus 4.8 --- components/Application.ts | 13 ++++++++ components/operations.js | 39 +++++++++++++++++++--- unitTests/components/deployStaging.test.js | 7 ++++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/components/Application.ts b/components/Application.ts index 9730e80f21..9c6b3cf285 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -1383,6 +1383,19 @@ export async function activateApplication(application: Application): Promise), then rename // the staged copy into place. Both live under the components root, so the rename is same-fs and // atomic — there is no interval where `dirPath` is a partially populated directory. retainAsPrevious diff --git a/components/operations.js b/components/operations.js index f679d404e0..86d52e9f7c 100644 --- a/components/operations.js +++ b/components/operations.js @@ -430,6 +430,24 @@ async function deployComponent(req) { return deployComponentTwoPhase(req, credentialReferences); } +/** + * A genuinely-new (never-loaded) component deployed without an immediate restart can't serve its + * routes until Harper restarts, so mark a restart as needed (harper#674). This is the setter only; it + * does not itself restart — it makes get_status report restartRequired:true and lets the REST + * route-miss path surface the actionable "needs a restart" 404. Scoped to new components (harper#1806): + * an existing, already-loaded component's own file watcher independently requests a restart if a + * redeploy actually needs one, so a redeploy stays quiet. Runs per node — each node checks its own + * isNewComponent, since directory state (new vs. redeploy) can differ across the cluster. The one-shot + * path has extractApplication set isNewComponent in place; the two-phase path has activateApplication + * set it at swap time (staging is always fresh, so extract never sees the live dir). + */ +function markRestartRequiredForNewComponent(application) { + if (application.isNewComponent) { + const { requestRestart } = require('./requestRestart.ts'); + requestRestart(); + } +} + /** * Legacy one-shot deploy: extract + `npm install` in place on the origin, then replicate the whole * deploy_component operation to peers, which each do the same. Preserved verbatim (behavior-for- @@ -555,10 +573,7 @@ async function deployComponentOneShot(req, credentialReferences, isReplicatedExe // genuinely is needed, that component's already-running file watcher (Scope/ // EntryHandler, see deployLifecycle.ts) independently detects the post-deploy file // changes and requests the restart itself. - if (application.isNewComponent) { - const { requestRestart } = require('./requestRestart.ts'); - requestRestart(); - } + markRestartRequiredForNewComponent(application); response.message = `Successfully deployed: ${application.name}`; } @@ -722,7 +737,12 @@ async function deployComponentTwoPhase(req, credentialReferences) { emit('phase', { phase: 'restart', status: 'done' }); response.restartJobId = jobResponse.job_id; response.message = `Successfully deployed: ${application.name}, restarting Harper`; - } else response.message = `Successfully deployed: ${application.name}`; + } else { + // No restart requested: a genuinely-new component still needs one to serve its routes + // (harper#674). activateApplication set isNewComponent from the pre-swap live dir above. + markRestartRequiredForNewComponent(application); + response.message = `Successfully deployed: ${application.name}`; + } // ---- Activate gate: rare, but a node can stage OK and then fail the swap. ---- if (!req.ignore_replication_errors) { @@ -841,6 +861,11 @@ async function deployPhaseActivate(req) { // The origin sets restart=true on the sub-op only for an immediate restart; rolling restarts are // driven separately by the origin via a replicated restart_service job. if (req.restart === true) manageThreads.restartWorkers('http'); + // Not restarting now: mark restart-required per node for a genuinely-new component (harper#674), the + // same marking the one-shot peer path does — so a new component deployed cluster-wide with + // restart:false reports restartRequired on every node, not just the origin. A rolling restart, which + // also arrives here with restart:false, clears the flag when it reaches this node. + else markRestartRequiredForNewComponent(application); return { message: `Activated component: ${req.project}`, project: req.project, activated: true }; } @@ -899,6 +924,10 @@ async function deployComponentActivateExisting(req, credentialReferences) { emit('phase', { phase: 'restart', status: 'done' }); response.restartJobId = jobResponse.job_id; response.message = `Activated component: ${req.project}, restarting Harper`; + } else { + // No restart requested: activating a genuinely-new component still needs one to serve its routes + // (harper#674). activateApplication set isNewComponent from the pre-swap live dir above. + markRestartRequiredForNewComponent(application); } // Best-effort: flip the staged deployment row (left 'staged' by the stage-and-stop) to success now diff --git a/unitTests/components/deployStaging.test.js b/unitTests/components/deployStaging.test.js index a97a5c0607..696c8c3b64 100644 --- a/unitTests/components/deployStaging.test.js +++ b/unitTests/components/deployStaging.test.js @@ -101,6 +101,10 @@ describe('two-phase deploy primitives (stage / activate / discard)', function () assert.match(await readMarker(app.dirPath), /v1/, 'live dir holds the staged content'); assert.strictEqual(existsSync(app.stagingDirPath), false, 'the staged copy was consumed by the swap'); assert.strictEqual(app.buildDirPath, app.dirPath, 'build target reset to live after activation'); + // No live version existed before the swap, so this is a first deploy. deploy_component reads this + // to mark restartRequired for a never-loaded component (harper#674); staging is always fresh, so + // activate — not extract — is what establishes it on the two-phase path. + assert.strictEqual(app.isNewComponent, true, 'a first-ever activate marks the component new'); await fs.rm(app.dirPath, { recursive: true, force: true }); }); @@ -119,6 +123,9 @@ describe('two-phase deploy primitives (stage / activate / discard)', function () assert.match(await readMarker(dirPath), /v2/, 'live dir now holds the new version'); assert.strictEqual(existsSync(path.join(dirPath, 'leftover.txt')), false, 'old-version files are gone from live'); + // A live version existed before the swap, so this is a redeploy, not a first deploy — deploy_component + // must NOT self-request a restart here (harper#1806); the existing component's own watcher does. + assert.strictEqual(app.isNewComponent, false, 'activating over an existing live version marks it not-new'); await fs.rm(dirPath, { recursive: true, force: true }); await fs.rm(path.join(COMPONENTS_ROOT, ASIDE_STAGING_DIR), { recursive: true, force: true }); From 1ec722b4ad621cc0ed04f5841ec492de2168d4da Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 19:43:22 -0400 Subject: [PATCH 17/22] test(deploy): assert restart-required directly on every two-phase leg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activate-existing test only checked res.message, which never mentions "restart" on the no-restart path — so it passed whether or not the restart-required marking ran. Assert restartNeeded() directly instead, and cover the legs that had no fast test at all: - activate-existing (deployComponentActivateExisting): now asserts a never-live component marks a restart. - origin two-phase (deployComponentTwoPhase): asserts a fresh deploy marks a restart. - peer _phase:activate (deployPhaseActivate): new test driving the peer activate leg via the public op with internal markers, over a directly staged build — the per-node marking had zero coverage. - redeploy negative: a redeploy of an already-live component must NOT self-request a restart (harper#1806), guarding activateApplication's isNewComponent:false path at the operation level. beforeEach resets the process-wide restart buffer so each assertion reflects only its own deploy. Regressions on any leg now fail fast here instead of only in the inactive-component-404 integration test. Co-Authored-By: Claude Opus 4.8 --- .../components/deployPhaseOperations.test.js | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index 5d26f95b2e..d820a1f37a 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -19,8 +19,8 @@ const testUtils = require('../testUtils.js'); testUtils.preTestPrep(); const operations = require('#src/components/operations'); -const { DEPLOY_STAGING_DIR } = require('#src/components/Application'); -const { resetRestartNeeded } = require('#src/components/requestRestart'); +const { DEPLOY_STAGING_DIR, Application, stageApplication } = require('#src/components/Application'); +const { restartNeeded, resetRestartNeeded } = require('#src/components/requestRestart'); const { getConfigPath } = require('#src/config/configUtils'); const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); @@ -82,6 +82,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo resetRestartNeeded(); }); + // Each test below deploys fresh components, and deploying a never-live component flips the + // process-wide restart-needed buffer (harper#674). Start every test from a known-clean buffer so the + // restartNeeded() assertions below reflect only that test's own deploy, not a prior test's leak. + beforeEach(() => resetRestartNeeded()); + it('deploy_component({activate:false}) stages into the hidden dir without going live, and returns a deployment_id', async () => { const name = freshName(); const res = await operations.deployComponent({ @@ -116,6 +121,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo assert.match(await readIndex(liveDir), /op-activated/); // A restart was not requested, so the message must not claim one. assert.doesNotMatch(res.message, /restart/i); + // ...but this component was never live before, so activating it without a restart must mark one + // as required (harper#674) — the activate-existing leg of the two-phase restart-required fix. The + // message assertion above can't see this: the string never mentions "restart" on the no-restart + // path whether or not the marking runs, so assert the flag directly. + assert.strictEqual(restartNeeded(), true, 'activating a never-live component without restart marks a restart required'); }); it('deploy_component (two-phase default) stages then activates end-to-end', async () => { @@ -126,6 +136,8 @@ describe('deploy operations: stage_component / activate_component / deploy_compo assert.strictEqual(typeof res.deployment_id, 'string'); const liveDir = path.join(COMPONENTS_ROOT, name); assert.match(await readIndex(liveDir), /op-deployed/, 'component is live after a two-phase deploy'); + // New component deployed without a restart → restart required (harper#674), the origin two-phase leg. + assert.strictEqual(restartNeeded(), true, 'a fresh two-phase deploy without restart marks a restart required'); // The staged copy was consumed by the swap; its per-deploy staging parent is cleaned up. assert.strictEqual( existsSync(path.join(COMPONENTS_ROOT, DEPLOY_STAGING_DIR, res.deployment_id)), @@ -134,6 +146,43 @@ describe('deploy operations: stage_component / activate_component / deploy_compo ); }); + it('redeploying an already-live component without restart does NOT mark a restart required', async () => { + // The negative direction of harper#674/#1806: an existing, already-active component's own watcher + // requests any restart a redeploy needs, so deploy_component must stay quiet. Two-phase can't lean + // on extractApplication's in-place check (it builds into a fresh staging dir), so this guards that + // activateApplication correctly reports isNewComponent:false when a live version already exists. + const name = freshName(); + await operations.deployComponent({ project: name, payload: await makeComponentPayload('redeploy-v1') }); + resetRestartNeeded(); // clear the flag the first (new-component) deploy legitimately set + await operations.deployComponent({ project: name, payload: await makeComponentPayload('redeploy-v2') }); + assert.strictEqual(restartNeeded(), false, 'a redeploy of an already-live component must not self-request a restart'); + }); + + it('peer _phase:activate takes a locally-staged build live and marks a restart for a new component', async () => { + // The peer leg of the fix: a peer applies the fanned-out deploy_component tagged _phase:'activate' + // (deployPhaseActivate), swapping its OWN locally-staged build live. It must mark restart-required + // per node for a genuinely-new component (harper#674) — otherwise a cluster-wide restart:false + // deploy reports restartRequired on the origin only. Stage the build directly (standing in for the + // peer's earlier stage phase), then drive the activate phase through the public op with the + // internal markers, exactly as the replicated fan-out does. + const name = freshName(); + const deploymentId = `peer-activate-${name}`; + const staged = new Application({ name, payload: await makeComponentPayload('peer-activated'), stagingId: deploymentId }); + await stageApplication(staged); + + const res = await operations.deployComponent({ + project: name, + _phase: 'activate', + _deploymentId: deploymentId, + restart: false, + }); + + assert.strictEqual(res.activated, true, 'peer activate reports the component activated'); + const liveDir = path.join(COMPONENTS_ROOT, name); + assert.match(await readIndex(liveDir), /peer-activated/, 'the locally-staged build is now live'); + assert.strictEqual(restartNeeded(), true, 'peer activate of a never-live component without restart marks a restart required'); + }); + it('deploy_component with two_phase:false runs the legacy one-shot path', async () => { const name = freshName(); const res = await operations.deployComponent({ From 911d617a2d19b9178b59db20f3a893356bf3505b Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 21 Jul 2026 19:45:46 -0400 Subject: [PATCH 18/22] style(test): prettier line-wrap for deployPhaseOperations assertions Co-Authored-By: Claude Opus 4.8 --- .../components/deployPhaseOperations.test.js | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index d820a1f37a..de5ab7e512 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -125,7 +125,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo // as required (harper#674) — the activate-existing leg of the two-phase restart-required fix. The // message assertion above can't see this: the string never mentions "restart" on the no-restart // path whether or not the marking runs, so assert the flag directly. - assert.strictEqual(restartNeeded(), true, 'activating a never-live component without restart marks a restart required'); + assert.strictEqual( + restartNeeded(), + true, + 'activating a never-live component without restart marks a restart required' + ); }); it('deploy_component (two-phase default) stages then activates end-to-end', async () => { @@ -155,7 +159,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo await operations.deployComponent({ project: name, payload: await makeComponentPayload('redeploy-v1') }); resetRestartNeeded(); // clear the flag the first (new-component) deploy legitimately set await operations.deployComponent({ project: name, payload: await makeComponentPayload('redeploy-v2') }); - assert.strictEqual(restartNeeded(), false, 'a redeploy of an already-live component must not self-request a restart'); + assert.strictEqual( + restartNeeded(), + false, + 'a redeploy of an already-live component must not self-request a restart' + ); }); it('peer _phase:activate takes a locally-staged build live and marks a restart for a new component', async () => { @@ -167,7 +175,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo // internal markers, exactly as the replicated fan-out does. const name = freshName(); const deploymentId = `peer-activate-${name}`; - const staged = new Application({ name, payload: await makeComponentPayload('peer-activated'), stagingId: deploymentId }); + const staged = new Application({ + name, + payload: await makeComponentPayload('peer-activated'), + stagingId: deploymentId, + }); await stageApplication(staged); const res = await operations.deployComponent({ @@ -180,7 +192,11 @@ describe('deploy operations: stage_component / activate_component / deploy_compo assert.strictEqual(res.activated, true, 'peer activate reports the component activated'); const liveDir = path.join(COMPONENTS_ROOT, name); assert.match(await readIndex(liveDir), /peer-activated/, 'the locally-staged build is now live'); - assert.strictEqual(restartNeeded(), true, 'peer activate of a never-live component without restart marks a restart required'); + assert.strictEqual( + restartNeeded(), + true, + 'peer activate of a never-live component without restart marks a restart required' + ); }); it('deploy_component with two_phase:false runs the legacy one-shot path', async () => { From 758047c3b3f37330fb67e0d67d125a973b71ead6 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Wed, 22 Jul 2026 07:40:03 -0400 Subject: [PATCH 19/22] fix(deploy): markDeploymentTerminal must patch, not mutate the fetched row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit markDeploymentTerminal read the deployment row via table.get() and assigned onto it (row.status = …). table.get() returns a read-only record, so that throws "Cannot assign to read only property 'status'". The one caller (deploy_component({ deployment_id }) taking a stage-and-stop build live) treats the failure as best-effort, so it only surfaced as a caught warning — but the observability write never landed: get_deployment kept reporting 'staged' after the component had gone live. Use table.patch(id, { status, completed_at }) instead — the idiomatic partial update (see resources/dataLoader.ts). It avoids the read-only mutation and, unlike a spread-and-put, can't truncate the rest of the row. Regression test: deploymentRecorder mock now models the real table (read-only get() via an opt-in freezeGet, plus patch()), and a new markDeploymentTerminal suite asserts the row flips to a terminal status and is a no-op for an absent id. A revert to the row.status = … form throws against the frozen get(), so the regression can't return silently. Co-Authored-By: Claude Opus 4.8 --- components/deploymentRecorder.ts | 7 +-- .../components/deploymentRecorder.test.js | 44 +++++++++++++++++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index a8b2ec0567..703dc94ce4 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -470,9 +470,10 @@ export async function markDeploymentTerminal( if (!table) return; const row = await table.get(deploymentId); if (!row) return; - row.status = status; - row.completed_at = Date.now(); - await table.put(row); + // Patch (partial update) rather than mutating the row: table.get() returns a read-only record, so + // `row.status = …` throws "Cannot assign to read only property". A patch also touches only these two + // fields, so it can't truncate the rest of the row the way a spread-and-put might. + await table.patch(deploymentId, { status, completed_at: Date.now() }); } // Default peer-wait budget for the hdb_deployment row to replicate. A deploy is a rare, diff --git a/unitTests/components/deploymentRecorder.test.js b/unitTests/components/deploymentRecorder.test.js index 0bd2b21c75..d78f5d28c1 100644 --- a/unitTests/components/deploymentRecorder.test.js +++ b/unitTests/components/deploymentRecorder.test.js @@ -18,23 +18,35 @@ const { DeploymentRecorder, awaitDeploymentRow, readPayloadBlobWithRetry, + markDeploymentTerminal, } = require('#src/components/deploymentRecorder'); const { databases } = require('#src/resources/databases'); const terms = require('#src/utility/hdbTerms'); const DEPLOYMENT_TABLE = terms.SYSTEM_TABLE_NAMES.DEPLOYMENT_TABLE_NAME; -// Lightweight mock: keeps a Map of rows, exposes get(id) and put(row). -function installMockDeploymentTable() { +// Lightweight mock: keeps a Map of rows, exposes get(id), put(row), and patch(id, partial). With +// `freezeGet`, get() returns a FROZEN shallow copy to mirror the real table (table.get() yields a +// read-only record) so a caller that tries to mutate the fetched row — as the pre-fix +// markDeploymentTerminal did — throws here just as it would in production; the default returns the +// stored object so awaitDeploymentRow callers keep the real row (and its payload_blob). patch() +// applies a partial update to the stored row. +function installMockDeploymentTable({ freezeGet = false } = {}) { const rows = new Map(); const mock = { rows, async get(id) { - return rows.get(id); + const row = rows.get(id); + if (!row) return undefined; + return freezeGet ? Object.freeze({ ...row }) : row; }, async put(row) { rows.set(row.deployment_id, row); }, + async patch(id, partial) { + const existing = rows.get(id); + if (existing) rows.set(id, { ...existing, ...partial }); + }, }; if (!databases.system) databases.system = {}; const prior = databases.system[DEPLOYMENT_TABLE]; @@ -639,3 +651,29 @@ describe('readPayloadBlobWithRetry', () => { ); }); }); + +describe('markDeploymentTerminal', () => { + let installed; + beforeEach(() => { + // freezeGet mirrors the real table's read-only get() so a regression back to mutating the fetched + // row (row.status = …) throws here instead of silently passing against a mutable mock. + installed = installMockDeploymentTable({ freezeGet: true }); + }); + afterEach(() => installed.restore()); + + it('flips an existing row to the terminal status via patch (not a read-only mutation)', async () => { + // This is what deploy_component({ deployment_id }) does after taking a stage-and-stop build live. + // The row from table.get() is read-only; markDeploymentTerminal must patch rather than assign onto + // it (assigning threw "Cannot assign to read only property 'status'" and silently lost the update). + installed.mock.rows.set('dep-1', { deployment_id: 'dep-1', status: 'staged', completed_at: null }); + await markDeploymentTerminal('dep-1', 'success'); + const row = installed.mock.rows.get('dep-1'); + assert.strictEqual(row.status, 'success', 'the staged row was flipped to success'); + assert.strictEqual(typeof row.completed_at, 'number', 'completed_at was stamped'); + }); + + it('is a no-op when the row is absent (best-effort, never throws)', async () => { + await markDeploymentTerminal('missing', 'success'); + assert.strictEqual(installed.mock.rows.has('missing'), false, 'no row is fabricated for an unknown id'); + }); +}); From 3ebced77365e64006e79ea7b96f28953d755a53e Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Tue, 28 Jul 2026 19:36:21 -0400 Subject: [PATCH 20/22] fix(deploy): gate peer activation failures on the activate-existing path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deploy_component({ deployment_id }) fanned the activate out to peers but never inspected the result: replicateOperation doesn't reject on a per-peer failure (failures surface only as 'failed' peer entries), so a partially failed activate returned 2xx {activated:true} while part of the cluster stayed on the old version. revert_on_failure and ignore_replication_errors are accepted by deployComponentValidator for deploy_component uniformly, so both were silently no-ops whenever deployment_id was set. Extract the two-phase activate gate into enforceActivatePeerGate() and call it from both paths, so the flags behave identically whether the activate came from a full two-phase deploy or from an earlier stage-and-stop. The activate-existing path has no DeploymentRecorder (the row was created and finished as 'staged' by the stage), so createPeerResultCollector() stands in for recorder.recordPeer/getFailedPeers — same normalizePeerResult (now exported) and the same upsert-by-node dedup, so a peer reported via both onPeerResult and the final `replicated` aggregate counts once. A gated failure also marks the row 'failed' rather than leaving it 'staged'. Tests: the failed-peer scenario had no coverage on this path. Added two (replicateOperation swapped via the repo's property-swap pattern, no sinon/rewire) — a failed peer rejects, and ignore_replication_errors still resolves. Verified both fail with the gate disabled. Co-Authored-By: Claude Opus 4.8 --- components/deploymentRecorder.ts | 2 +- components/operations.js | 174 +++++++++++++----- .../components/deployPhaseOperations.test.js | 54 ++++++ 3 files changed, 180 insertions(+), 50 deletions(-) diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index 703dc94ce4..652de07f00 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -647,7 +647,7 @@ async function* readPayloadBlobChunks( } } -function normalizePeerResult(raw: unknown): Record { +export function normalizePeerResult(raw: unknown): Record { if (!raw || typeof raw !== 'object') { // Replication layer returned a primitive — preserve as a stringified marker so the // audit row at least records that something came back from a peer. diff --git a/components/operations.js b/components/operations.js index b21536918a..7b9a8171d9 100644 --- a/components/operations.js +++ b/components/operations.js @@ -40,6 +40,7 @@ const { DeploymentRecorder, awaitDeploymentRow, markDeploymentTerminal, + normalizePeerResult, readPayloadBlobWithRetry, coerceTimeoutMs, DEFAULT_AWAIT_ROW_TIMEOUT_MS, @@ -747,54 +748,14 @@ async function deployComponentTwoPhase(req, credentialReferences) { } // ---- Activate gate: rare, but a node can stage OK and then fail the swap. ---- - if (!req.ignore_replication_errors) { - const failed = recorder.getFailedPeers(); - if (failed.length > 0) { - let revertNote = ''; - // Opt-in swap-back: some nodes went live and some didn't, leaving the cluster split across - // versions. When revert_on_failure is set, roll the nodes that DID activate back to the - // retained previous version so the cluster reconverges. Best-effort — a revert failure must - // not mask the original activate failure. - if (req.revert_on_failure) { - try { - emit('phase', { phase: 'revert', status: 'start' }); - // The origin activated, so revert it. - await revertApplication(application); - // Revert ONLY the peers that successfully activated (see selectRevertTargets): every known - // node minus the ones that failed to activate (still on the correct version) and minus this - // node (already reverted directly above; a second bidirectional revert would flip it back). - // replicateOperation has no subset targeting, so send point-to-point via sendOperationToNode. - const { getThisNodeName } = require('../server/nodeName.ts'); - const activatedPeers = selectRevertTargets(server.nodes, failed, getThisNodeName()); - const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { - restart: req.restart, - deploymentId: recorder.deploymentId, - }); - revertOp.replicated = false; // point-to-point; the peer must not re-fan the revert - const revertResults = await Promise.allSettled( - activatedPeers.map((node) => server.replication.sendOperationToNode(node, revertOp)) - ); - const revertFailures = revertResults.filter((result) => result.status === 'rejected').length; - emit('phase', { phase: 'revert', status: 'done' }); - revertNote = - ` Rolled the origin and ${activatedPeers.length - revertFailures} of ${activatedPeers.length} ` + - `activated peer(s) back to the previous version (revert_on_failure); the ${failed.length} peer(s) ` + - `that never activated were left on their current (correct) version.` + - (revertFailures > 0 ? ` ${revertFailures} peer revert(s) also failed.` : '') + - ` Verify with get_components.`; - } catch (revertErr) { - log.warn('revert_on_failure rollback failed', revertErr); - revertNote = ` An automatic rollback (revert_on_failure) was attempted but also failed: ${revertErr?.message ?? revertErr}.`; - } - } - throw new ServerError( - `Component '${application.name}' was activated on the origin but failed to activate on ${failed.length} ` + - `of ${recorder.row.peer_results.length} peer node(s): ${describePeers(failed)}. Those nodes have the staged ` + - `build but did not go live.${revertNote} See deployment ${recorder.deploymentId} (get_deployment), or pass ` + - `ignore_replication_errors: true.` - ); - } - } + await enforceActivatePeerGate({ + req, + application, + emit, + failed: recorder.getFailedPeers(), + totalPeers: recorder.row.peer_results.length, + deploymentId: recorder.deploymentId, + }); response.deployment_id = recorder.deploymentId; maybeReclaimPayload(recorder, emit); @@ -900,7 +861,17 @@ async function deployComponentActivateExisting(req, credentialReferences) { restart: rollingRestart ? false : req.restart, deploymentId: stagingId, }); - const rep = await server.replication.replicateOperation(activateOp, {}); + // Collect per-peer outcomes so a partially-failed activate can be gated below. There is no + // DeploymentRecorder on this path (the row was created and finished as `staged` by the earlier + // stage-and-stop), so a local collector stands in for recorder.recordPeer/getFailedPeers. + const peers = createPeerResultCollector(); + const rep = await server.replication.replicateOperation(activateOp, { + onPeerResult: (result) => { + peers.record(result); + emit('peer', result); + }, + }); + if (rep?.replicated) peers.recordAll(rep.replicated); const response = { message: `Activated component: ${req.project}`, @@ -932,6 +903,27 @@ async function deployComponentActivateExisting(req, credentialReferences) { markRestartRequiredForNewComponent(application); } + // ---- Activate gate: a peer can hold a good staged build and still fail the swap. Same gate the + // two-phase activate phase uses, so revert_on_failure / ignore_replication_errors behave identically + // whether the activate came from a full deploy or from `deploy_component({ deployment_id })`. + try { + await enforceActivatePeerGate({ + req, + application, + emit, + failed: peers.getFailed(), + totalPeers: peers.total, + deploymentId: stagingId, + }); + } catch (err) { + // The origin went live but the cluster did not converge — record the terminal state before + // surfacing the failure, so get_deployment doesn't still read `staged`. + await markDeploymentTerminal(stagingId, 'failed').catch((markErr) => + log.warn('Failed to mark deployment as failed after a partial activate', markErr) + ); + throw err; + } + // Best-effort: flip the staged deployment row (left 'staged' by the stage-and-stop) to success now // that it is live. Observability only — a tracking-write failure must not fail the activate. await markDeploymentTerminal(stagingId, 'success').catch((err) => @@ -1215,6 +1207,90 @@ function describePeers(failedPeers) { return failedPeers.map((peer) => `${peer.node ?? 'unknown'} (${peer.error?.message ?? 'unknown error'})`).join(', '); } +/** + * Collect per-peer replication outcomes when there is no DeploymentRecorder to hold them — the + * activate-existing path, where the hdb_deployment row was already created (and finished as `staged`) + * by the earlier stage-and-stop. Mirrors DeploymentRecorder.recordPeer's semantics exactly: results are + * normalized by the same normalizePeerResult and upserted by node name, so a peer reported both through + * the streaming `onPeerResult` callback and again in replicateOperation's final `replicated` aggregate + * is counted once, not twice. + */ +function createPeerResultCollector() { + const list = []; + const record = (result) => { + const normalized = normalizePeerResult(result); + const nodeName = normalized.node; + const idx = nodeName ? list.findIndex((entry) => entry.node === nodeName) : -1; + if (idx >= 0) list[idx] = normalized; + else list.push(normalized); + }; + return { + record, + recordAll(results) { + if (Array.isArray(results)) for (const result of results) record(result); + }, + getFailed: () => list.filter((peer) => peer?.status === 'failed'), + get total() { + return list.length; + }, + }; +} + +/** + * Shared post-activate failure gate for every cluster-wide activate (the two-phase deploy's activate + * phase and `deploy_component({ deployment_id })`). replicateOperation never rejects on a per-peer + * failure — failures surface only as 'failed' peer entries — so without this gate a partially-failed + * activate returns 2xx and silently leaves the cluster split across versions. + * + * Unless `ignore_replication_errors` is set, throws when any peer failed to activate. When + * `revert_on_failure` is set, first rolls the origin and the peers that DID activate back to the + * retained previous version so the cluster reconverges — best-effort, since a revert failure must not + * mask the original activate failure. + */ +async function enforceActivatePeerGate({ req, application, emit, failed, totalPeers, deploymentId }) { + if (req.ignore_replication_errors) return; + if (!failed || failed.length === 0) return; + let revertNote = ''; + if (req.revert_on_failure) { + try { + emit('phase', { phase: 'revert', status: 'start' }); + // The origin activated, so revert it. + await revertApplication(application); + // Revert ONLY the peers that successfully activated (see selectRevertTargets): every known + // node minus the ones that failed to activate (still on the correct version) and minus this + // node (already reverted directly above; a second bidirectional revert would flip it back). + // replicateOperation has no subset targeting, so send point-to-point via sendOperationToNode. + const { getThisNodeName } = require('../server/nodeName.ts'); + const activatedPeers = selectRevertTargets(server.nodes, failed, getThisNodeName()); + const revertOp = buildReplicatedSubOp(req, hdbTerms.OPERATIONS_ENUM.REVERT_COMPONENT, { + restart: req.restart, + deploymentId, + }); + revertOp.replicated = false; // point-to-point; the peer must not re-fan the revert + const revertResults = await Promise.allSettled( + activatedPeers.map((node) => server.replication.sendOperationToNode(node, revertOp)) + ); + const revertFailures = revertResults.filter((result) => result.status === 'rejected').length; + emit('phase', { phase: 'revert', status: 'done' }); + revertNote = + ` Rolled the origin and ${activatedPeers.length - revertFailures} of ${activatedPeers.length} ` + + `activated peer(s) back to the previous version (revert_on_failure); the ${failed.length} peer(s) ` + + `that never activated were left on their current (correct) version.` + + (revertFailures > 0 ? ` ${revertFailures} peer revert(s) also failed.` : '') + + ` Verify with get_components.`; + } catch (revertErr) { + log.warn('revert_on_failure rollback failed', revertErr); + revertNote = ` An automatic rollback (revert_on_failure) was attempted but also failed: ${revertErr?.message ?? revertErr}.`; + } + } + throw new ServerError( + `Component '${application.name}' was activated on the origin but failed to activate on ${failed.length} ` + + `of ${totalPeers} peer node(s): ${describePeers(failed)}. Those nodes have the staged ` + + `build but did not go live.${revertNote} See deployment ${deploymentId} (get_deployment), or pass ` + + `ignore_replication_errors: true.` + ); +} + // Choose which peers a revert_on_failure swap-back should target: every known node EXCEPT // - `thisNodeName`: the origin, already reverted directly by the caller — a second (bidirectional) // revert would flip it back to the just-activated version; and diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index de5ab7e512..dd7484acd2 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -21,6 +21,7 @@ testUtils.preTestPrep(); const operations = require('#src/components/operations'); const { DEPLOY_STAGING_DIR, Application, stageApplication } = require('#src/components/Application'); const { restartNeeded, resetRestartNeeded } = require('#src/components/requestRestart'); +const { server } = require('#src/server/Server'); const { getConfigPath } = require('#src/config/configUtils'); const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); @@ -132,6 +133,59 @@ describe('deploy operations: stage_component / activate_component / deploy_compo ); }); + // The activate-existing path fans the activate out to peers just like the two-phase activate phase, + // and replicateOperation never rejects on a per-peer failure — failures surface only as 'failed' + // entries. Without the shared activate gate this path returned 2xx {activated:true} while part of the + // cluster stayed on the old version, making revert_on_failure/ignore_replication_errors no-ops here. + describe('deploy_component({deployment_id}) peer-failure gate', () => { + let priorReplicate; + // Swap in a replicator that reports one failed peer (the repo's property-swap pattern; no + // sinon/rewire per AGENTS.md). Restored after each test. + function stubFailedPeer() { + priorReplicate = server.replication.replicateOperation; + server.replication.replicateOperation = async () => ({ + replicated: [{ node: 'peer-a', status: 'failed', reason: 'no staged build found' }], + }); + } + afterEach(() => { + if (priorReplicate) server.replication.replicateOperation = priorReplicate; + priorReplicate = undefined; + }); + + async function stageOnly(name, marker) { + const staged = await operations.deployComponent({ + project: name, + payload: await makeComponentPayload(marker), + activate: false, + }); + return staged.deployment_id; + } + + it('rejects (does not report success) when a peer fails to activate', async () => { + const name = freshName(); + const deploymentId = await stageOnly(name, 'gate-fail'); + stubFailedPeer(); + await assert.rejects( + () => operations.deployComponent({ project: name, deployment_id: deploymentId }), + /failed to activate on 1 .*peer node\(s\).*peer-a/s, + 'a partially-failed activate must surface as an error, not a 2xx success' + ); + }); + + it('honors ignore_replication_errors on this path, resolving despite the failed peer', async () => { + const name = freshName(); + const deploymentId = await stageOnly(name, 'gate-ignore'); + stubFailedPeer(); + const res = await operations.deployComponent({ + project: name, + deployment_id: deploymentId, + ignore_replication_errors: true, + }); + assert.strictEqual(res.activated, true, 'the opt-out still returns success'); + assert.match(await readIndex(path.join(COMPONENTS_ROOT, name)), /gate-ignore/, 'the origin still went live'); + }); + }); + it('deploy_component (two-phase default) stages then activates end-to-end', async () => { const name = freshName(); const res = await operations.deployComponent({ project: name, payload: await makeComponentPayload('op-deployed') }); From 525dfd76d2b4a0757f09a3224cd1b0c768eeeec3 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Thu, 30 Jul 2026 09:25:08 -0400 Subject: [PATCH 21/22] fix(deploy): persist root config on activate-by-id; restart the reverted origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings from cb1kenobi on #1849. Medium — a package deploy staged with `activate: false` and activated later by `deployment_id` never persisted its root-config entry. writeComponentRootConfig is gated on `req.package`, and an activate-by-id request carries none (`harper activate` sends only project + deployment_id). The fan-out copies `package` from the same request, so peers skipped it too: the package reference and the credential references that cold reinstalls and newly-joined peers rely on were silently lost, leaving the component recorded as a plain directory. deployComponentActivateExisting now recovers `package_identifier` — and the credential REFERENCES, which the row already stores — from the staged deployment row when the request omits them, so the origin persists config and the recovered identifier rides the sub-op out to every peer. Explicit request values still win. Recovery also means the protected-core-name guard now applies to this path. Added getDeploymentRow() for the lookup: a plain point-read, deliberately not awaitDeploymentRow, which polls for a row AND its payload_blob and so would never return a deployment whose payload retention already reclaimed — exactly the row this path still needs. Low — with `restart: true`, revert_on_failure left the origin diverged. The origin restart runs before the activate gate, so by the time the gate reverts the origin's directory its workers are already on the new version, while the peers' revert op carries `restart` and does come back on the previous one. The origin now restarts after its revert, so both ends of the reconvergence match. A rolling restart arrives with `restart` normalized to false on both sides, so it needs no second restart. Tests: 4 for getDeploymentRow (including the reclaimed-payload case that distinguishes it from awaitDeploymentRow). 146 deploy tests passing. --- components/deploymentRecorder.ts | 16 ++++++ components/operations.js | 32 +++++++++++ .../components/deploymentRecorder.test.js | 53 +++++++++++++++++++ 3 files changed, 101 insertions(+) diff --git a/components/deploymentRecorder.ts b/components/deploymentRecorder.ts index 64cf9b2c0b..c33bb59384 100644 --- a/components/deploymentRecorder.ts +++ b/components/deploymentRecorder.ts @@ -455,6 +455,22 @@ export class DeploymentRecorder { } } +/** + * Point-read a deployment row by id, or undefined when the row (or the table) is absent. + * + * Distinct from `awaitDeploymentRow`, which polls for a row to arrive by replication AND to carry a + * `payload_blob`: this is for reading a row the caller already owns locally — e.g. recovering a staged + * deployment's `package_identifier` when it is activated later by `deployment_id`. A row whose payload + * has been reclaimed by retention is a valid result here, which is exactly what awaitDeploymentRow + * would refuse to return. + */ +export async function getDeploymentRow(deploymentId: string): Promise | undefined> { + if (!deploymentId) return undefined; + const table = (databases as any).system?.[terms.SYSTEM_TABLE_NAMES.DEPLOYMENT_TABLE_NAME]; + if (!table) return undefined; + return table.get(deploymentId); +} + /** * Best-effort terminal-status update for an existing deployment row by id, used when a later operation * finishes a deployment the current process didn't record with a live DeploymentRecorder — e.g. diff --git a/components/operations.js b/components/operations.js index c00fdc4d44..6f2b0aac80 100644 --- a/components/operations.js +++ b/components/operations.js @@ -39,6 +39,7 @@ const { server } = require('../server/Server.ts'); const { DeploymentRecorder, awaitDeploymentRow, + getDeploymentRow, markDeploymentTerminal, normalizePeerResult, pruneProjectPayloads, @@ -845,6 +846,29 @@ async function deployPhaseActivate(req) { */ async function deployComponentActivateExisting(req, credentialReferences) { const stagingId = req.deployment_id; + // An activate-by-id call carries no `package` — `harper activate` sends only project + deployment_id, + // and the docs describe this path as fetching/installing nothing — so recover the staged deployment's + // package identifier and credential references from its row. Without this, a component staged as a + // `package` deploy and activated later would never persist its root-config entry: not on the origin + // (writeComponentRootConfig is gated on `req.package`) and not on any peer either, since the fanned-out + // sub-op copies `package`/`credentials` from this same `req`. The package reference and the credential + // references that cold reinstalls and newly-joined peers depend on would be silently lost, leaving the + // component recorded as a plain directory. Explicit values on the request always win. + if (!req.package) { + const stagedRow = await getDeploymentRow(stagingId).catch((err) => { + log.warn(`Could not read deployment ${stagingId} to recover its package identifier`, err); + return undefined; + }); + if (stagedRow?.package_identifier) { + req.package = stagedRow.package_identifier; + // The row stores credential REFERENCES (tokens were never persisted), which is exactly what + // root config should carry. Only fall back to them when the caller supplied none. + if (!req.credentials?.length && Array.isArray(stagedRow.credentials) && stagedRow.credentials.length) { + req.credentials = stagedRow.credentials; + } + credentialReferences = (req.credentials ?? []).filter((entry) => entry && entry.secret !== undefined); + } + } if (req.package) assertNotProtectedCoreComponent(req.project, req.force); const emitter = req.progress ?? new ProgressEmitter(); if (!req.progress) req.progress = emitter; @@ -1261,6 +1285,14 @@ async function enforceActivatePeerGate({ req, application, emit, failed, totalPe emit('phase', { phase: 'revert', status: 'start' }); // The origin activated, so revert it. await revertApplication(application); + // With `restart: true` the origin's workers already reloaded onto the new (failed-cluster) + // version — the origin restart runs before this gate — so the directory rollback above is not + // picked up on its own. The peers' revert op carries `restart`, so they DO come back on the + // previous version; without this the origin would be the one node left serving the new version, + // the exact opposite of the reconvergence revert_on_failure exists to provide. A rolling restart + // arrives here with `restart` already normalized to false and its peers likewise un-restarted, + // so origin and peers stay consistent in that case without a second restart. + if (req.restart === true) manageThreads.restartWorkers('http'); // Revert ONLY the peers that successfully activated (see selectRevertTargets): every known // node minus the ones that failed to activate (still on the correct version) and minus this // node (already reverted directly above; a second bidirectional revert would flip it back). diff --git a/unitTests/components/deploymentRecorder.test.js b/unitTests/components/deploymentRecorder.test.js index 7d234af481..b4ce153b36 100644 --- a/unitTests/components/deploymentRecorder.test.js +++ b/unitTests/components/deploymentRecorder.test.js @@ -20,6 +20,7 @@ const { readPayloadBlobWithRetry, markDeploymentTerminal, pruneProjectPayloads, + getDeploymentRow, } = require('#src/components/deploymentRecorder'); const { databases } = require('#src/resources/databases'); const terms = require('#src/utility/hdbTerms'); @@ -686,6 +687,58 @@ describe('markDeploymentTerminal', () => { }); }); +describe('getDeploymentRow', () => { + let installed; + beforeEach(() => { + installed = installMockDeploymentTable(); + }); + afterEach(() => installed.restore()); + + it('returns the row, including its package_identifier', async () => { + // deploy_component({deployment_id}) reads this to recover the package identifier of a deployment + // that was staged as a `package` deploy — an activate-by-id request carries no `package` of its own. + installed.mock.rows.set('dep-1', { + deployment_id: 'dep-1', + project: 'my-app', + package_identifier: 'npm:@my-org/my-app@1.2.3', + status: 'staged', + }); + const row = await getDeploymentRow('dep-1'); + assert.strictEqual(row?.package_identifier, 'npm:@my-org/my-app@1.2.3'); + }); + + it('returns a row whose payload has already been reclaimed', async () => { + // The point of this helper over awaitDeploymentRow: that one polls until the row carries a + // payload_blob, so it would never return a deployment whose payload retention already dropped — + // which is exactly the row an activate-by-id still needs to read. + installed.mock.rows.set('reclaimed', { + deployment_id: 'reclaimed', + package_identifier: 'npm:pkg@1', + payload_blob: null, + status: 'success', + }); + const row = await getDeploymentRow('reclaimed'); + assert.strictEqual(row?.package_identifier, 'npm:pkg@1', 'a payload-less row is still returned'); + }); + + it('returns undefined for an unknown id or a blank id', async () => { + assert.strictEqual(await getDeploymentRow('nope'), undefined); + assert.strictEqual(await getDeploymentRow(''), undefined); + }); + + it('returns undefined when the deployment table is not provisioned', async () => { + installed.restore(); + const prior = databases.system?.[DEPLOYMENT_TABLE]; + if (databases.system) delete databases.system[DEPLOYMENT_TABLE]; + try { + assert.strictEqual(await getDeploymentRow('dep-1'), undefined, 'a missing table is not an error'); + } finally { + if (databases.system && prior !== undefined) databases.system[DEPLOYMENT_TABLE] = prior; + installed = installMockDeploymentTable(); + } + }); +}); + describe('pruneProjectPayloads (deployment_payloadRetention_maxCount)', () => { let installed; beforeEach(() => { From 764ce6777becf072f2e0780d1b8ac87c660a68a5 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Thu, 30 Jul 2026 09:41:15 -0400 Subject: [PATCH 22/22] test(deploy): cover package-identifier recovery on activate-by-id end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recovery branch added in 525dfd76d was only covered at the getDeploymentRow unit level; nothing drove a `package` deploy through stage → activate-by-id to prove root config actually gets persisted and that the recovered identifier reaches peers — the exact bug that commit fixed. Added that test: stages a `file:` tarball package with `activate: false` (no network, and a package deploy needs no payload blob, so a mock deployment table is enough), then activates by deployment_id with NO `package` on the request — what `harper activate` sends — and asserts both that the origin wrote the package reference to root config and that the activate sub-op carries it so peers do the same. The config file is snapshotted and restored around the test. Verified it is a real regression test: with the recovery branch disabled it fails on the sub-op assertion, and passes with it restored. A `package` deploy runs the protected-core-name guard, which requires componentLoader and so pulls in the private @harperfast/skills dependency that some local checkouts lack (which is why every other test in this file uses payload deploys). The test probes for it and skips rather than failing on a missing dependency; CI has it and runs the assertions for real. --- .../components/deployPhaseOperations.test.js | 91 ++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/unitTests/components/deployPhaseOperations.test.js b/unitTests/components/deployPhaseOperations.test.js index dd7484acd2..7da26cb21a 100644 --- a/unitTests/components/deployPhaseOperations.test.js +++ b/unitTests/components/deployPhaseOperations.test.js @@ -22,9 +22,12 @@ const operations = require('#src/components/operations'); const { DEPLOY_STAGING_DIR, Application, stageApplication } = require('#src/components/Application'); const { restartNeeded, resetRestartNeeded } = require('#src/components/requestRestart'); const { server } = require('#src/server/Server'); -const { getConfigPath } = require('#src/config/configUtils'); +const { databases } = require('#src/resources/databases'); +const { SYSTEM_TABLE_NAMES } = require('#src/utility/hdbTerms'); +const { getConfigPath, getConfiguration, getConfigFilePath } = require('#src/config/configUtils'); const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); +const DEPLOYMENT_TABLE = SYSTEM_TABLE_NAMES.DEPLOYMENT_TABLE_NAME; const COMPONENTS_ROOT = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); // Pack a directory's CONTENTS into a gzipped tar Buffer, the shape a deploy payload takes. @@ -55,6 +58,18 @@ async function makeComponentPayload(marker) { const readIndex = (dir) => fs.readFile(path.join(dir, 'index.js'), 'utf8'); +// componentLoader reaches the private `@harperfast/skills` dependency, which is absent from some local +// checkouts. Only the `package`-deploy path touches it (via the protected-core-name guard), so probe +// once and let that one test skip rather than fail on a missing dependency. +function componentLoaderAvailable() { + try { + require('#src/components/componentLoader'); + return true; + } catch { + return false; + } +} + describe('deploy operations: stage_component / activate_component / deploy_component', function () { this.timeout(30_000); @@ -161,6 +176,80 @@ describe('deploy operations: stage_component / activate_component / deploy_compo return staged.deployment_id; } + // End-to-end cover for the package-identifier recovery: stage a `package` deploy with + // `activate: false`, then activate it by id with no `package` on the request (what `harper activate` + // sends) and assert root config is persisted AND the recovered identifier reaches peers. A `file:` + // tarball package needs no network and no payload blob — extraction reads the tarball directly — so + // a mock deployment table is enough to drive the whole path. + it('recovers a staged package deploy: persists root config and fans the package out to peers', async function () { + // Unlike the payload deploys used elsewhere in this file, a `package` deploy runs the + // protected-core-name guard, which requires componentLoader and so pulls in the private + // `@harperfast/skills` dependency. That isn't installed in every local checkout, so skip there + // instead of failing on the environment; CI installs it and runs this for real. + if (!componentLoaderAvailable()) return this.skip(); + const name = freshName(); + const tgzDir = await fs.mkdtemp(path.join(os.tmpdir(), 'harper-op-pkg-')); + const tgzPath = path.join(tgzDir, 'component.tgz'); + await fs.writeFile(tgzPath, await makeComponentPayload('pkg-staged')); + const packageId = `file:${tgzPath}`; + + const rows = new Map(); + const priorTable = databases.system?.[DEPLOYMENT_TABLE]; + if (!databases.system) databases.system = {}; + databases.system[DEPLOYMENT_TABLE] = { + async get(id) { + return rows.get(id); + }, + async put(row) { + rows.set(row.deployment_id, { ...row }); + }, + async patch(id, partial) { + const existing = rows.get(id); + if (existing) rows.set(id, { ...existing, ...partial }); + }, + async *search(conditions = []) { + for (const row of rows.values()) if (conditions.every((c) => row[c.attribute] === c.value)) yield row; + }, + }; + const configBackup = await fs.readFile(getConfigFilePath(), 'utf8'); + + try { + const staged = await operations.deployComponent({ project: name, package: packageId, activate: false }); + assert.strictEqual(staged.staged, true, 'the package deploy staged without going live'); + assert.strictEqual( + rows.get(staged.deployment_id)?.package_identifier, + packageId, + 'the stage recorded the package identifier on the row — the source the activate recovers from' + ); + + let fannedOut; + priorReplicate = server.replication.replicateOperation; + server.replication.replicateOperation = async (op) => { + fannedOut = op; + return {}; + }; + + // No `package` here — exactly what `harper activate project=… deployment_id=…` sends. + await operations.deployComponent({ project: name, deployment_id: staged.deployment_id }); + + assert.strictEqual( + fannedOut?.package, + packageId, + 'the recovered identifier rides the activate sub-op, so peers persist root config too' + ); + assert.strictEqual( + getConfiguration()[name]?.package, + packageId, + 'the origin persisted the package reference to root config' + ); + } finally { + await fs.writeFile(getConfigFilePath(), configBackup); + if (priorTable === undefined) delete databases.system[DEPLOYMENT_TABLE]; + else databases.system[DEPLOYMENT_TABLE] = priorTable; + await fs.rm(tgzDir, { recursive: true, force: true }); + } + }); + it('rejects (does not report success) when a peer fails to activate', async () => { const name = freshName(); const deploymentId = await stageOnly(name, 'gate-fail');