aea35fe695
## Thinking Path
> - Paperclip orchestrates AI agents and provisions sandboxed execution
environments for them; one of those provisioners is the exe.dev plugin,
which runs each agent inside a long-lived VM reached over SSH.
> - The instance-config form for that plugin is rendered generically by
`JsonSchemaForm` from the plugin's `instanceConfigSchema`, so any UX
problem with the form is split between the shared form component and the
plugin's schema/runtime code.
> - Users coming in cold hit a 12-field flat config they couldn't reason
about (PAPA-407), a form that silently submitted `cpu: 0` for untouched
optional fields (PAPA-407 root cause), a `sshPrivateKey` textarea that
truncated RSA-4096 keys at 4096 chars (PAPA-449), a save flow that
accepted clearly-malformed keys and only blew up at lease time with raw
SSH stderr (PAPA-450, PAPA-451), and a manifest that didn't distinguish
"essential" from "advanced" knobs (PAPA-410 / PAPA-411 — duplicate
sub-issues with identical scope; PAPA-418 reconciliation kept PAPA-410
canonical).
> - These problems all point at the same surface (exe.dev sandbox
config) and are tightly coupled in code — PAPA-449/450/451 patch fields
that PAPA-410/411 introduce — so they get reviewed together.
> - This pull request lands the shared-form changes (advanced-options
disclosure, optional-scalar defaults) and the exe.dev-specific changes
(manifest restructure, longer `maxLength`, stderr translation, save-time
key validation) as five focused commits stacked on `master`.
> - The benefit is a config form that defaults to the two fields a new
user actually needs (API key + SSH private key) with a collapsible
disclosure for the rest, no silent truncation or zero-default
submissions, and SSH key problems surfaced at save time with actionable
messages instead of cryptic post-provision failures.
## What Changed
- **JsonSchemaForm advanced-options disclosure** (PAPA-410, PAPA-411 —
same scope, see note above): adds `x-paperclip-advanced` /
`x-paperclip-group` schema annotations and renders flagged fields behind
a collapsible "Advanced options" disclosure that auto-opens when a
hidden field has a validation error. Exe.dev manifest is restructured to
use the new annotations, so essentials (`apiKey`, `sshPrivateKey`) show
by default while the long tail of optional knobs is grouped under "SSH
access" / "VM resources" / "More options" headings.
- **Omit optional scalar defaults** (PAPA-407): `getDefaultForSchema` no
longer materialises `0` / `""` for optional
`number`/`integer`/`string`/`secret-ref` fields without an explicit
`default`. Object recursion drops properties whose default is
`undefined`. Fields that declare a `default` (e.g. `sshPort: 22`) still
round-trip. Adds a regression test against `getDefaultValues`.
- **Raise `sshPrivateKey` `maxLength`** (PAPA-449): bumps the exe.dev
manifest cap from 4096 to 8192 so RSA-4096 OpenSSH private keys (which
can exceed 4 KB with comments/metadata) aren't silently truncated at
submit.
- **Translate `invalid format` SSH stderr** (PAPA-450):
`formatSshFailure` now recognises `Load key … invalid format` in
combined stderr/stdout and returns a specific message naming the
key-format problem ("isn't an OpenSSH/PEM private key — confirm the
secret starts with `-----BEGIN … PRIVATE KEY-----` and isn't the `.pub`
or a PuTTY `.ppk` export") instead of dumping the raw stderr.
- **Save-time SSH key validation** (PAPA-451):
`onEnvironmentValidateConfig` inline-parses `sshPrivateKey` and rejects
common failure modes — pasted public keys, PuTTY `.ppk` format, missing
`-----END-----` footer, non-base64 body — so the form surfaces an inline
error before any VM is provisioned. Secret-ref bindings (UUIDs) are
still passed through unchanged.
## Verification
CI gates (`pnpm typecheck`, `pnpm test`, the targeted vitest suites
below) all pass.
Run locally:
```bash
# Shared form
pnpm --filter @paperclipai/ui exec vitest run src/components/JsonSchemaForm
# 9 tests pass — includes the new "omits optional scalar fields" regression
# and the three advanced-options-disclosure tests.
# exe.dev plugin
cd packages/plugins/sandbox-providers/exe-dev && pnpm test
# 32 tests pass — includes the new sshPrivateKey-validation cases
# and the new "invalid format" stderr-translation case.
```
Manual smoke (after reinstalling the plugin so the DB manifest
refreshes):
1. Open the exe.dev environment config page. **Default view shows API
Key + SSH Private Key only**, with an "Advanced options" disclosure for
everything else (PAPA-410 / PAPA-411).
2. Paste a `.pub` file's contents into SSH Private Key, click Save.
**Inline error** rejecting the wrong-format key (PAPA-451).
3. Re-paste a valid OpenSSH/PEM private key longer than 4096 bytes —
saves cleanly (PAPA-449).
4. Save the form with everything optional left blank — server no longer
rejects with `"cpu must be greater than 0 when provided"` (PAPA-407).
5. Force a bad key through via a stored secret-ref binding and lease a
VM — failure message names the key-format problem instead of dumping raw
SSH stderr (PAPA-450).
## Risks
- **PAPA-410 / PAPA-411 manifest restructure** is the largest surface
here. Schemas using `x-paperclip-*` extensions are forward-compatible
with stricter JSON Schema validators (extensions are ignored by
default), and the form gracefully renders a flat layout when no field
opts in.
- **PAPA-407** changes form-default behaviour: optional scalar fields
that previously round-tripped as `""` / `0` will now be `undefined` and
absent from the submitted payload. Downstream consumers that expected
the empty-string/zero shape need to treat the field as optional.
Spot-checked the existing exe.dev driver — it already uses
`parseOptionalString` / `parseOptionalInteger`, which treat missing
fields as `null` rather than `0`/`""`.
- **PAPA-451** adds a save-time check, so a
previously-saved-but-malformed `sshPrivateKey` raw value will now fail
to re-save. Bound secret-refs are unaffected, matching how the user
reaches the bad-key state today (via the secrets picker).
- **PAPA-449** simply raises a cap; no semantic risk.
- **PAPA-450** only kicks in on the "invalid format" code path; existing
onboarding-marker branch is untouched.
## Model Used
- Provider: Anthropic
- Model: Claude Opus 4.7 (`claude-opus-4-7`)
- Capabilities used: code reading, code editing, test execution, git/PR
mechanics, Paperclip API for issue coordination
## Checklist
- [x] PR body sections present (Thinking Path, What Changed,
Verification, Risks, Model Used, Checklist)
- [x] Unit tests added for the new behaviours (JsonSchemaForm
default-value omission + advanced disclosure; exe.dev plugin validation
+ stderr translation)
- [x] Existing tests still pass locally (`vitest run` on both packages)
- [x] No raw secrets, IP addresses, or machine-local config in commits
or PR body
- [x] Commits are atomic per linked issue (PAPA-410 / PAPA-411,
PAPA-407, PAPA-449, PAPA-450, PAPA-451)
- [x] Branch is up-to-date with `origin/master`
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
410 lines
12 KiB
TypeScript
410 lines
12 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { act } from "react";
|
|
import { createRoot } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { JsonSchemaForm, getDefaultValues } from "./JsonSchemaForm";
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
// SecretBindingPicker pulls in CompanyContext + react-query. Stub it so we can
|
|
// exercise SecretField in isolation. The stub renders a select with the same
|
|
// onChange contract as the real picker.
|
|
vi.mock("./SecretBindingPicker", () => ({
|
|
SecretBindingPicker: ({
|
|
value,
|
|
onChange,
|
|
disabled,
|
|
}: {
|
|
value: { secretId: string } | null;
|
|
onChange: (next: { secretId: string } | null) => void;
|
|
disabled?: boolean;
|
|
}) => (
|
|
<select
|
|
data-testid="secret-binding-picker"
|
|
value={value?.secretId ?? ""}
|
|
onChange={(event) => {
|
|
const next = event.target.value;
|
|
onChange(next ? { secretId: next } : null);
|
|
}}
|
|
disabled={disabled}
|
|
>
|
|
<option value="">none</option>
|
|
<option value="11111111-1111-4111-8111-111111111111">existing-secret</option>
|
|
</select>
|
|
),
|
|
}));
|
|
|
|
describe("JsonSchemaForm secret-ref rendering", () => {
|
|
let container: HTMLDivElement;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
});
|
|
|
|
afterEach(() => {
|
|
container.remove();
|
|
document.body.innerHTML = "";
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it("renders multiline secret-ref fields as textareas alongside the picker", async () => {
|
|
const root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
sshPrivateKey: {
|
|
type: "string",
|
|
format: "secret-ref",
|
|
maxLength: 4096,
|
|
},
|
|
},
|
|
}}
|
|
values={{ sshPrivateKey: "secret" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
// The picker is always rendered, and a non-UUID raw value auto-opens the
|
|
// textarea fallback.
|
|
expect(container.querySelector('[data-testid="secret-binding-picker"]')).not.toBeNull();
|
|
expect(container.querySelector("textarea")).not.toBeNull();
|
|
expect(container.querySelector('input[type="password"]')).toBeNull();
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("renders the picker and hides the raw input when the value is a UUID secret ref", async () => {
|
|
const root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
apiKey: {
|
|
type: "string",
|
|
format: "secret-ref",
|
|
},
|
|
},
|
|
}}
|
|
values={{ apiKey: "11111111-1111-4111-8111-111111111111" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
expect(
|
|
container.querySelector('[data-testid="secret-binding-picker"]'),
|
|
).not.toBeNull();
|
|
// No raw input or textarea is visible while a secret is bound.
|
|
expect(container.querySelector('input[type="password"]')).toBeNull();
|
|
expect(container.querySelector("textarea")).toBeNull();
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("writes the secret id to form values when the picker selects an existing secret", async () => {
|
|
const root = createRoot(container);
|
|
const onChange = vi.fn();
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
apiKey: {
|
|
type: "string",
|
|
format: "secret-ref",
|
|
},
|
|
},
|
|
}}
|
|
values={{ apiKey: "" }}
|
|
onChange={onChange}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const picker = container.querySelector<HTMLSelectElement>(
|
|
'[data-testid="secret-binding-picker"]',
|
|
);
|
|
expect(picker).not.toBeNull();
|
|
|
|
const setSelectValue = Object.getOwnPropertyDescriptor(
|
|
window.HTMLSelectElement.prototype,
|
|
"value",
|
|
)?.set;
|
|
expect(setSelectValue).toBeTruthy();
|
|
|
|
await act(async () => {
|
|
setSelectValue!.call(picker!, "11111111-1111-4111-8111-111111111111");
|
|
picker!.dispatchEvent(new Event("change", { bubbles: true }));
|
|
});
|
|
|
|
expect(onChange).toHaveBeenCalledWith({
|
|
apiKey: "11111111-1111-4111-8111-111111111111",
|
|
});
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("auto-opens the raw input when a raw value arrives after mount", async () => {
|
|
const root = createRoot(container);
|
|
|
|
const schema = {
|
|
type: "object" as const,
|
|
properties: {
|
|
apiKey: {
|
|
type: "string" as const,
|
|
format: "secret-ref" as const,
|
|
},
|
|
},
|
|
};
|
|
|
|
// First render with empty value — picker visible, no raw input.
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm schema={schema} values={{ apiKey: "" }} onChange={() => {}} />,
|
|
);
|
|
});
|
|
expect(container.querySelector('input[type="password"]')).toBeNull();
|
|
|
|
// Parent fills in a previously-saved raw value (the async load case).
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={schema}
|
|
values={{ apiKey: "loaded-from-api" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const input = container.querySelector<HTMLInputElement>('input[type="password"]');
|
|
expect(input).not.toBeNull();
|
|
expect(input?.value).toBe("loaded-from-api");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("renders no Advanced disclosure when no field opts in", async () => {
|
|
const root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
apiKey: { type: "string", format: "secret-ref" },
|
|
region: { type: "string" },
|
|
},
|
|
}}
|
|
values={{ apiKey: "", region: "" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
// No disclosure button should be present in the passthrough case.
|
|
const buttons = Array.from(container.querySelectorAll("button"));
|
|
const advancedButton = buttons.find((b) =>
|
|
b.textContent?.includes("Advanced options"),
|
|
);
|
|
expect(advancedButton).toBeUndefined();
|
|
|
|
// Both fields render in the flat layout: the secret picker (rendered as
|
|
// a <select> stub) for apiKey and a text input for region.
|
|
expect(
|
|
container.querySelector('[data-testid="secret-binding-picker"]'),
|
|
).not.toBeNull();
|
|
expect(container.querySelector('input[type="text"]')).not.toBeNull();
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("hides advanced fields behind a collapsed disclosure with group headings", async () => {
|
|
const root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
apiKey: { type: "string", format: "secret-ref" },
|
|
sshPort: {
|
|
type: "number",
|
|
"x-paperclip-advanced": true,
|
|
"x-paperclip-group": "SSH access",
|
|
},
|
|
namePrefix: {
|
|
type: "string",
|
|
"x-paperclip-advanced": true,
|
|
},
|
|
},
|
|
}}
|
|
values={{ apiKey: "", sshPort: 22, namePrefix: "paperclip" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const buttons = Array.from(container.querySelectorAll("button"));
|
|
const advancedButton = buttons.find((b) =>
|
|
b.textContent?.includes("Advanced options"),
|
|
);
|
|
expect(advancedButton).toBeDefined();
|
|
expect(advancedButton!.getAttribute("aria-expanded")).toBe("false");
|
|
|
|
// Collapsed: number/text inputs from advanced fields aren't rendered.
|
|
expect(container.querySelector('input[type="number"]')).toBeNull();
|
|
// Group headings aren't visible while collapsed.
|
|
expect(container.textContent).not.toContain("SSH access");
|
|
expect(container.textContent).not.toContain("More options");
|
|
|
|
// Expand and verify both groups + the default bucket appear.
|
|
await act(async () => {
|
|
advancedButton!.click();
|
|
});
|
|
|
|
expect(advancedButton!.getAttribute("aria-expanded")).toBe("true");
|
|
expect(container.querySelector('input[type="number"]')).not.toBeNull();
|
|
expect(container.textContent).toContain("SSH access");
|
|
expect(container.textContent).toContain("More options");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("force-opens the disclosure when an error lands on a hidden advanced field", async () => {
|
|
const root = createRoot(container);
|
|
|
|
const schema = {
|
|
type: "object" as const,
|
|
properties: {
|
|
apiKey: { type: "string" as const, format: "secret-ref" as const },
|
|
sshPort: {
|
|
type: "number" as const,
|
|
"x-paperclip-advanced": true,
|
|
},
|
|
},
|
|
};
|
|
|
|
// No errors -> collapsed
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={schema}
|
|
values={{ apiKey: "", sshPort: 22 }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
let advancedButton = Array.from(container.querySelectorAll("button")).find(
|
|
(b) => b.textContent?.includes("Advanced options"),
|
|
);
|
|
expect(advancedButton!.getAttribute("aria-expanded")).toBe("false");
|
|
|
|
// Submit validation error on the hidden advanced field -> forced open
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={schema}
|
|
values={{ apiKey: "", sshPort: 22 }}
|
|
onChange={() => {}}
|
|
errors={{ "/sshPort": "Must be at least 1" }}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
advancedButton = Array.from(container.querySelectorAll("button")).find(
|
|
(b) => b.textContent?.includes("Advanced options"),
|
|
);
|
|
expect(advancedButton!.getAttribute("aria-expanded")).toBe("true");
|
|
expect(container.textContent).toContain("Must be at least 1");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
|
|
it("omits optional scalar fields from getDefaultValues so empty inputs aren't submitted as 0/''", () => {
|
|
const defaults = getDefaultValues({
|
|
type: "object",
|
|
properties: {
|
|
apiKey: { type: "string", format: "secret-ref" },
|
|
sshPort: { type: "number", default: 22 },
|
|
cpu: { type: "number" },
|
|
memory: { type: "string" },
|
|
reuseLease: { type: "boolean", default: false },
|
|
tags: { type: "array", items: { type: "string" } },
|
|
},
|
|
});
|
|
|
|
// Fields with explicit defaults round-trip.
|
|
expect(defaults.sshPort).toBe(22);
|
|
expect(defaults.reuseLease).toBe(false);
|
|
expect(defaults.tags).toEqual([]);
|
|
|
|
// Optional scalars without explicit defaults stay out of the payload so
|
|
// the server doesn't see e.g. `cpu: 0` and reject the submission.
|
|
expect("apiKey" in defaults).toBe(false);
|
|
expect("cpu" in defaults).toBe(false);
|
|
expect("memory" in defaults).toBe(false);
|
|
});
|
|
|
|
it("keeps the password fallback for short raw values", async () => {
|
|
const root = createRoot(container);
|
|
|
|
await act(async () => {
|
|
root.render(
|
|
<JsonSchemaForm
|
|
schema={{
|
|
type: "object",
|
|
properties: {
|
|
apiKey: {
|
|
type: "string",
|
|
format: "secret-ref",
|
|
},
|
|
},
|
|
}}
|
|
values={{ apiKey: "raw-value" }}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
});
|
|
|
|
const input = container.querySelector<HTMLInputElement>(
|
|
'input[type="password"]',
|
|
);
|
|
expect(input).not.toBeNull();
|
|
expect(input?.value).toBe("raw-value");
|
|
|
|
await act(async () => {
|
|
root.unmount();
|
|
});
|
|
});
|
|
});
|