From e650edc6282971777a9a763ae6e4199df60df885 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 01:22:13 -0400 Subject: [PATCH] fix(doctor): name explicit recovery rollback for a literal override a pending adoption pins After a refused `hack config adopt --recover --stop` (HACK-1318, journaled switch), `hack doctor` told the user to run `hack down` and plain `hack config adopt --rollback`, both of which that pending state refuses. The compose-env-at-rest report now carries `adoptionPending`, and the check and `--fix` note name `hack config adopt --recover --rollback` while the adoption is pending; the settled wording is unchanged. Deadline-guard follow-ups from #397 (HACK-1391): - prove the keep-CHANGED branch of the `mutateRetainedContainers` catch with a drift reported after the caller deadline (mutation: dropping the exclusion fails only this test) - prove `requireMutationDeadline`'s expiry branch directly through the callback revocation fence (mutation: issuing CHANGED fails only this test) - correct the retained-routing helper comment (deadline expiry is E_LEGACY_ADOPTION_DEADLINE) and the HACK-1283 comment (HACK-1340) - document that a second `--recover --stop` after the refusal gets the generic E_LEGACY_ADOPTION_BUSY refusal, and assert it in the HACK-1318 test Refs HACK-1391 Co-Authored-By: Claude Fable 5.1 --- docs/reference/native-compose-adoption.md | 8 ++ src/commands/doctor.ts | 4 +- src/lib/compose-env-at-rest.ts | 61 +++++++++++-- tests/compose-env-at-rest.test.ts | 55 ++++++++++++ tests/helpers/retained-routing-adoption.ts | 11 +-- ...native-compose-adoption-generation.test.ts | 40 +++++++++ ...ompose-adoption-routing-generation.test.ts | 85 ++++++++++++++++++- 7 files changed, 247 insertions(+), 17 deletions(-) diff --git a/docs/reference/native-compose-adoption.md b/docs/reference/native-compose-adoption.md index 72ff6d64..88b35345 100644 --- a/docs/reference/native-compose-adoption.md +++ b/docs/reference/native-compose-adoption.md @@ -531,6 +531,14 @@ publication and `--recover` switch completion refuse at the `publication-inputs` boundary with the closed `literal-env-override` reason, before any original moves. The switch stays journaled and the fixed message names `hack config adopt --recover --rollback`; rollback stays allowed (HACK-1318). +A repeated `--recover --stop` after that refusal does not reach the publication +boundary again: the switch is already journaled, so the retried preparation stop +refuses it as a pending publication with the generic `E_LEGACY_ADOPTION_BUSY` +code ("busy or requires explicit interrupted-lock recovery") and no publication +diagnostic; the literal-env message appears only on the first refusal. +`hack doctor` reports the pinned literal override and names +`hack config adopt --recover --rollback` as the next step while the switch is +pending, since `hack down` and plain `--rollback` refuse in that state (HACK-1391). Unmodified v4 clients cannot be universally fenced: they ignore future legacy versions and may select ancestor Compose inputs. Adoption requires the upgraded diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 03574297..f10d99e8 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -61,7 +61,7 @@ import { } from "../lib/caddy-hosts.ts"; import { emitCliResult, okResult } from "../lib/cli-result.ts"; import { - ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP, + adoptedComposeEnvOverrideNextStep, composeEnvAtRestRepairs, describeComposeEnvAtRest, inspectComposeEnvAtRest, @@ -4186,7 +4186,7 @@ async function maybeRemovePlaintextComposeEnvOverride(opts: { const repairs = composeEnvAtRestRepairs(report); if (report.adoptionBound) { note( - `Kept ${file}: a saved 'hack config adopt' generation pins it, and removing it would break the adoption (values omitted). ${ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP}`, + `Kept ${file}: a saved 'hack config adopt' generation pins it, and removing it would break the adoption (values omitted). ${adoptedComposeEnvOverrideNextStep(report)}`, "compose env at rest" ); } diff --git a/src/lib/compose-env-at-rest.ts b/src/lib/compose-env-at-rest.ts index 4ae75a2c..78ab53ae 100644 --- a/src/lib/compose-env-at-rest.ts +++ b/src/lib/compose-env-at-rest.ts @@ -38,8 +38,10 @@ import { * and inode in its integrity proof, and adopted commands never write it. While * such a generation exists (prepared, active or pending), nothing here removes * the file, because removal would make the adoption unloadable; the supported - * way to drop its values is rollback, `hack up`, then adopting again. New - * adoptions refuse a literal override (`hack config adopt`). + * way to drop its values is rollback (`hack config adopt --rollback`, or + * `--recover --rollback` while an adoption operation or switch is pending), + * `hack up`, then adopting again. New adoptions refuse a literal override + * (`hack config adopt`). */ export const COMPOSE_ENV_OVERRIDE_FILENAME = "compose.env.override.yml"; @@ -331,17 +333,34 @@ export async function removePlaintextComposeEnvOverride(opts: { export async function composeEnvOverrideAdoptionBound(opts: { readonly projectDir: string; }): Promise { + return (await composeEnvOverrideAdoptionBinding(opts)).bound; +} + +/** + * How a saved adoption binds the override: `bound` as in + * {@link composeEnvOverrideAdoptionBound}, and `pending` when the binding + * adoption has a pending or interrupted operation or switch. A pending + * adoption fences `hack down` and plain `hack config adopt --rollback`, so + * only explicit recovery (`hack config adopt --recover --rollback`) releases + * the file (HACK-1318). Unreadable adoption state is bound and not pending. + */ +export async function composeEnvOverrideAdoptionBinding(opts: { + readonly projectDir: string; +}): Promise<{ readonly bound: boolean; readonly pending: boolean }> { const projectDir = resolve(opts.projectDir); if (basename(projectDir) !== HACK_PROJECT_DIR_PRIMARY) { - return false; + return { bound: false, pending: false }; } try { const status = await inspectLegacyComposeAdoptionSelection({ projectRoot: dirname(projectDir), }); - return status !== null && status !== "rolled-back"; + return { + bound: status !== null && status !== "rolled-back", + pending: status === "pending", + }; } catch { - return true; + return { bound: true, pending: false }; } } @@ -451,6 +470,10 @@ export type ComposeEnvAtRestReport = { readonly override: ComposeEnvOverrideAtRest; /** A saved adoption may pin the override; see {@link composeEnvOverrideAdoptionBound}. */ readonly adoptionBound: boolean; + /** The pinning adoption has a pending or interrupted operation or switch, + * so only `hack config adopt --recover --rollback` releases the file; see + * {@link composeEnvOverrideAdoptionBinding}. False when not bound. */ + readonly adoptionPending: boolean; /** Asides left by interrupted removals; each may hold literal values. */ readonly asides: { readonly stale: number; readonly fresh: number }; }; @@ -472,11 +495,13 @@ export async function inspectComposeEnvAtRest(opts: { now: opts.now ?? Date.now, }) : { stale: [], fresh: [] }; + const binding = composeEnvOverrideMayHoldValues(override) + ? await composeEnvOverrideAdoptionBinding(opts) + : { bound: false, pending: false }; return { override, - adoptionBound: - composeEnvOverrideMayHoldValues(override) && - (await composeEnvOverrideAdoptionBound(opts)), + adoptionBound: binding.bound, + adoptionPending: binding.bound && binding.pending, asides: { stale: asides.stale.length, fresh: asides.fresh.length }, }; } @@ -506,6 +531,22 @@ export function composeEnvAtRestRepairs(report: ComposeEnvAtRestReport): { /** Supported way to drop values that a saved adoption pins; see the module doc. */ export const ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP = "To drop them, stop the adopted containers ('hack down'), run 'hack config adopt --rollback', run 'hack up' so Hack rewrites the file with references only, then adopt again with 'hack config adopt --stop'."; +/** + * The same, while the pinning adoption has a pending or interrupted operation + * or switch: `hack down` and plain `--rollback` refuse in that state, and only + * explicit recovery releases the switch (HACK-1318, HACK-1391). + */ +export const PENDING_ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP = + "To drop them, release the pending adoption switch with 'hack config adopt --recover --rollback' (plain rollback and 'hack down' refuse while it is pending), run 'hack up' so Hack rewrites the file with references only, then adopt again with 'hack config adopt --stop'."; + +/** The next step for a report whose override a saved adoption pins. */ +export function adoptedComposeEnvOverrideNextStep( + report: Pick +): string { + return report.adoptionPending + ? PENDING_ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP + : ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP; +} /** * Doctor status and message. Names the project-relative path and counts only; @@ -521,6 +562,7 @@ export function describeComposeEnvAtRest(opts: { file, projectDirName: opts.projectDirName, adoptionBound: opts.report.adoptionBound, + adoptionNextStep: adoptedComposeEnvOverrideNextStep(opts.report), }); const asides = opts.report.asides.stale + opts.report.asides.fresh; if (asides === 0) { @@ -539,10 +581,11 @@ function describeOverride(opts: { readonly file: string; readonly projectDirName: string; readonly adoptionBound: boolean; + readonly adoptionNextStep: string; }): { readonly status: "ok" | "warn"; readonly message: string } { const { state, file } = opts; const next = opts.adoptionBound - ? `A saved 'hack config adopt' generation pins this file, so 'hack doctor --fix' keeps it: removing it would break the adoption. ${ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP}` + ? `A saved 'hack config adopt' generation pins this file, so 'hack doctor --fix' keeps it: removing it would break the adoption. ${opts.adoptionNextStep}` : "Run 'hack doctor --fix' to remove it; Compose commands now write references only."; switch (state.kind) { case "absent": diff --git a/tests/compose-env-at-rest.test.ts b/tests/compose-env-at-rest.test.ts index 1e5e9154..65c60c56 100644 --- a/tests/compose-env-at-rest.test.ts +++ b/tests/compose-env-at-rest.test.ts @@ -16,11 +16,14 @@ import { join } from "node:path"; import { YAML } from "bun"; import { buildDoctorRemediationPlanLines } from "../src/commands/doctor.ts"; import { + ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP, + adoptedComposeEnvOverrideNextStep, composeEnvAtRestRepairs, composeEnvOverridePath, describeComposeEnvAtRest, inspectComposeEnvAtRest, inspectComposeEnvOverrideAtRest, + PENDING_ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP, removePlaintextComposeEnvOverride, writeComposeEnvOverride, } from "../src/lib/compose-env-at-rest.ts"; @@ -372,6 +375,7 @@ test("doctor descriptions are ok for absent and reference-only files", () => { report: { override, adoptionBound: false, + adoptionPending: false, asides: { stale: 0, fresh: 0 }, }, projectDirName: ".hack", @@ -481,6 +485,9 @@ test("an override that an adoption state may pin is kept by removal and reported expect(await readFile(path, "utf8")).toBe(literalText()); const report = await inspectComposeEnvAtRest({ projectDir }); expect(report.adoptionBound).toBe(true); + // Unreadable state is bound but not known to be pending, so the next step + // stays the plain rollback. + expect(report.adoptionPending).toBe(false); expect(composeEnvAtRestRepairs(report).override).toBe(false); const described = describeComposeEnvAtRest({ report, @@ -489,6 +496,7 @@ test("an override that an adoption state may pin is kept by removal and reported expect(described.status).toBe("warn"); expect(described.message).toContain("pins this file"); expect(described.message).toContain("hack config adopt --rollback"); + expect(described.message).not.toContain("--recover --rollback"); expect(described.message).not.toContain(CANARY); // Without the adoption state the same file is removed. @@ -498,6 +506,52 @@ test("an override that an adoption state may pin is kept by removal and reported expect(await lstat(path).catch(() => null)).toBeNull(); }); +test("doctor names explicit recovery rollback for an override that a pending adoption pins (HACK-1391)", () => { + const path = "/unused"; + for (const kind of ["plaintext", "unverifiable"] as const) { + const override = + kind === "plaintext" + ? { kind, path, values: 2, services: 1 } + : { kind, path }; + const pending = describeComposeEnvAtRest({ + report: { + override, + adoptionBound: true, + adoptionPending: true, + asides: { stale: 0, fresh: 0 }, + }, + projectDirName: ".hack", + }); + expect(pending.status).toBe("warn"); + expect(pending.message).toContain("pins this file"); + expect(pending.message).toContain("hack config adopt --recover --rollback"); + // Plain rollback and `hack down` refuse while the adoption is pending, so + // neither is the next step. + expect(pending.message).not.toContain("'hack config adopt --rollback'"); + expect(pending.message).not.toContain("stop the adopted containers"); + expect(pending.message).toContain( + PENDING_ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP + ); + expect(adoptedComposeEnvOverrideNextStep({ adoptionPending: true })).toBe( + PENDING_ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP + ); + expect(adoptedComposeEnvOverrideNextStep({ adoptionPending: false })).toBe( + ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP + ); + const settled = describeComposeEnvAtRest({ + report: { + override, + adoptionBound: true, + adoptionPending: false, + asides: { stale: 0, fresh: 0 }, + }, + projectDirName: ".hack", + }); + expect(settled.message).toContain(ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP); + expect(settled.message).not.toContain("--recover --rollback"); + } +}); + /** Names and contents of every entry in a directory outside the project. */ async function snapshotDirectory( directory: string @@ -546,6 +600,7 @@ for (const linked of [".hack", ".hack/.internal"] as const) { expect(report).toEqual({ override: { kind: "refused", path }, adoptionBound: false, + adoptionPending: false, asides: { stale: 0, fresh: 0 }, }); expect(composeEnvAtRestRepairs(report)).toEqual({ diff --git a/tests/helpers/retained-routing-adoption.ts b/tests/helpers/retained-routing-adoption.ts index d8d63090..e2d20840 100644 --- a/tests/helpers/retained-routing-adoption.ts +++ b/tests/helpers/retained-routing-adoption.ts @@ -33,11 +33,12 @@ export type RetainedRoutingOperationDeadlineMs = | 15_000 | typeof DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS; /** Value-free timing of the most recent `operation()` call. A retained mutation - * refuses as `E_LEGACY_ADOPTION_CHANGED` both when its observed binding differs - * from the recorded one and when its caller deadline expires (the product's - * fail-closed deadline guard). `deadlineExpired` and `bindingUnchanged` (whether - * every synthetic binding fact except the running state still equals the - * fixture's original) tell the two apart without exposing any value. */ + * refuses as `E_LEGACY_ADOPTION_CHANGED` when its observed binding differs from + * the recorded one and as `E_LEGACY_ADOPTION_DEADLINE` when only its caller + * deadline expires (the product's fail-closed deadline guard, HACK-1340). + * `deadlineExpired` and `bindingUnchanged` (whether every synthetic binding fact + * except the running state still equals the fixture's original) tell the two + * apart without exposing any value. */ export interface RetainedRoutingOperationTiming { readonly operation: "start" | "stop"; readonly preparation: boolean; diff --git a/tests/native-compose-adoption-generation.test.ts b/tests/native-compose-adoption-generation.test.ts index a674fc7d..4504b1db 100644 --- a/tests/native-compose-adoption-generation.test.ts +++ b/tests/native-compose-adoption-generation.test.ts @@ -2336,6 +2336,46 @@ boundedTest( await expectOriginals(original); expect((await lstat(override)).ino).toBe(pinned.ino); } + // A second `--recover --stop` no longer reaches the publication boundary: + // the switch is already journaled, so the retried preparation stop refuses + // it as a pending publication (the generic busy code) without the + // literal-env message or a publication diagnostic (HACK-1391). + const repeated = await adoptCli(["--recover", "--stop", "--json"], binary); + expect(repeated.code).not.toBe(0); + expect(JSON.parse(repeated.stdout)).toEqual({ + ok: false, + error: { + code: "E_CONFIG_INVALID", + message: new LegacyComposeAdoptedGenerationError( + "E_LEGACY_ADOPTION_BUSY" + ).message, + }, + }); + expect(repeated.output).not.toContain(CANARY); + expect((await readReceipt()).publication).toMatchObject({ + phase: "switching", + native: null, + }); + // Doctor's next step for the pinned literal override must be a command + // this pending state accepts: explicit recovery rollback, not plain + // rollback or `hack down`, which it refuses (HACK-1391). + const report = await inspectComposeEnvAtRest({ + projectDir: join(projectRoot, ".hack"), + }); + expect(report).toMatchObject({ + override: { kind: "plaintext" }, + adoptionBound: true, + adoptionPending: true, + }); + const checked = await checkComposeEnvOverrideAtRest({ + startDir: projectRoot, + }); + expect(checked.status).toBe("warn"); + expect(checked.message).toContain("pins this file"); + expect(checked.message).toContain("hack config adopt --recover --rollback"); + expect(checked.message).not.toContain("'hack config adopt --rollback'"); + expect(checked.message).not.toContain("stop the adopted containers"); + expect(checked.message).not.toContain(CANARY); // The named step releases the switch; nothing pins the file afterwards. const released = await adoptCli( ["--recover", "--rollback", "--json"], diff --git a/tests/native-compose-adoption-routing-generation.test.ts b/tests/native-compose-adoption-routing-generation.test.ts index 96d48add..a0662811 100644 --- a/tests/native-compose-adoption-routing-generation.test.ts +++ b/tests/native-compose-adoption-routing-generation.test.ts @@ -508,7 +508,7 @@ test( code: "E_LEGACY_ADOPTION_DEADLINE", deadlineMs: 15_000, deadlineExpired: true, - // Nothing in the synthetic binding moved, so HACK-1318 reports the deadline. + // Nothing in the synthetic binding moved, so HACK-1340 reports the deadline. bindingUnchanged: true, }); expect((await h.receipt()).routingHandoff).toBe("held"); @@ -1442,3 +1442,86 @@ test("HACK-1340: a real binding drift within the caller deadline still refuses a await store.close(); } }); +// HACK-1391: the two deadline guards of a retained v14 operation, each proven on +// its own. The caller-deadline catch in `mutateRetainedContainers` keeps an +// observed drift's code even when the drift is reported after the deadline, and +// `requireMutationDeadline` issues the deadline code, not a drift, when the +// callback's revocation fence finds the deadline passed. +test("HACK-1391: a binding drift reported after the caller deadline keeps its binding-change code", async () => { + const { store, generation } = await prepare(); + const realNow = Date.now; + let offset = 0; + const clock = spyOn(Date, "now").mockImplementation(() => realNow() + offset); + const before = bindingFacts(); + let driftedReads = 0; + try { + h.hooks.afterEffect = async () => { + h.model.volumeBirth = "2026-02-02T01:02:03Z"; + }; + h.hooks.afterProbe = async (args) => { + // The first read of the drifted volume returns the changed birth; the + // clock then passes the deadline before the store compares it (the + // bracketed scan may reread the volume once more before comparing). + if ( + h.effects.length === 1 && + args[0] === "volume" && + args[1] === "inspect" && + ++driftedReads === 1 + ) { + offset = SLOW_OPERATION_ADVANCE_MS; + } + }; + const code = await refusalCode( + h.operation(store, generation, "stop", { preparation: true }) + ); + expect(driftedReads).toBeGreaterThanOrEqual(1); + expect(bindingFacts()).not.toBe(before); + expect(h.lastOperation()).toMatchObject({ + outcome: "refused", + deadlineExpired: true, + bindingUnchanged: false, + }); + expect(code).toBe("E_LEGACY_ADOPTION_CHANGED"); + expect(h.effects).toHaveLength(1); + expect((await h.receipt()).pendingOperation?.operation).toBe("stop"); + } finally { + h.hooks.afterEffect = undefined; + h.hooks.afterProbe = undefined; + clock.mockRestore(); + await store.close(); + } +}); +test("HACK-1391: the callback revocation fence past the caller deadline refuses as a deadline, not a binding change", async () => { + const { store, generation } = await prepare(); + const realNow = Date.now; + let offset = 0; + const clock = spyOn(Date, "now").mockImplementation(() => realNow() + offset); + const before = bindingFacts(); + let fenced = false; + try { + const deadline = Date.now() + 15_000; + const code = await refusalCode( + store.withPreparationStop({ + generation, + binary: h.compiler, + deadline, + run: async (input) => { + // No probe or check runs between the jump and the fence, so only + // `requireMutationDeadline` can refuse here. + offset = SLOW_OPERATION_ADVANCE_MS; + fenced = true; + input.assertActive(); + return 0; + }, + }) + ); + expect(fenced).toBe(true); + expect(bindingFacts()).toBe(before); + expect(code).toBe("E_LEGACY_ADOPTION_DEADLINE"); + expect(h.effects).toHaveLength(0); + expect((await h.receipt()).pendingOperation?.operation).toBe("stop"); + } finally { + clock.mockRestore(); + await store.close(); + } +});