Preserve environment resources through editor saves

This commit is contained in:
Scott Werner 2026-10-16 14:02:44 -04:00
parent 7ff4456b54
commit 79b091b035
2 changed files with 159 additions and 106 deletions

View file

@ -1,9 +1,16 @@
import { describe, expect, test } from "bun:test";
import { createElement, useState } from "react";
import TestRenderer, { act } from "react-test-renderer";
import type { Environment, EnvironmentResourcesSettings } from "@qltysh/fabro-api-client";
import { setupReactTestEnv } from "../lib/test-utils";
import {
EMPTY_ENVIRONMENT_FORM,
EnvironmentFormFields,
createRequestFromForm,
environmentToFormValues,
isEnvironmentFormValid,
replaceRequestFromForm,
type EnvironmentFormValues,
} from "./environment-form";
@ -48,3 +55,113 @@ describe("environment image source", () => {
expect(request.image.dockerfile?.value).toBe("FROM ubuntu");
});
});
function environment(resources: EnvironmentResourcesSettings): Environment {
return {
id: "roundtrip",
revision: "revision-1",
provider: "docker",
image: { docker: "ubuntu:24.04", dockerfile: null },
resources,
network: { mode: "allow_all", allow: [] },
lifecycle: { preserve: false, stop_on_terminal: true, auto_stop: null },
labels: { owner: "api", purpose: "roundtrip" },
env: { TZ: "UTC" },
};
}
describe("environment resource round trips", () => {
const cases: EnvironmentResourcesSettings[] = [
// API response for 512MiB; regression originally reported in #952.
{ cpu: null, memory: "536870912B", disk: null },
{ cpu: 4, memory: "4GB", disk: "8GB" },
{ cpu: null, memory: null, disk: null },
{ cpu: 32, memory: "64GB", disk: "2TB" },
{ cpu: 1, memory: "512MiB", disk: "1GiB" },
{ cpu: 2, memory: "1.5GB", disk: "512MB" },
{ cpu: 2, memory: "1B", disk: "1TiB" },
];
for (const resources of cases) {
test(`no-op save preserves ${JSON.stringify(resources)}`, () => {
const original = environment(resources);
const { id, revision, ...settings } = original;
expect(replaceRequestFromForm(environmentToFormValues(original))).toEqual(settings);
});
test(`unrelated edits preserve ${JSON.stringify(resources)} and labels`, () => {
const original = environment(resources);
const values = environmentToFormValues(original);
const saved = replaceRequestFromForm({
...values,
dockerRef: "ubuntu:26.04",
preserve: true,
autoStop: "30m",
envVars: [...values.envVars, { key: "EDITOR", value: "vi" }],
});
expect(saved.resources).toEqual(resources);
expect(saved.labels).toEqual(original.labels);
expect(saved.image.docker).toBe("ubuntu:26.04");
expect(saved.lifecycle).toEqual({ preserve: true, stop_on_terminal: true, auto_stop: "30m" });
expect(saved.env).toEqual({ TZ: "UTC", EDITOR: "vi" });
});
}
test("editing one resource preserves the other resources", () => {
const values = environmentToFormValues(environment(cases[0]));
expect(replaceRequestFromForm({ ...values, cpu: 16 }).resources).toEqual({
...cases[0], cpu: 16,
});
expect(replaceRequestFromForm({ ...values, memory: "768MiB" }).resources).toEqual({
...cases[0], memory: "768MiB",
});
expect(replaceRequestFromForm({ ...values, disk: "32GB" }).resources).toEqual({
...cases[0], disk: "32GB",
});
});
test("clearing resources restores provider defaults", () => {
const values = environmentToFormValues(environment(cases[1]));
expect(replaceRequestFromForm({ ...values, cpu: null, memory: "", disk: " " }).resources)
.toEqual({ cpu: null, memory: null, disk: null });
});
test("new environments keep the existing resource defaults", () => {
expect(createRequestFromForm(form({ dockerRef: "ubuntu:24.04" })).resources)
.toEqual({ cpu: 4, memory: "8GB", disk: "16GB" });
});
});
test("resource inputs display API values and preserve them through user edits", () => {
const cleanup = setupReactTestEnv();
let renderer: TestRenderer.ReactTestRenderer | undefined;
let saved: ReturnType<typeof replaceRequestFromForm> | undefined;
function Editor() {
const [values, setValues] = useState(() => environmentToFormValues(environment({
cpu: null, memory: "536870912B", disk: null,
})));
saved = replaceRequestFromForm(values);
return createElement(EnvironmentFormFields, { values, onChange: setValues, lockId: true });
}
try {
act(() => { renderer = TestRenderer.create(createElement(Editor)); });
const input = (label: string) => renderer!.root.findByProps({ "aria-label": label });
expect(input("CPU").props.value).toBe("");
expect(input("Memory").props.value).toBe("536870912B");
expect(input("Disk").props.value).toBe("");
act(() => { input("Image reference").props.onChange({ target: { value: "alpine:latest" } }); });
expect(saved!.resources).toEqual({ cpu: null, memory: "536870912B", disk: null });
expect(saved!.labels).toEqual({ owner: "api", purpose: "roundtrip" });
act(() => { input("CPU").props.onChange({ target: { value: "32" } }); });
act(() => { input("Memory").props.onChange({ target: { value: "768MiB" } }); });
act(() => { input("Disk").props.onChange({ target: { value: "64GB" } }); });
expect(saved!.resources).toEqual({ cpu: 32, memory: "768MiB", disk: "64GB" });
for (const label of ["CPU", "Memory", "Disk"]) {
act(() => { input(label).props.onChange({ target: { value: "" } }); });
}
expect(saved!.resources).toEqual({ cpu: null, memory: null, disk: null });
} finally {
act(() => { renderer?.unmount(); });
cleanup();
}
});

View file

@ -39,12 +39,6 @@ export function parseCreatableProvider(value: string | null): string {
// Environment ids are server-managed file names: lowercase, digits, hyphens.
const ENVIRONMENT_ID_PATTERN = /^[a-z0-9][a-z0-9-]{0,62}$/;
// Resource sliders pick a concrete value within a fixed range. Memory and disk
// are expressed in whole GB; the wire format keeps the `GB` suffix string.
const CPU = { min: 1, max: 8, step: 1, default: 4 };
const MEMORY = { min: 1, max: 16, step: 1, default: 8 };
const DISK = { min: 1, max: 20, step: 1, default: 16 };
// An environment image comes from exactly one source: a prebuilt image
// reference or an inline Dockerfile. The form keeps both field values around so
// switching back and forth doesn't lose typed text, and this discriminator
@ -57,9 +51,10 @@ export interface EnvironmentFormValues {
imageSource: ImageSource;
dockerRef: string;
dockerfile: string;
cpu: number;
memory: number;
disk: number;
// Keep API resource values intact; blank size inputs represent provider defaults.
cpu: number | null;
memory: string;
disk: string;
blockNetwork: boolean;
preserve: boolean;
stopOnTerminal: boolean;
@ -77,9 +72,9 @@ export const EMPTY_ENVIRONMENT_FORM: EnvironmentFormValues = {
imageSource: "image",
dockerRef: "",
dockerfile: "",
cpu: CPU.default,
memory: MEMORY.default,
disk: DISK.default,
cpu: 4,
memory: "8GB",
disk: "16GB",
blockNetwork: false,
preserve: false,
stopOnTerminal: true,
@ -95,9 +90,9 @@ export function environmentToFormValues(environment: Environment): EnvironmentFo
imageSource: environment.image.dockerfile ? "dockerfile" : "image",
dockerRef: environment.image.docker ?? "",
dockerfile: environment.image.dockerfile?.value ?? "",
cpu: clampGb(environment.resources.cpu, CPU),
memory: parseGb(environment.resources.memory, MEMORY),
disk: parseGb(environment.resources.disk, DISK),
cpu: environment.resources.cpu,
memory: environment.resources.memory ?? "",
disk: environment.resources.disk ?? "",
blockNetwork: environment.network.mode === EnvironmentNetworkMode.BLOCK,
preserve: environment.lifecycle.preserve,
stopOnTerminal: environment.lifecycle.stop_on_terminal,
@ -169,43 +164,11 @@ function imageFromForm(values: EnvironmentFormValues): EnvironmentApiImageSettin
function resourcesFromForm(values: EnvironmentFormValues): EnvironmentResourcesSettings {
return {
cpu: values.cpu,
memory: `${values.memory}GB`,
disk: `${values.disk}GB`,
memory: values.memory.trim() || null,
disk: values.disk.trim() || null,
};
}
interface ResourceRange {
min: number;
max: number;
step: number;
default: number;
}
// Snap a numeric value into the slider range, falling back to the default when
// the environment leaves the resource unset (provider default).
function clampGb(value: number | null, range: ResourceRange): number {
if (value === null) return range.default;
return Math.min(range.max, Math.max(range.min, Math.round(value)));
}
// Parse a size string ("16GB", "512MiB", or a bare integer interpreted as GB)
// into whole GB within the slider range. Existing values may use other units or
// fall outside the range, so the result is rounded and clamped.
function parseGb(value: string | null, range: ResourceRange): number {
if (value === null) return range.default;
const match = value.trim().match(/^([\d.]+)\s*([a-zA-Z]*)$/);
if (!match) return range.default;
const amount = Number(match[1]);
if (!Number.isFinite(amount)) return range.default;
const perGb: { [unit: string]: number } = {
"": 1, g: 1, gb: 1, gib: 1,
m: 1 / 1000, mb: 1 / 1000, mib: 1 / 1000,
t: 1000, tb: 1000, tib: 1000,
};
const factor = perGb[match[2].toLowerCase()] ?? 1;
return clampGb(amount * factor, range);
}
function networkFromForm(values: EnvironmentFormValues): EnvironmentNetworkSettings {
return {
mode: values.blockNetwork ? EnvironmentNetworkMode.BLOCK : EnvironmentNetworkMode.ALLOW_ALL,
@ -318,31 +281,42 @@ export function EnvironmentFormFields({
</Panel>
<Panel title="Resources">
<Row title="CPU" help="Number of vCPUs allocated to each run.">
<ResourceSlider
ariaLabel="CPU"
range={CPU}
value={values.cpu}
onChange={(cpu) => patch({ cpu })}
format={(n) => `${n} CPU`}
<Row title={<Label optional>CPU</Label>} help="Number of vCPUs allocated to each run. Leave blank for the provider default.">
<input
type="number"
name="cpu"
aria-label="CPU"
step={1}
value={values.cpu ?? ""}
onChange={(e) => patch({ cpu: e.target.value === "" ? null : Number(e.target.value) })}
placeholder="Provider default"
className={`${INPUT_CLASS} font-mono`}
/>
</Row>
<Row title="Memory" help="Memory limit for each run.">
<ResourceSlider
ariaLabel="Memory"
range={MEMORY}
<Row title={<Label optional>Memory</Label>} help="Memory limit, including units (e.g. 512MiB or 4GB). Leave blank for the provider default.">
<input
type="text"
name="memory"
aria-label="Memory"
value={values.memory}
onChange={(memory) => patch({ memory })}
format={(n) => `${n} GB`}
onChange={(e) => patch({ memory: e.target.value })}
placeholder="Provider default"
autoComplete="off"
spellCheck={false}
className={`${INPUT_CLASS} font-mono`}
/>
</Row>
<Row title="Disk" help="Disk limit for each run.">
<ResourceSlider
ariaLabel="Disk"
range={DISK}
<Row title={<Label optional>Disk</Label>} help="Disk limit, including units (e.g. 8GB). Leave blank for the provider default.">
<input
type="text"
name="disk"
aria-label="Disk"
value={values.disk}
onChange={(disk) => patch({ disk })}
format={(n) => `${n} GB`}
onChange={(e) => patch({ disk: e.target.value })}
placeholder="Provider default"
autoComplete="off"
spellCheck={false}
className={`${INPUT_CLASS} font-mono`}
/>
</Row>
</Panel>
@ -425,44 +399,6 @@ export function EnvironmentFormFields({
);
}
function ResourceSlider({
value,
range,
ariaLabel,
format,
onChange,
}: {
value: number;
range: ResourceRange;
ariaLabel: string;
format: (value: number) => string;
onChange: (value: number) => void;
}) {
const fill = ((value - range.min) / (range.max - range.min)) * 100;
return (
<div className="flex items-center gap-4">
<div className="relative h-4 flex-1">
<div className="pointer-events-none absolute inset-x-0 top-1/2 h-1.5 -translate-y-1/2 rounded-full bg-overlay-strong">
<div className="h-full rounded-full bg-teal-500" style={{ width: `${fill}%` }} />
</div>
<input
type="range"
aria-label={ariaLabel}
value={value}
min={range.min}
max={range.max}
step={range.step}
onChange={(e) => onChange(Number(e.target.value))}
className="relative h-4 w-full cursor-pointer appearance-none bg-transparent focus-visible:outline-none [&::-moz-range-thumb]:size-4 [&::-moz-range-thumb]:rounded-full [&::-moz-range-thumb]:border-0 [&::-moz-range-thumb]:bg-fg [&::-moz-range-thumb]:shadow-sm [&::-moz-range-track]:h-1.5 [&::-moz-range-track]:rounded-full [&::-moz-range-track]:bg-transparent [&::-webkit-slider-runnable-track]:h-1.5 [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:bg-transparent [&::-webkit-slider-thumb]:-mt-[5px] [&::-webkit-slider-thumb]:size-4 [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-fg [&::-webkit-slider-thumb]:shadow-sm [&::-webkit-slider-thumb]:outline [&::-webkit-slider-thumb]:outline-1 [&::-webkit-slider-thumb]:-outline-offset-1 [&::-webkit-slider-thumb]:outline-line-strong focus-visible:[&::-webkit-slider-thumb]:outline-2 focus-visible:[&::-webkit-slider-thumb]:outline-teal-500"
/>
</div>
<output className="w-16 shrink-0 text-right font-mono text-sm tabular-nums text-fg">
{format(value)}
</output>
</div>
);
}
function ToggleSwitch({
checked,
onChange,