Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .ai/skills/hack-repo-verify/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ exit code, a guarded global function or a process listener changed fails with a
environment, cwd, umask and exit code so later files run clean; it keeps
replaced functions, which may still protect unsettled work. It keeps a test's
leftover listeners until its file ends, then puts the file's listeners back, so
a later `process.emit` is not blamed for them. Fix the named file's restore
a later `process.emit` is not blamed for them. Removing a leftover `SIGINT` or
`SIGTERM` handler also drops whatever cleanup it would have run, so stop what
it owns in the file's own teardown. Fix the named file's restore
(`try`/`finally` or `afterEach`), not the later failures. To emit a real signal
in a test, use `withOwnedProcessListeners` from
`tests/helpers/owned-process-listeners.ts`.
`tests/helpers/owned-process-listeners.ts`; it puts the listeners it took off
back even when the test times out, with `once` listeners kept `once`.
Restore environment keys with `restoreEnv` from `tests/helpers/env.ts`: Bun keeps
a deleted `HTTP_PROXY`-style key for `fetch` and `Bun.spawn` without `env`, and
the guard reports that as `native env <KEY> changed`.
Expand Down
5 changes: 3 additions & 2 deletions docs/reference/native-compose-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,9 @@ seed, verification, closing root proof, completion write and publication. Carrie
transport additionally distinguishes target, create, policy, start, response and
cleanup; policy reasons identify shape, command, labels, host policy, mount
cardinality, program/storage mounts and policy stability. A re-inspected carrier
whose creation time or policy changed reports `helper-policy-stability`, and a
saved material binding that changed during a carrier step reports
whose creation time, policy or stopped state changed reports
`helper-policy-stability`, and a saved material binding that changed during a
carrier step, or before a verification's deferred evidence retirement, reports
`private-state`. A policy or state refusal keeps its reason even when it is
caught after the carrier's deadline. A timeout,
budget or otherwise unclassified refusal that finds its carrier's storage budget
Expand Down
5 changes: 3 additions & 2 deletions docs/reference/native-compose-storage-xattr-carrier.md
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,9 @@ that end. A refusal with its own observed reason keeps that reason even when it
is caught after the deadline, including when a recorded command or its command
record replaces it with a fixed refusal. Such reasons include a helper policy
refusal, `helper-policy-stability` for a re-inspected carrier whose creation
time or policy changed, and `private-state` for a saved material binding that
changed during a carrier step. Cancellation
time, policy or stopped state changed, and `private-state` for a saved material
binding that changed during a carrier step or before a verification's deferred
evidence retirement. Cancellation
keeps `probe-cancel`. The reason changes no check, retention or recovery rule.

Only the original in-memory writer can advance its exact sequence. Publication
Expand Down
42 changes: 42 additions & 0 deletions packages/runtime-core/tests/project_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,48 @@ fn hack_ownership_label_namespaces_block_enrollment() {
assert!(!fixture.candidate.state_root.exists());
}
}
// The dependency cache declaration form: its own `hack.dependencies.*`
// labels are recognized, so without an ownership key nothing else reports
// an owner label. With one, the declaration does not exempt it.
let dependency_cache = |extra: &str| {
Fixture::new(&format!(
"services:\n deps:\n image: alpine:3.21\n volumes: ['node_modules:/app/node_modules']\n labels:\n hack.dependencies.cache-volume: node_modules\n hack.dependencies.lockfiles: bun.lock\n hack.dependencies.bootstrap: 'true'\n{extra}volumes:\n node_modules: {{}}\n"
))
};
let declared = dependency_cache("");
assert!(
declared.plan().plan.services["deps"]
.dependency_cache
.is_some()
);
assert!(!owner_label(&declared));
for key in [
"io.hack-local.owner",
"io.hack.native-config.owner",
"io.hack.native-file.token",
"io.hack.storage-witness.carrier",
] {
let fixture = dependency_cache(&format!(" '{key}': owner-canary\n"));
let plan = fixture.plan();
assert!(
plan.plan.services["deps"].dependency_cache.is_some(),
"{key}"
);
assert!(owner_label(&fixture), "{key}");
assert!(!plan.plan.enrollment_compatible, "{key}");
assert!(
!serde_json::to_string(&plan)
.unwrap()
.contains("owner-canary")
);
assert_eq!(
project::enroll(&fixture.candidate, fixture.options(), &plan.plan_id)
.unwrap_err()
.code,
"incompatible_compose"
);
assert!(!fixture.candidate.state_root.exists());
}
// Near misses outside every ownership namespace stay reviewable. Closer
// misses (a longer first segment, or another segment under the same
// reverse-DNS root) are pinned by the `ownership_labels` unit tests; they
Expand Down
40 changes: 28 additions & 12 deletions src/lib/native-compose-storage-witness-docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ export function assertNativeComposeStorageCarrierStable(
refuseObserved("storage-helper-policy", "helper-policy-stability");
}
}
/**
* Refuse unless a stopped helper carrier, inspected again, still shows the
* exact stopped state first observed. A change is the same instability as a
* changed creation time or policy, so it reports `helper-policy-stability` at
* `storage-helper-policy`; untagged, such a refusal caught after the carrier
* deadline would read `storage-budget`.
*/
export function assertNativeComposeStorageCarrierStateUnchanged(
observed: NativeComposeStorageDockerCarrier,
stopped: NativeComposeStorageDockerCarrier
): void {
if (!same(observed.state, stopped.state)) {
refuseObserved("storage-helper-policy", "helper-policy-stability");
}
}
function lifetime(context: Context): void {
if (context.signal.aborted) {
refuse();
Expand Down Expand Up @@ -2013,9 +2028,7 @@ export async function createNativeComposeDockerStorageXattrCarrier(
}
const repeated = await inspectCarrier(true);
assertNativeComposeStorageCarrierStable(repeated, known);
if (!same(repeated.state, stopped.state)) {
return refuse();
}
assertNativeComposeStorageCarrierStateUnchanged(repeated, stopped);
stage = "storage-helper-cleanup";
await checkFiles(files);
tool();
Expand All @@ -2042,9 +2055,10 @@ export async function createNativeComposeDockerStorageXattrCarrier(
assertAdmitted: async () => {
const finalCarrier = await inspectCarrier(true);
assertNativeComposeStorageCarrierStable(finalCarrier, owned);
if (!same(finalCarrier.state, stopped.state)) {
refuse();
}
assertNativeComposeStorageCarrierStateUnchanged(
finalCarrier,
stopped
);
await engine(probe(true), context);
tool();
assertNativeComposeStorageBindingUnchanged({
Expand Down Expand Up @@ -2117,14 +2131,16 @@ export async function createNativeComposeDockerStorageXattrCarrier(
const check = async () => {
await checkCompletion();
tool();
if (
!(
nativeComposeStorageCommandOwnerConfirmed(owner) &&
same(before, await current(context, input.target))
)
) {
if (!nativeComposeStorageCommandOwnerConfirmed(owner)) {
return refuse();
}
// Deferred retirement is the carrier's own cleanup: a binding
// changed since the invocation began reports `private-state`.
assertNativeComposeStorageBindingUnchanged({
stage: "storage-helper-cleanup",
before,
after: await current(context, input.target),
});
};
try {
await check();
Expand Down
33 changes: 26 additions & 7 deletions tests/global-state-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,39 +135,58 @@ test("a replaced function or listener is reported but left for its owner", () =>
expect(diffGlobalState(before)).toEqual([]);
});

test("the file-boundary listener restore removes leftovers, adds back missing listeners and keeps a kept once listener", async () => {
test("the file-boundary listener restore removes leftovers, adds back missing listeners with their kind and keeps a kept once listener", async () => {
await withOwnedProcessListeners("SIGUSR2", () => {
let keptOnceCalls = 0;
let removedOnceCalls = 0;
const keptOnce = () => {
keptOnceCalls += 1;
};
const removed = () => undefined;
const removedOnce = () => {
removedOnceCalls += 1;
};
const leftover = () => undefined;
const secondLeftover = () => undefined;
process.once("SIGUSR2", keptOnce);
process.on("SIGUSR2", removed);
process.once("SIGUSR2", removedOnce);
try {
const before = captureGlobalState();
process.removeListener("SIGUSR2", removed);
process.removeListener("SIGUSR2", removedOnce);
process.on("SIGUSR2", leftover);
process.once("SIGUSR2", secondLeftover);
expect(diffGlobalState(before)).toContain(
"process 'SIGUSR2' listeners changed (2 -> 3)"
"process 'SIGUSR2' listeners changed (3 -> 3)"
);

expect(restoreProcessListeners(before)).toEqual(["SIGUSR2"]);

expect(sameItems(process.listeners("SIGUSR2"), [keptOnce, removed])).toBe(
true
);
expect(
sameItems(process.listeners("SIGUSR2"), [
keptOnce,
removed,
removedOnce,
])
).toBe(true);
expect(diffGlobalState(before)).toEqual([]);
expect(restoreProcessListeners(before)).toEqual([]);
// The kept listener was never re-added, so it is still a `once`.
// The kept listener was never re-added, so it is still a `once`; the
// re-added `once` listener is a `once` again and the persistent one
// stays.
process.emit("SIGUSR2");
expect(keptOnceCalls).toBe(1);
expect(removedOnceCalls).toBe(1);
expect(sameItems(process.listeners("SIGUSR2"), [removed])).toBe(true);
} finally {
for (const listener of [keptOnce, removed, leftover, secondLeftover]) {
for (const listener of [
keptOnce,
removed,
removedOnce,
leftover,
secondLeftover,
]) {
process.removeListener("SIGUSR2", listener);
}
}
Expand Down
11 changes: 11 additions & 0 deletions tests/helpers/global-state-guard-preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
* file's own hooks and preload `afterEach` hooks after them, so a file's
* `beforeEach`/`afterEach` pair that restores correctly passes. A mismatch
* fails that test, restores the pre-test environment and cwd, and names the file.
* Before comparing, it puts back the listeners that an unsettled
* `withOwnedProcessListeners` callback took off (a test that timed out in it).
* - Per file: when Bun loads the next test file, the state must match the
* state before the previous file loaded. This catches module-scope and
* `beforeAll`/`afterAll` leaks, including an `afterAll` that never ran its
Expand Down Expand Up @@ -43,6 +45,12 @@ import {
restoreGlobalState,
restoreProcessListeners,
} from "./global-state-guard.ts";
import { restoreUnsettledOwnedProcessListeners } from "./owned-process-listeners.ts";
import { installProcessOnceListeners } from "./process-once-listeners.ts";

// Before any test file registers a listener, so a `once` listener that a
// helper takes off and puts back stays a `once` listener.
installProcessOnceListeners();

/**
* Environment keys a test file may change without a restore. Every entry
Expand Down Expand Up @@ -133,6 +141,9 @@ beforeEach(() => {
});

afterEach(() => {
// A test that timed out inside `withOwnedProcessListeners` hands the
// listeners it took off back first, so it is not blamed for removing them.
restoreUnsettledOwnedProcessListeners();
const before = testBaseline;
testBaseline = undefined;
if (before === undefined) {
Expand Down
35 changes: 26 additions & 9 deletions tests/helpers/global-state-guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
* because fixtures may hold secret-shaped values.
*/

import {
addProcessListener,
isProcessOnceListener,
} from "./process-once-listeners.ts";

/**
* Process events whose listener lists are compared. A change is reported per
* test and per file, and restored only at the file boundary (see
Expand Down Expand Up @@ -316,15 +321,23 @@ function countListeners(listeners: readonly Listener[]): Map<Listener, number> {
* (HACK-1416).
*
* Only the difference is edited: a listener added since the snapshot is
* removed and a snapshot listener that went missing is added back with
* `process.on`. Listeners present in both stay untouched, so a `once`
* listener there keeps its `once` behavior; Bun does not expose `once`, so a
* missing listener always comes back as a persistent one, at the end of the
* list.
* removed and a snapshot listener that went missing is added back. Listeners
* present in both stay untouched. A missing listener comes back with the
* registration kind it last had: a `once` listener as `once`, otherwise
* persistent (see `tests/helpers/process-once-listeners.ts`), at the end of
* the list.
*
* Unlike replaced functions, removing a leftover listener cannot let unsettled
* work reach the host: it only stops that work from reacting to a signal or
* exit event of the shared test process.
* Removing a leftover listener is not free of effects. A leftover `SIGINT` or
* `SIGTERM` handler can be legitimate cleanup, for example one that kills a
* child process the file spawned when the run is interrupted. Once it is
* removed, an interrupted test run no longer runs that cleanup, and whatever
* the handler owned can outlive the run. The guard still removes it rather
* than calling it or waiting for what it owns: it cannot know what the handler
* owns or whether running it now is safe, and calling another file's handler
* at an arbitrary point (or leaving it for a later file's `process.emit`) is
* the cross-file interference this restore prevents. The leak is reported
* with the file's name and fails the run, so the fix belongs in that file:
* remove its handler, and stop what it owns, in its own teardown.
*/
export function restoreProcessListeners(
snapshot: GlobalStateSnapshot
Expand Down Expand Up @@ -354,7 +367,11 @@ export function restoreProcessListeners(
if (left > 0) {
kept.set(listener, left - 1);
} else {
process.on(event, listener);
addProcessListener(
event,
listener,
isProcessOnceListener(event, listener)
);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions tests/helpers/native-compose-storage-docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,18 @@ if(storageArgs[0]==="container"&&storageArgs[1]==="inspect"){
if(format.includes(".HostConfig")){
// Fault: the Nth helper policy inspection observes a replaced carrier (a new creation time).
const replaced=storageRoot+"/fault-carrier-replaced",seen=storageRoot+"/carrier-policy-inspects";
// Fault: the Nth helper policy inspection observes a changed state (another finish time) under
// the same creation time and policy.
const restated=storageRoot+"/fault-carrier-state-changed",restatedSeen=storageRoot+"/carrier-state-inspects";
for(const id of selected){const row=carriers.find(value=>value.id===id);if(!row)process.exit(1);
if(await Bun.file(replaced).exists()){
const count=Number(await Bun.file(seen).exists()?await Bun.file(seen).text():0)+1;await Bun.write(seen,String(count));
if(count===Number(await Bun.file(replaced).text())){emit({...row,createdAt:"2026-01-01T00:00:00.000Z"});continue;}
}
if(await Bun.file(restated).exists()){
const count=Number(await Bun.file(restatedSeen).exists()?await Bun.file(restatedSeen).text():0)+1;await Bun.write(restatedSeen,String(count));
if(count===Number(await Bun.file(restated).text())){emit({...row,state:{...row.state,FinishedAt:"2026-01-01T00:00:00.000Z"}});continue;}
}
emit(row);}process.exit(0);
}
if(format.includes(".Mounts")){
Expand Down
Loading
Loading