diff --git a/docs/reference/native-compose-adoption.md b/docs/reference/native-compose-adoption.md index cb3e77d9..91f4d439 100644 --- a/docs/reference/native-compose-adoption.md +++ b/docs/reference/native-compose-adoption.md @@ -553,7 +553,60 @@ diagnostic; the literal-env message appears only on the first refusal. Plain override, so whenever `hack config adopt` refuses as busy over a pending switch its message adds the step that state accepts: `--recover --rollback` when the bound override holds literal values, otherwise `--recover` to complete or -`--recover --rollback` to restore (HACK-1414). +`--recover --rollback` to restore (HACK-1414). Once the switch's rollback is +journaled (`rolling-back`, an interrupted rollback), plain `--recover` refuses +whatever the override holds, so the message names only `--recover --rollback` +to finish restoring the originals (HACK-1427). + +#### Rollback releases the generated env override (HACK-1437) + +A saved read binds each generated override in `.hack/.internal` by its exact +bytes and file identity: device, inode, mode, link count, size, owner, mtime +and ctime. Moving the file aside and back keeps its bytes and inode but +changes its ctime, so the binding cannot be restored by hand. The binding +protects two things. Adopted execution and switch completion must run exactly +the inputs that preparation qualified and whose Compose config hash the +retained containers carry, so a moved, edited or replaced file is treated as +tampering and refused. And while an adoption is prepared, active or pending, +`hack doctor --fix` and the env scrubs leave the bound file in place. + +Rollback is different. It restores the two held legacy originals, archives the +candidate and verifies the original resources. It never reads, runs, rewrites +or removes the generated env override, which is derived from the managed env +layers and which the next `hack up` writes again. No data depends on it. So +rollback (`--rollback` from `active`, and `--recover --rollback` from +`switching` or `rolling-back`) reads the saved proof with the env override +released: the override may be missing, moved back or replaced, and rollback +neither reads it nor requires it. The saved `config_files` path list keeps the +override's path, so each retained container is still matched against the +exact label Compose recorded. Every other check stays exact: the managed env +revision, the runtime override and the `.hack/.internal` directory identity, +the saved config hashes, the original resource identities and stopped state, +the held originals and the installed candidate. A change to any of those is +still refused before anything moves. Execution and completion keep the full +binding, so they still refuse a moved override. + +Before this change, moving the bound override aside left an interrupted +rollback with no supported way out. `--recover --rollback` journaled +`rolling-back` and then refused, plain `--recover` refuses a rollback, and the +busy message named `--recover --rollback` again, so the user looped. Now +`--recover --rollback` finishes in each pending state, and every step names +only a command that the state does not refuse for a known reason: + +- `rolling-back`: `--recover --rollback`, whatever the override holds. +- `switching` with a literal override: `--recover --rollback`. +- `switching` with the bound override moved or changed, while the other + generated inputs still match: `--recover --rollback`, with a message that + completing refuses. +- `switching` otherwise: `--recover` to complete it, or `--recover --rollback` + to restore the originals. + +A refused plain `--recover` adds the same step when the cause is one of these +known states (`rolling-back`, or a moved or changed override). If rollback +itself refuses, something it still checks has changed, such as a running +original container, edited held originals or candidate, changed resources or +managed env layers, or conflicting bytes at an original path. That refusal +keeps the journal and recovery evidence for inspection. `hack doctor` reports the pinned literal override and names the first command the pending state accepts, since `hack down` and plain `--rollback` refuse in every @@ -630,7 +683,10 @@ files, and preserve both legacy originals for rollback. Raw-byte or selection drift, unsafe managed-file permissions, generated-file identity changes, and original-resource drift refuse before a new effect; drift after an effect retains pending evidence. Rechecks do not freeze -external editors or the engine. Old version 1/2 receipts remain readable; earlier +external editors or the engine. Rollback is the one exception: it releases the +generated env override, which it never reads or writes, and rechecks the rest +(see "Rollback releases the generated env override"). Old version 1/2 receipts +remain readable; earlier upgraded owners refuse version 3 through the existing selector boundary. The version 3 synthetic publication, saved-readback and raw-drift controls are diff --git a/packages/runtime-core/src/provider/test_executable.rs b/packages/runtime-core/src/provider/test_executable.rs index 6acd16d0..4df133af 100644 --- a/packages/runtime-core/src/provider/test_executable.rs +++ b/packages/runtime-core/src/provider/test_executable.rs @@ -4,6 +4,7 @@ use std::{ os::unix::fs::{OpenOptionsExt, PermissionsExt}, path::Path, process::{Child, Command, Stdio}, + thread::JoinHandle, time::{Duration, Instant}, }; @@ -48,12 +49,69 @@ pub(super) fn sleeping_executable(binary: &Path) { /// may be heavily loaded, so this bounds a hang rather than the fixture's own timing. pub(super) const ISOLATED_HANG_GUARD: Duration = Duration::from_secs(120); +/// Bytes of each child stream kept for a failure message. The readers always drain the +/// whole stream, so the child is never blocked on a full pipe; only the tail, where libtest +/// prints the failing assertion and the result line, is retained. +const RETAINED_TAIL: usize = 16 * 1024; + +/// Largest stdout a passing child may print: one isolated test's libtest summary is far +/// smaller, so more means the child ran something other than the requested fixture. +const PASSING_STDOUT_LIMIT: usize = 4096; + +/// One child stream drained to its end on a reader thread, keeping a bounded tail. +struct DrainedStream { + tail: Vec, + total: usize, +} + +impl DrainedStream { + fn spawn(mut source: impl Read + Send + 'static) -> JoinHandle { + std::thread::spawn(move || { + let mut drained = Self { + tail: Vec::new(), + total: 0, + }; + let mut chunk = [0_u8; 8192]; + loop { + match source.read(&mut chunk) { + Ok(0) => break, + Ok(read) => { + drained.total += read; + drained.tail.extend_from_slice(&chunk[..read]); + let excess = drained.tail.len().saturating_sub(RETAINED_TAIL); + drained.tail.drain(..excess); + } + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => {} + Err(_) => break, + } + } + drained + }) + } + + fn text(&self) -> String { + let omitted = self.total - self.tail.len(); + let tail = String::from_utf8_lossy(&self.tail); + if omitted == 0 { + tail.into_owned() + } else { + format!("[{omitted} earlier bytes omitted]\n{tail}") + } + } +} + /// Run one ignored test alone in a child process of this test binary and require its /// complete assertion path. Use it when a fixture's descriptors must not be shared with /// unrelated parallel tests: their pre-exec forks (`process::clean_command`) hold copies /// of every descriptor of this process until they exec, so a whole-host process scan or /// an `flock` release can observe this test's sockets, files or locks in another /// process. `bound` is a hang guard for launching the whole binary on a loaded host. +/// +/// The child's stdout and stderr are drained on reader threads while it runs: a child +/// that fails with more output than a pipe buffer holds must exit and report its own +/// failure, not block on a write until the hang guard reports a watchdog instead. The +/// bound covers both the child's exit and the readers reaching end of stream, so a +/// descendant that keeps a copy of either pipe open is reported rather than waited on. pub(super) fn run_isolated(test: &str, bound: Duration) { struct OwnedChild(Child); impl Drop for OwnedChild { @@ -67,11 +125,13 @@ pub(super) fn run_isolated(test: &str, bound: Duration) { .args(["--ignored", "--exact", test, "--test-threads=1"]) .stdin(Stdio::null()) .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) + .stderr(Stdio::piped()) .spawn() .unwrap(), ); let started = Instant::now(); + let stdout = DrainedStream::spawn(child.0.stdout.take().unwrap()); + let stderr = DrainedStream::spawn(child.0.stderr.take().unwrap()); let status = loop { if let Some(status) = child.0.try_wait().unwrap() { break status; @@ -83,20 +143,80 @@ pub(super) fn run_isolated(test: &str, bound: Duration) { ); std::thread::sleep(Duration::from_millis(1)); }; - let mut output = String::new(); - child - .0 - .stdout - .take() - .unwrap() - .take(4097) - .read_to_string(&mut output) - .unwrap(); - assert!(output.len() <= 4096); - assert!(status.success(), "isolated test {test} failed: {output}"); + while !(stdout.is_finished() && stderr.is_finished()) { + assert!( + started.elapsed() < bound, + "isolated test {test} exited with {status} but its output stayed open past its \ + watchdog after {:?}", + started.elapsed() + ); + std::thread::sleep(Duration::from_millis(1)); + } + let stdout = stdout.join().unwrap(); + let stderr = stderr.join().unwrap(); + let (output, errors) = (stdout.text(), stderr.text()); assert!( - output.contains(&format!("test {test} ... ok")) + status.success(), + "isolated test {test} failed with {status}\n--- stdout ---\n{output}\n--- stderr ---\n{errors}" + ); + assert!( + stdout.total <= PASSING_STDOUT_LIMIT + && output.contains(&format!("test {test} ... ok")) && output.contains("test result: ok. 1 passed"), - "isolated test {test} assertions did not run: {output}" + "isolated test {test} assertions did not run\n--- stdout ---\n{output}\n--- stderr ---\n{errors}" ); } + +#[cfg(test)] +mod tests { + use super::*; + + /// More than a 64 KiB pipe buffer on each stream, so an unread pipe blocks the child. + const NOISY_BYTES: usize = 160 * 1024; + const NOISY_FAILURE: &str = "noisy isolated child failed on purpose"; + const NOISY_STDERR_END: &str = "noisy isolated child stderr end"; + + #[test] + #[ignore = "deliberately failing child launched by isolated_child_failure_with_large_output_is_reported"] + fn noisy_failure_child() { + let line = "x".repeat(1023); + // libtest captures this and prints it with the failure, on stdout, before the panic. + for _ in 0..NOISY_BYTES / 1024 { + println!("{line}"); + } + // Direct writes bypass libtest's capture and reach the stderr pipe as written. + let mut stderr = std::io::stderr().lock(); + for _ in 0..NOISY_BYTES / 1024 { + writeln!(stderr, "{line}").unwrap(); + } + writeln!(stderr, "{NOISY_STDERR_END}").unwrap(); + drop(stderr); + panic!("{NOISY_FAILURE}"); + } + + /// A child whose failure output overflows both pipes must still exit and report its own + /// failure: an unread pipe would block it until the hang guard hid the real failure + /// behind a watchdog panic. + #[test] + fn isolated_child_failure_with_large_output_is_reported() { + let test = "provider::test_executable::tests::noisy_failure_child"; + let started = Instant::now(); + let panic = std::panic::catch_unwind(|| run_isolated(test, ISOLATED_HANG_GUARD)) + .expect_err("a failing isolated child must fail its parent"); + let message = panic.downcast_ref::().cloned().unwrap_or_default(); + assert!( + started.elapsed() < ISOLATED_HANG_GUARD, + "{:?}", + started.elapsed() + ); + assert!(!message.contains("watchdog"), "{message}"); + assert!( + message.starts_with(&format!("isolated test {test} failed with ")), + "{message}" + ); + assert!(message.contains(NOISY_FAILURE), "{message}"); + assert!(message.contains(NOISY_STDERR_END), "{message}"); + assert!(message.contains("earlier bytes omitted"), "{message}"); + assert!(message.len() < 3 * RETAINED_TAIL, "{}", message.len()); + } +} diff --git a/src/commands/config-adopt.ts b/src/commands/config-adopt.ts index ac739043..e18670dc 100644 --- a/src/commands/config-adopt.ts +++ b/src/commands/config-adopt.ts @@ -100,26 +100,107 @@ export const PENDING_SWITCH_BUSY_NEXT_STEP = "A publication switch is pending. If no other adoption command is running, run 'hack config adopt --recover' to complete it or 'hack config adopt --recover --rollback' to restore the originals."; export const PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP = "A publication switch is pending and the bound .hack/.internal/compose.env.override.yml holds literal env values, so completing it refuses. If no other adoption command is running, run 'hack config adopt --recover --rollback' to release it."; +/** + * The step for a switch whose rollback is journaled (`rolling-back`): plain + * `--recover` completes only a `switching` publication and refuses this one, + * whatever the override holds, so only `--recover --rollback` finishes it + * (HACK-1427). Fixed text; values omitted. + */ +export const PENDING_ROLLBACK_BUSY_NEXT_STEP = + "A rollback of the publication switch is pending. If no other adoption command is running, run 'hack config adopt --recover --rollback' to finish restoring the originals."; +/** + * The step for a `switching` publication whose bound env override was moved + * or changed since preparation: completing it refuses, because its saved read + * binds the override's bytes and identity (ctime included, so moving it back + * does not help), while rollback releases the override and restores the + * originals (HACK-1437). Fixed text; values omitted. + */ +export const PENDING_CHANGED_SWITCH_BUSY_NEXT_STEP = + "A publication switch is pending and its bound .hack/.internal/compose.env.override.yml was moved or changed since preparation, so completing it refuses. If no other adoption command is running, run 'hack config adopt --recover --rollback' to restore the originals."; + +/** + * Whether the pending switch's bound env override changed since preparation. + * A hint only: unreadable state answers false, which keeps the generic step. + */ +async function pendingSwitchEnvOverrideChanged(opts: { + readonly projectRoot: string; + readonly branch?: string; +}): Promise { + try { + const store = await openLegacyComposeAdoptedGenerationStore({ + projectRoot: opts.projectRoot, + requestedBranch: opts.branch, + mode: "saved", + }); + try { + return await store.pendingSwitchEnvOverrideChanged(); + } finally { + await store.close(); + } + } catch { + return false; + } +} -/** The store refusal's fixed message, plus the pending-switch step for BUSY. */ +/** + * The fixed step a pending switch accepts. Every step names only a command + * that state does not refuse for a known reason: plain `--recover` refuses a + * `rolling-back` switch, a bound literal override and a bound override that + * was moved or changed, so those name `--recover --rollback` alone. With + * `busy` false (a refused completion) only those known steps are returned. + */ +async function pendingSwitchNextStep(opts: { + readonly projectRoot: string; + readonly branch?: string; + readonly busy: boolean; +}): Promise { + const detail = await inspectLegacyComposeAdoptionSelectionDetail({ + projectRoot: opts.projectRoot, + }); + if (detail?.pending !== "switch") { + return null; + } + if (detail.switchPhase === "rolling-back") { + return PENDING_ROLLBACK_BUSY_NEXT_STEP; + } + if ( + await legacyAdoptionEnvOverrideHoldsValues({ + projectRoot: opts.projectRoot, + }) + ) { + // A refused completion's own literal-env message already names it. + return opts.busy ? PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP : null; + } + if (await pendingSwitchEnvOverrideChanged(opts)) { + return PENDING_CHANGED_SWITCH_BUSY_NEXT_STEP; + } + return opts.busy ? PENDING_SWITCH_BUSY_NEXT_STEP : null; +} + +/** + * The store refusal's fixed message, plus the step a pending switch accepts: + * for BUSY, and for a refused plain `--recover` whose cause is known. + */ async function adoptionRefusalMessage(opts: { readonly error: LegacyComposeAdoptedGenerationError; readonly projectRoot: string; + readonly branch?: string; + readonly completing: boolean; }): Promise { - if (opts.error.code !== "E_LEGACY_ADOPTION_BUSY") { + const busy = opts.error.code === "E_LEGACY_ADOPTION_BUSY"; + if ( + !(busy || opts.completing) || + opts.error.code === "E_LEGACY_ADOPTION_LITERAL_ENV" + ) { return opts.error.message; } try { - const detail = await inspectLegacyComposeAdoptionSelectionDetail({ - projectRoot: opts.projectRoot, - }); - if (detail?.pending !== "switch") { - return opts.error.message; - } - const literal = await legacyAdoptionEnvOverrideHoldsValues({ + const step = await pendingSwitchNextStep({ projectRoot: opts.projectRoot, + branch: opts.branch, + busy, }); - return `${opts.error.message} ${literal ? PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP : PENDING_SWITCH_BUSY_NEXT_STEP}`; + return step ? `${opts.error.message} ${step}` : opts.error.message; } catch { // Unreadable state keeps the store's fixed message alone. return opts.error.message; @@ -337,7 +418,16 @@ export const configAdoptCommand = withHandler(spec, async ({ ctx, args }) => { code: "E_CONFIG_INVALID", message: error instanceof LegacyComposeAdoptedGenerationError - ? await adoptionRefusalMessage({ error, projectRoot }) + ? await adoptionRefusalMessage({ + error, + projectRoot, + branch: args.options.branch, + completing: Boolean( + args.options.recover && + !args.options.rollback && + !args.options.stop + ), + }) : "Legacy adoption refused; original data and recovery evidence retained. Values omitted.", ...(diagnostic ? { detail: { legacy_adoption_publication_refusal: diagnostic } } diff --git a/src/lib/native-compose-adoption-generation.ts b/src/lib/native-compose-adoption-generation.ts index 8cb9eda4..62156034 100644 --- a/src/lib/native-compose-adoption-generation.ts +++ b/src/lib/native-compose-adoption-generation.ts @@ -50,7 +50,10 @@ import { planLegacyComposeRetainedRoutingAdoption, planLegacyComposeSourceBindAdoption, } from "./native-compose-adoption-plan.ts"; -import { readSavedLegacyComposeAdoptionProjection } from "./native-compose-adoption-projection.ts"; +import { + readSavedLegacyComposeAdoptionProjection, + savedLegacyComposeEnvOverrideChanged, +} from "./native-compose-adoption-projection.ts"; import { type LegacyComposePublicationRefusal, legacyComposePublicationRefusal, @@ -604,6 +607,13 @@ export type LegacyComposeAdoptedGenerationStore = { readonly binary?: string; }) => Promise; readonly rollback: () => Promise; + /** + * Read-only, lock-free hint for a refusal message: true when a `switching` + * publication's bound env override was moved or changed since preparation + * while its other generated inputs still match, so completing the switch + * refuses and only `--recover --rollback` releases it (HACK-1437). + */ + readonly pendingSwitchEnvOverrideChanged: () => Promise; /** Explicit repair after lock recovery; incomplete source/resource ownership refuses rather than overwriting edits. */ readonly repairPublication: (opts: { readonly action: "complete" | "rollback"; @@ -1050,11 +1060,17 @@ function selectedPlanner(routing: boolean, basic: boolean) { } return planLegacyComposeAdoption; } +/** + * `envOverride: "released"` is rollback's read only: rollback never reads, + * executes, rewrites or removes the generated env override, so it neither + * reads nor requires it; every other saved proof stays exact (HACK-1437). + */ async function readInputs( ctx: Context, selected: Anchor, preparing = false, - material: "verify" | "saved" = "verify" + material: "verify" | "saved" = "verify", + envOverride: "bound" | "released" = "bound" ): Promise<{ readonly manifest: Manifest; readonly literalEnvOverride: boolean; @@ -1168,6 +1184,7 @@ async function readInputs( proof: meta.projectionProof, signal: ctx.signal, checkOwner: ctx.check, + envOverride, }; const projection = (meta.adoption_generation_version === 5 && @@ -2351,7 +2368,13 @@ async function completeRollback(ctx: Context, state: Receipt) { } const routingDeadline = Date.now() + Math.min(ctx.timeoutMs ?? 15_000, 60_000); - const loaded = await readInputs(ctx, publication.generation, false, "saved"); + const loaded = await readInputs( + ctx, + publication.generation, + false, + "saved", + "released" + ); await requireStopped(ctx, loaded.inputs.binding); const held = await holdDirectory( join(ctx.generationsRoot, publication.generation.id, "originals"), @@ -2403,7 +2426,13 @@ async function completeRollback(ctx: Context, state: Receipt) { if (!(await absent(join(transaction.root, ".hack/hack.project.json")))) { refuse(); } - await readInputs(transaction, publication.generation, false, "saved"); + await readInputs( + transaction, + publication.generation, + false, + "saved", + "released" + ); await requireStopped(transaction, loaded.inputs.binding); await recheckDirectories([held]); if (loaded.inputs.retainedRouting) { @@ -3550,7 +3579,8 @@ export async function openLegacyComposeAdoptedGenerationStore(input: { ctx, state.publication.generation, false, - "saved" + "saved", + "released" ); await requireActiveCandidate(ctx, state.publication, loaded.inputs); await requireStopped(ctx, loaded.inputs.binding); @@ -3565,6 +3595,35 @@ export async function openLegacyComposeAdoptedGenerationStore(input: { translate(error, signal); } }, + async pendingSwitchEnvOverrideChanged() { + try { + const state = await publicationState(ctx); + if (state.publication?.phase !== "switching") { + return false; + } + const anchor = state.publication.generation; + const saved = await json( + join(ctx.generationsRoot, anchor.id, "manifest.json") + ); + if ( + !sameFile(saved.info, anchor.manifest) || + hash(saved.text) !== anchor.manifest.hash + ) { + refuse(); + } + const meta = manifest(saved.value, ctx.root, anchor.id); + if (meta.projectionProof === undefined) { + return false; + } + return await savedLegacyComposeEnvOverrideChanged({ + projectRoot: ctx.root, + proof: meta.projectionProof, + signal: ctx.signal, + }); + } catch (error: unknown) { + translate(error, signal); + } + }, async repairPublication(opts) { try { const action = opts.action, diff --git a/src/lib/native-compose-adoption-marker.ts b/src/lib/native-compose-adoption-marker.ts index 4f5fd1ed..ca8f8399 100644 --- a/src/lib/native-compose-adoption-marker.ts +++ b/src/lib/native-compose-adoption-marker.ts @@ -93,10 +93,13 @@ function refuse(): never { /** * What a `pending` selection waits on, which decides the only command that * moves it on: - * - `switch`: a journaled publication switch or rollback. `hack config adopt - * --recover` completes it and `--recover --rollback` restores the originals. + * - `switch`: a journaled publication switch or rollback. + * `hack config adopt --recover --rollback` restores the originals from + * either phase; plain `--recover` completes only a `switching` one, and + * refuses once the switch is `rolling-back` (see `switchPhase`). * - `preparation-stop`: an interrupted `hack config adopt --stop` before - * publication. Only `hack config adopt --recover --stop` retries it. + * publication, or before a new publication over a rolled-back one. Only + * `hack config adopt --recover --stop` retries it. * - `retained-operation`: an interrupted retained-container command of an * active adoption. Only `hack down --recover` settles it. * A receipt never holds a pending operation and a pending switch together. @@ -111,6 +114,11 @@ export type LegacyComposeAdoptionSelectionDetail = { readonly status: LegacyComposeAdoptionSelection; /** Null unless `status` is `pending`. */ readonly pending: LegacyComposeAdoptionPending | null; + /** + * The journaled phase of a pending `switch`, which decides whether plain + * `--recover` can still complete it. Null unless `pending` is `switch`. + */ + readonly switchPhase: "switching" | "rolling-back" | null; }; function classify( @@ -119,27 +127,34 @@ function classify( ): LegacyComposeAdoptionSelectionDetail { const state = parseLegacyComposeAdoptionReceipt(value, checkout); if (state.pendingOperation !== null) { - // The receipt parser admits a pending operation only over an active - // publication (a retained command) or before one (a preparation stop). + // The receipt parser (`pendingSelectionMatches`) admits a pending + // operation only over an active publication (a retained command), or as + // a stop of the prepared generation either before any publication or + // over a rolled-back one (a preparation stop before the next switch). return { status: "pending", pending: state.publication?.phase === "active" ? "retained-operation" : "preparation-stop", + switchPhase: null, }; } if (state.publication === null) { - return { status: "prepared", pending: null }; + return { status: "prepared", pending: null, switchPhase: null }; } switch (state.publication.phase) { case "switching": case "rolling-back": - return { status: "pending", pending: "switch" }; + return { + status: "pending", + pending: "switch", + switchPhase: state.publication.phase, + }; case "active": - return { status: "active", pending: null }; + return { status: "active", pending: null, switchPhase: null }; case "rolled-back": - return { status: "rolled-back", pending: null }; + return { status: "rolled-back", pending: null, switchPhase: null }; default: refuse(); } diff --git a/src/lib/native-compose-adoption-projection.ts b/src/lib/native-compose-adoption-projection.ts index 8d349eba..19cb5a66 100644 --- a/src/lib/native-compose-adoption-projection.ts +++ b/src/lib/native-compose-adoption-projection.ts @@ -65,6 +65,8 @@ const FILES = [ "compose.runtime.override.yml", "compose.env.override.yml", ] as const; +const ENV_OVERRIDE_FILE = "compose.env.override.yml"; +const ENV_OVERRIDE_INDEX = FILES.indexOf(ENV_OVERRIDE_FILE); const CONSTRUCTION_TOKEN = Symbol("legacy-adoption-projection"); const ownedProjections = new WeakSet(); type Source = Extract; @@ -152,6 +154,37 @@ function generatedProof(generated: Generated) { }), }; } +/** + * A saved generated proof with its env override entry released, for rollback + * only (HACK-1437). The directory identity and every other entry stay exact; + * `bound` records whether preparation bound an env override at all, which + * keeps its path in the containers' saved `config_files` list. + */ +function releasedEnvOverrideProof(generated: unknown) { + if ( + !( + isRecord(generated) && + Array.isArray(generated.files) && + generated.files.length === FILES.length + ) + ) { + refuse(); + } + const bound: unknown = generated.files[ENV_OVERRIDE_INDEX]; + if (!(bound === null || isRecord(bound))) { + refuse(); + } + return { + proof: { + ...generated, + files: generated.files.map((file: unknown, index) => + index === ENV_OVERRIDE_INDEX ? null : file + ), + }, + bound: bound !== null, + files: generated.files as readonly unknown[], + }; +} function privatePrimaryProof( provenance: Awaited< ReturnType @@ -255,10 +288,22 @@ export async function readSavedLegacyComposeAdoptionProjection(opts: { readonly proof: unknown; readonly signal?: AbortSignal; readonly checkOwner: () => Promise; + /** + * `bound` (the default) requires the generated env override's exact bytes + * and identity. `released` is rollback's read: the override is neither read + * nor required, because rollback never reads, executes, rewrites or removes + * it; every other proof stays exact (HACK-1437, see + * docs/reference/native-compose-adoption.md). + */ + readonly envOverride?: "bound" | "released"; }) { try { const { projectRoot, configText, composeText, signal, checkOwner } = opts; + const released = opts.envOverride === "released"; if ( + (opts.envOverride !== undefined && + opts.envOverride !== "bound" && + !released) || typeof projectRoot !== "string" || projectRoot.includes("\0") || resolve(projectRoot) !== projectRoot || @@ -412,10 +457,14 @@ export async function readSavedLegacyComposeAdoptionProjection(opts: { projectRoot, assertFresh: checkOwner, signal, + ...(released ? { releaseEnvOverride: true as const } : {}), }); + const releasedEnv = released + ? releasedEnvOverrideProof(proof.generated) + : undefined; if ( JSON.stringify(generatedProof(generated)) !== - JSON.stringify(proof.generated) + JSON.stringify(releasedEnv ? releasedEnv.proof : proof.generated) ) { refuse(); } @@ -436,7 +485,7 @@ export async function readSavedLegacyComposeAdoptionProjection(opts: { // current writer, or an earlier version's literal values. A literal form // holds a value (resolved values never contain `$`), so it never // classifies as references. Private and value-free (HACK-1318). - const envText = generated.files[1]?.text; + const envText = generated.files[ENV_OVERRIDE_INDEX]?.text; const literalEnvOverride = envText !== undefined && classifyComposeEnvOverrideText(envText).kind !== "references"; @@ -447,9 +496,18 @@ export async function readSavedLegacyComposeAdoptionProjection(opts: { ? { routingResolution: proof.routingResolution } : {}), metadata, + // A released override keeps the saved path list that the containers' + // `config_files` label records, without reading the file. composeFiles: [ join(projectRoot, ".hack/docker-compose.yml"), - ...generated.files.flatMap((file) => (file ? [file.path] : [])), + ...generated.files.flatMap((file, index) => { + if (file) { + return [file.path]; + } + return index === ENV_OVERRIDE_INDEX && releasedEnv?.bound + ? [join(projectRoot, ".hack/.internal", ENV_OVERRIDE_FILE)] + : []; + }), ], }; await checkOwner(); @@ -470,6 +528,8 @@ async function acquireGenerated(opts: { readonly signal?: AbortSignal; }) => Promise; readonly signal?: AbortSignal; + /** Leaves the env override unread and unobserved (a null entry). */ + readonly releaseEnvOverride?: true; }): Promise { check(opts.signal); await opts.assertFresh(opts); @@ -488,8 +548,12 @@ async function acquireGenerated(opts: { const directory = await holdDirectory(path, false); try { const files: (GeneratedFile | null)[] = []; - for (const filename of FILES) { + for (const [index, filename] of FILES.entries()) { check(opts.signal); + if (opts.releaseEnvOverride && index === ENV_OVERRIDE_INDEX) { + files.push(null); + continue; + } const selected = join(path, filename); const info = await lstat(selected).catch((error: unknown) => { if (hasCode(error, "ENOENT")) { @@ -529,6 +593,56 @@ async function acquireGenerated(opts: { } } +/** + * Whether the generated env override no longer matches a saved projection + * proof while its other generated inputs still do. Completing the pending + * switch then refuses (its saved read binds the override's bytes and + * identity, ctime included), and only rollback can release it (HACK-1437). + * Read-only and value-free; refuses when another generated input changed, + * since neither answer then names a command known to work. + */ +export async function savedLegacyComposeEnvOverrideChanged(opts: { + readonly projectRoot: string; + readonly proof: unknown; + readonly signal?: AbortSignal; +}): Promise { + try { + const saved = releasedEnvOverrideProof( + savedProjectionEnvelope(opts.proof).generated + ); + const fresh = (): Promise => { + check(opts.signal); + return Promise.resolve(); + }; + const others = await acquireGenerated({ + projectRoot: opts.projectRoot, + assertFresh: fresh, + signal: opts.signal, + releaseEnvOverride: true, + }); + if ( + JSON.stringify(generatedProof(others)) !== JSON.stringify(saved.proof) + ) { + refuse(); + } + const current = await acquireGenerated({ + projectRoot: opts.projectRoot, + assertFresh: fresh, + signal: opts.signal, + }).catch(() => null); + if (current === null) { + // The override is no longer a private regular file it can read. + return true; + } + return ( + JSON.stringify(generatedProof(current).files[ENV_OVERRIDE_INDEX]) !== + JSON.stringify(saved.files[ENV_OVERRIDE_INDEX]) + ); + } catch (error: unknown) { + redact(error); + } +} + /** Alternate generated authority remains outside this ordered branch-free slice. */ export async function hasLegacyComposeGeneratedSources( root: string, diff --git a/tests/native-compose-adoption-generation.test.ts b/tests/native-compose-adoption-generation.test.ts index bdad6963..80dc6c98 100644 --- a/tests/native-compose-adoption-generation.test.ts +++ b/tests/native-compose-adoption-generation.test.ts @@ -21,7 +21,11 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP } from "../src/commands/config-adopt.ts"; +import { + PENDING_CHANGED_SWITCH_BUSY_NEXT_STEP, + PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP, + PENDING_ROLLBACK_BUSY_NEXT_STEP, +} from "../src/commands/config-adopt.ts"; import { checkComposeEnvOverrideAtRest } from "../src/commands/doctor.ts"; import { resolveModernComposeEnvOverrides, @@ -75,6 +79,12 @@ import { privateCliEnv } from "./helpers/private-cli-env.ts"; const test = (name: string, run: () => Promise) => boundedTest(name, run, 20_000); const CANARY = "synthetic-private-adoption-canary"; +/** + * Budget for a rollback-recovery test that runs several saved adoption reads + * or CLI adoption commands; adoption tests are load-sensitive on shared hosts + * (HACK-1367). + */ +const ROLLBACK_RECOVERY_BUDGET_MS = 240_000; const ID = "a".repeat(64); const NETWORK = "b".repeat(64); const VOLUME = "original_private_database"; @@ -2080,6 +2090,100 @@ test("v3 saved adoption consumes canonical original generated sources without en } }); +/** A published v3 adoption over reference-form generated overrides. */ +async function publishedManagedAdoption() { + Reflect.deleteProperty(process.env, "CI"); + Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); + await trackedManagedCheckout(); + const files = await managedGenerated(); + await writeFile(join(projectRoot, ".hack.secret.key"), "synthetic-v3-key"); + await setProjectEnvValue({ + projectRoot, + projectDir: join(projectRoot, ".hack"), + envName: null, + scope: "global", + key: "MANAGED", + value: CANARY, + secret: true, + }); + const binary = await managedEnvCompilerFixture( + join(root, "managed-compiler") + ); + const store = await openLegacyComposeAdoptedGenerationStore({ projectRoot }); + try { + const generation = await store.prepare({ binary }); + await store.publish({ generation, binary }); + } finally { + await store.close(); + } + return files; +} + +boundedTest( + "rollback releases a moved-aside generated env override that execution still refuses (HACK-1437)", + async () => { + const files = await publishedManagedAdoption(); + const override = files[2] ?? ""; + const envText = await readFile(override, "utf8"); + const aside = join(root, "moved-aside-override.yml"); + await rename(override, aside); + const saved = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + mode: "saved", + }); + try { + // Execution keeps the exact binding: the adoption no longer loads. + await refusal(saved.loadActive()); + expect((await readReceipt()).publication.phase).toBe("active"); + // Rollback never reads or writes the override, so it is released. + await saved.rollback(); + } finally { + await saved.close(); + } + expect(await inspectLegacyComposeAdoptionSelection({ projectRoot })).toBe( + "rolled-back" + ); + expect(await readFile(files[0] ?? "", "utf8")).toContain("name: fixture"); + expect(await lstat(override).catch(() => null)).toBeNull(); + expect(await readFile(aside, "utf8")).toBe(envText); + }, + ROLLBACK_RECOVERY_BUDGET_MS +); + +boundedTest( + "rollback still binds the generated runtime override when the env override is released (HACK-1437)", + async () => { + const files = await publishedManagedAdoption(); + // Releasing the env override must not release its sibling: a runtime + // override rewritten with its exact bytes (new mtime and ctime, so only + // the identity proof can notice) still refuses rollback before anything + // moves. + await rename(files[2] ?? "", join(root, "moved-aside-override.yml")); + const runtimeText = await readFile(files[1] ?? "", "utf8"); + const runtimeBefore = await lstat(files[1] ?? ""); + await Bun.sleep(5); + await writeFile(files[1] ?? "", runtimeText); + expect(await readFile(files[1] ?? "", "utf8")).toBe(runtimeText); + expect((await lstat(files[1] ?? "")).ctimeMs).not.toBe( + runtimeBefore.ctimeMs + ); + const saved = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + mode: "saved", + }); + try { + await refusal(saved.rollback()); + } finally { + await saved.close(); + } + expect((await readReceipt()).publication.phase).toBe("active"); + expect( + await Bun.file(join(projectRoot, ".hack/hack.project.json")).exists() + ).toBe(true); + }, + ROLLBACK_RECOVERY_BUDGET_MS +); + test("an adoption bound to an earlier version's literal env override keeps it loadable until rollback frees it (HACK-1273)", async () => { Reflect.deleteProperty(process.env, "CI"); Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); @@ -2200,6 +2304,44 @@ async function adoptableManagedFixture(form: "literal" | "reference") { ); return { override: files[2] ?? "", binary }; } +/** Each command a fixed recovery step names, in order. */ +function namedAdoptCommands(message: string): string[][] { + return [...message.matchAll(/'hack config adopt([^']*)'/g)].map((match) => + (match[1] ?? "").split(" ").filter(Boolean) + ); +} +/** + * Runs the only command a pending-switch step names and asserts it finishes + * the rollback: originals restored, the selection rolled back, and the + * moved or restored override neither read into output nor rewritten. + */ +async function runNamedRecoveryStep(opts: { + readonly message: string; + readonly binary: string; + readonly original: Awaited>; + readonly moved: { + readonly path: string; + readonly text: string; + readonly ino: number; + }; + readonly override: string; +}) { + const named = namedAdoptCommands(opts.message); + expect(named).toEqual([["--recover", "--rollback"]]); + const released = await adoptCli([...(named[0] ?? []), "--json"], opts.binary); + expect(released.code).toBe(0); + expect(JSON.parse(released.stdout)).toMatchObject({ status: "rolled-back" }); + expect(released.output).not.toContain(CANARY); + await expectOriginals(opts.original); + expect(await inspectLegacyComposeAdoptionSelection({ projectRoot })).toBe( + "rolled-back" + ); + expect(await readFile(opts.moved.path, "utf8")).toBe(opts.moved.text); + expect((await lstat(opts.moved.path)).ino).toBe(opts.moved.ino); + if (opts.moved.path !== opts.override) { + expect(await lstat(opts.override).catch(() => null)).toBeNull(); + } +} async function adoptCli(flags: readonly string[], binary: string) { const child = Bun.spawn( [ @@ -2439,6 +2581,254 @@ boundedTest( 90_000 ); +boundedTest( + "CLI busy refusal over an interrupted rollback names only the recovery rollback that finishes it (HACK-1427)", + async () => { + // A reference-form override, so the busy step cannot come from the + // literal-override branch. + const { override, binary } = await adoptableManagedFixture("reference"); + const original = await originalSnapshots(); + const adopted = await adoptCli(["--json"], binary); + expect(adopted.code).toBe(0); + const active = await readReceipt(); + expect(active.publication).toMatchObject({ phase: "active" }); + const pinned = await lstat(override); + // An interrupted rollback: `rollback()` journals `rolling-back` and is + // stopped before it restores the originals. + const rollingBack = { + ...active, + publication: { ...active.publication, phase: "rolling-back" }, + }; + await writeReceipt(rollingBack); + const busy = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(busy.code).not.toBe(0); + expect(JSON.parse(busy.stdout)).toEqual({ + ok: false, + error: { + code: "E_CONFIG_INVALID", + message: `${ + new LegacyComposeAdoptedGenerationError("E_LEGACY_ADOPTION_BUSY") + .message + } ${PENDING_ROLLBACK_BUSY_NEXT_STEP}`, + }, + }); + expect(busy.output).not.toContain("'hack config adopt --recover' to"); + expect(busy.output).not.toContain(CANARY); + expect(await readReceipt()).toEqual(rollingBack); + // Plain `--recover`, which the generic pending-switch step names first, + // refuses in this state and changes nothing. + const completed = await adoptCli(["--recover", "--json"], binary); + expect(completed.code).not.toBe(0); + expect(completed.output).not.toContain(CANARY); + expect(await readReceipt()).toEqual(rollingBack); + // The named step finishes the rollback. + const released = await adoptCli( + ["--recover", "--rollback", "--json"], + binary + ); + expect(released.code).toBe(0); + expect(JSON.parse(released.stdout)).toMatchObject({ + status: "rolled-back", + }); + await expectOriginals(original); + expect((await lstat(override)).ino).toBe(pinned.ino); + expect(await inspectLegacyComposeAdoptionSelection({ projectRoot })).toBe( + "rolled-back" + ); + }, + 90_000 +); + +boundedTest( + "CLI busy refusal over an interrupted rollback of a literal-bound switch does not name plain recovery once the override is moved aside, and that rollback finishes (HACK-1427, HACK-1437)", + async () => { + // The reported state: the HACK-1318 fixture after its switch is + // journaled, rewritten to an interrupted rollback, with the bound literal + // override moved aside so the busy step no longer comes from the + // literal-override branch. + const { override, binary } = await adoptableManagedFixture("literal"); + const original = await originalSnapshots(); + const store = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + }); + try { + const generation = await store.prepare({ binary }); + expect( + await store.withPreparationStop({ + generation, + binary, + run: async () => 7, + }) + ).toBe(7); + } finally { + await store.close(); + } + const journaled = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(journaled.code).not.toBe(0); + const switching = await readReceipt(); + expect(switching.publication).toMatchObject({ phase: "switching" }); + const rollingBack = { + ...switching, + publication: { ...switching.publication, phase: "rolling-back" }, + }; + await writeReceipt(rollingBack); + const literalText = await readFile(override, "utf8"); + const literalInfo = await lstat(override); + const aside = join(root, "moved-aside-override.yml"); + await rename(override, aside); + for (const flags of [ + ["--recover", "--stop", "--json"], + ["--json"], + ["--stop", "--json"], + ]) { + const busy = await adoptCli(flags, binary); + expect(busy.code).not.toBe(0); + expect(JSON.parse(busy.stdout)).toEqual({ + ok: false, + error: { + code: "E_CONFIG_INVALID", + message: `${ + new LegacyComposeAdoptedGenerationError("E_LEGACY_ADOPTION_BUSY") + .message + } ${PENDING_ROLLBACK_BUSY_NEXT_STEP}`, + }, + }); + expect(busy.output).not.toContain("'hack config adopt --recover' to"); + expect(busy.output).not.toContain(CANARY); + expect(await readReceipt()).toEqual(rollingBack); + } + // The step the generic message named first refuses here, and its + // refusal names the step that state accepts (HACK-1437). + const completed = await adoptCli(["--recover", "--json"], binary); + expect(completed.code).not.toBe(0); + expect(JSON.parse(completed.stdout).error.message).toEndWith( + ` ${PENDING_ROLLBACK_BUSY_NEXT_STEP}` + ); + expect(completed.output).not.toContain(CANARY); + expect(await readReceipt()).toEqual(rollingBack); + // The named step finishes the rollback with the override still moved + // aside: rollback releases it instead of looping (HACK-1437). + await runNamedRecoveryStep({ + message: PENDING_ROLLBACK_BUSY_NEXT_STEP, + binary, + original, + moved: { path: aside, text: literalText, ino: literalInfo.ino }, + override, + }); + }, + ROLLBACK_RECOVERY_BUDGET_MS +); + +boundedTest( + "CLI recovery rollback finishes a pending switch whose bound literal override was moved aside and back, without hand edits (HACK-1437)", + async () => { + // The reachable state: the HACK-1318 literal fixture with its switch + // journaled by the CLI itself, then the bound override moved aside. + const { override, binary } = await adoptableManagedFixture("literal"); + const original = await originalSnapshots(); + const store = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + }); + try { + const generation = await store.prepare({ binary }); + expect( + await store.withPreparationStop({ + generation, + binary, + run: async () => 7, + }) + ).toBe(7); + } finally { + await store.close(); + } + const journaled = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(journaled.code).not.toBe(0); + const switching = await readReceipt(); + expect(switching.publication).toMatchObject({ phase: "switching" }); + const literalText = await readFile(override, "utf8"); + const pinned = await lstat(override); + const aside = join(root, "moved-aside-override.yml"); + await rename(override, aside); + // Busy names only the step this state accepts: completing refuses, + // because the saved read binds the override's bytes and identity. + const busy = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(busy.code).not.toBe(0); + expect(JSON.parse(busy.stdout)).toEqual({ + ok: false, + error: { + code: "E_CONFIG_INVALID", + message: `${ + new LegacyComposeAdoptedGenerationError("E_LEGACY_ADOPTION_BUSY") + .message + } ${PENDING_CHANGED_SWITCH_BUSY_NEXT_STEP}`, + }, + }); + expect(busy.output).not.toContain("'hack config adopt --recover' to"); + expect(busy.output).not.toContain(CANARY); + // Plain completion refuses, journals nothing, and names the same step. + const completed = await adoptCli(["--recover", "--json"], binary); + expect(completed.code).not.toBe(0); + expect(JSON.parse(completed.stdout).error.message).toEndWith( + ` ${PENDING_CHANGED_SWITCH_BUSY_NEXT_STEP}` + ); + expect(completed.output).not.toContain(CANARY); + expect(await readReceipt()).toEqual(switching); + // Moving it back keeps its bytes and inode but not its ctime, which the + // saved read binds; rollback does not depend on it. The literal bytes + // are back, so the step comes from the literal branch. + await rename(aside, override); + const restored = await lstat(override); + expect(restored.ino).toBe(pinned.ino); + expect(restored.ctimeMs).not.toBe(pinned.ctimeMs); + const busyBack = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(JSON.parse(busyBack.stdout).error.message).toEndWith( + ` ${PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP}` + ); + await runNamedRecoveryStep({ + message: PENDING_LITERAL_SWITCH_BUSY_NEXT_STEP, + binary, + original, + moved: { path: override, text: literalText, ino: pinned.ino }, + override, + }); + // Nothing pins the literal file once the switch is rolled back. + expect( + await removePlaintextComposeEnvOverride({ + projectDir: join(projectRoot, ".hack"), + includeUnverifiable: true, + }) + ).toMatchObject({ removed: true, adoptionBound: false }); + }, + ROLLBACK_RECOVERY_BUDGET_MS +); + +boundedTest( + "CLI rollback of an active adoption releases a replaced generated env override while plain recovery stays refused (HACK-1437)", + async () => { + const { override, binary } = await adoptableManagedFixture("reference"); + const original = await originalSnapshots(); + const adopted = await adoptCli(["--json"], binary); + expect(adopted.code).toBe(0); + // A different value-free override at the bound path (a new inode). + const replaced = `${await readFile(override, "utf8")}# replaced\n`; + await rm(override); + await writeFile(override, replaced, { mode: 0o600 }); + const rolledBack = await adoptCli(["--rollback", "--json"], binary); + expect(rolledBack.code).toBe(0); + expect(JSON.parse(rolledBack.stdout)).toMatchObject({ + status: "rolled-back", + }); + expect(rolledBack.output).not.toContain(CANARY); + await expectOriginals(original); + // Rollback neither read nor rewrote the replaced file. + expect(await readFile(override, "utf8")).toBe(replaced); + expect(await inspectLegacyComposeAdoptionSelection({ projectRoot })).toBe( + "rolled-back" + ); + }, + ROLLBACK_RECOVERY_BUDGET_MS +); + test("v3 value-free env override binds the engine config hash computed with the values (HACK-1273)", async () => { Reflect.deleteProperty(process.env, "CI"); Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE");