fix(config): /config reports what is actually in paddock.config.yaml, and null clears instead of zeroing (#722, #723)

[edspencer]

Fixes #722. Fixes #723.

#722 — three symptoms, one root cause

buildInstanceConfig built the GET response out of the boot-frozen
PaddockConfig and never read paddock.config.yaml. So a GET /api/instance-config could not observe any write — not another tab's, not even
one the same client had made a millisecond earlier. All three reported defects
fall out of that:

1. A successful save visibly un-did itself. save() re-fetches and clears
   edits; the re-fetch returned the pre-save values, so setting OVERVIEW.md
   max tokens to 1234 wrote 1234 to disk, showed a green "Saved to disk",
   and put 2000 back in the box.
2. Two tabs silently last-writer-won, with nothing — not polling, not an
   ETag — able to reveal it, because the value a client would have polled for was
   never read from the file.
3. restartRequired was hardcoded false, because there was nothing to
   compare against.

So it is one fix, as the issue suggested. Each field now reports two values:

field	meaning
value	in force now — what the running process resolved at boot
pendingValue	pending — what the file says this instant, i.e. what a restart would load
pendingRestart	those two differ

restartRequired is now pendingRestart over all fields rather than an
assertion, and the response carries a configVersion (a fingerprint of the file)
and a configFileError for a file that exists but won't parse — the screen that
exists to fix a broken config should say it is broken rather than report
"nothing pending".

The editor binds to pendingValue. That one substitution is what makes the
screen honest: it is an editor for the file, so a save round-trips, another
tab's write shows up on the next load, and where the two disagree the field shows
an In force now: line and the banner says a restart is outstanding.

Pending values are computed for editable, non-env-shadowed fields only: an
env-shadowed field resolves to the same env value after a restart, and the
read-only advanced bindings are normalised at boot (paths canonicalised, port
Number()-ed), so comparing them against raw file text would manufacture
divergence that isn't there. A field the frozen config leaves unset is compared
as its documented default, so e.g. models — unset, therefore null — doesn't
read as permanently diverging from the catalog list its absence implies.

Two tabs: visible and refused

Rendering the file makes a concurrent write visible on the next load, but a
tab that already has the page open would still clobber the other one. So a save
is now conditional: the UI echoes the configVersion it read back as
expectedVersion, and a write composed against a stale snapshot gets a 409
with the file left alone. The client keeps the operator's edits, reloads so they
can see what the other writer did, and lets them save again deliberately.
expectedVersion is optional, so curl and scripts write unconditionally as
before.

#723 — null on a numeric field cleared, not zeroed

The PUT contract is that a null deletes the key. nonNegInt used
Number(raw), and Number(null) === 0 is a finite non-negative integer, so
{"recovery.maxRetries": null} wrote 0 — recovery stops retrying entirely.
The validator now has the same explicit null / "" / undefined branch as its
optNonNegNumber sibling, and a deliberate 0 still works.

maxSpawnDepth had the identical hole in its own validator (a "restore the
default" wrote depth 0, which takes the self-MCP away from every child) and is
fixed the same way.

The same missing type check let Number()'s other coercions through —
overviewMaxTokens: true wrote 1, [7] wrote 7, debounceMs: false wrote
0. Numeric fields now accept a number or a numeric string and nothing else.

Two smaller holes from the same audit

• Env-shadowed fields were writable via the API. The UI already rendered them
  read-only, but validatePatch didn't check, so a PUT returned 200 +
  restartRequired: true for a write that could never take effect. Now a 400
  naming the variable. (Tradeoff: you can no longer stage a file value in
  preparation for removing the env var — an option the UI never offered anyway.)
• No upper bound on numeric or string fields. A 200 KB brand.name produced
  a 200 KB paddock.config.yaml for every boot to parse. Numbers cap at 1e9,
  plain strings at 1024 characters, list fields at 64 entries; environmentPrompt
  keeps its own 32 KiB cap.

Nothing that worked was weakened

The existing patch validation is genuinely strong and is untouched: negative /
zero / fractional budgets, "abc", bad and wrong-case enums, unknown keys,
read-only keys, unknown model ids, an empty model list, non-hex accents, NUL
bytes and oversized prompts are all still rejected, __proto__ is still inert,
and the file still round-trips through the yaml Document API with operator
comments and unmanaged keys intact. The client's dirty-tracking still sends only
genuinely-changed keys — it just compares against the file now instead of the
boot config.

Tests

All new tests were confirmed failing on main (source reverted, tests kept):

• Integration (instance-config.test.ts, the load-bearing one — this file
  covered the PUT but never asserted what a subsequent GET reported, which is
  exactly why /config: a successful save reverts the form to stale values, two tabs silently last-writer-wins, and restartRequired is hardcoded false #722 shipped): a write is visible to the next GET as
  pendingValue/pendingRestart; a second client sees the first's write and a
  stale conditional write is refused with the file intact; null clears the three
  recovery keys; env-shadowed and oversized writes are refused. 5 fail on main.
• Unit: PUT /api/instance-config: a null on a nonNegInt field writes 0 instead of clearing the override #723's clear-vs-zero and the type checks; pending/effective
  computation including the cases that must not report divergence; a malformed
  file. 11 fail on main.
• Web: the saved value survives the post-save re-fetch; the pending-restart
  banner; the 409 path keeps the edits and re-reads. 4 fail on main.
• E2E: /config now actually saves — no test anywhere did — asserts the
  value is still on screen after the re-fetch and after a reload, then restores
  the instance's original value so the shared server is left as found. Verified
  failing against a main build.

Full suite green locally (server + web), npm run typecheck clean.

One thing this cannot cover: I could not restart a server mid-test either, so
"the pending value becomes the effective one after a restart" is asserted
through the loader (writeInstanceConfig → loadConfigFile round-trip) rather
than through a real reboot. The two failure modes that worried the reporter —
a zeroed config and a bloated one — can no longer be written through this API
at all.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying paddock with    Cloudflare Pages

Latest commit:	79f195b
Status:	✅  Deploy successful!
Preview URL:	https://9c28e351.paddock-7u2.pages.dev
Branch Preview URL:	https://fix-instance-config-pending.paddock-7u2.pages.dev

View logs