validate a standing unattended render route when it is saved, not on every job it fails - #4823
Merged
Conversation
…nstead of on every job (#4348) Instances -> Unattended render routing saved whatever it was given. A route naming a peer that is unknown, switched off, not enabled as a media provider, not allowlisted for that exact engine/model pair, or reachable outside the tailnet is not a transient capacity problem -- it can never run -- and unattended work has no human at the moment it fails, so it broke every future Creative Director / Creative Commission render of that kind in silence. PUT /api/settings now validates federation.mediaRouting against the durable configuration and refuses it with the same typed reason the enqueue would have given later. Live capacity is deliberately NOT a save-time gate: a provider is routinely asleep when its route is configured. Clearing a route is always allowed, so a bad configuration can never become permanent. The card itself stops being the one surface that re-derives all this by hand: it now reads the shared readiness lib the Instances peer card, System Health and the interactive pickers use, gains the routed peer's readiness and queue occupancy as a caption, and surfaces the server's refusal message instead of a generic failure. Its inline copy of the tailnet predicate -- which range-checked only the second octet, so 100.64.999.1 read as tailnet in the browser and as public space on the server -- is replaced by client/src/lib/tailnetPeer.js, a faithful port carrying the server suite's own test table.
The two addresses carry an octet > 255, so they are provably not any real peer — but they sit inside 100.64.0.0/10, which is what the identity-leak guard scans for. Same treatment the existing top-of-range fixture already gets.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The peer picker this run was dispatched for shipped in #4819, so this slice takes the one gap in "Proposed approach" item 6 that is neither ADR-gated nor already decided against: provider/model selection and capacity messaging on the Creative Commission flow — Instances → Unattended render routing, the surface left behind when the interactive ones were consolidated.
A standing route is now validated where it is SAVED, not only where it is used.
federatedMediaRoutingSchemaproves a route is well-shaped and nothing more, so a route naming a peer that is unknown, switched off, not enabled as a media provider, not allowlisted for that exact engine/model pair, or reachable outside the tailnet saved happily and then failed every future Creative Director / Creative Commission render of that kind. None of those is transient capacity — such a route can never run — and unattended work has no human at the moment it fails, so it broke in silence.PUT /api/settingsnow refuses it with the same typed reason the enqueue would have given (MEDIA_PROVIDER_PEER_NOT_FOUND/_PEER_DISABLED/_NOT_CONFIGURED/_MODEL_NOT_ALLOWED/MEDIA_ROUTING_PEER_NOT_TAILNET), rather than two vocabularies for one misconfiguration.Two boundaries are deliberate:
The card stops being the surface that re-derives all this by hand. It now reads the shared
federatedMediaReadinesshelpers the Instances peer card, System Health, and the interactive pickers use, so it cannot become a fourth opinion; it gains the routed peer's readiness and shared-queue occupancy as a caption (item 6's "capacity messaging", wired to the select witharia-describedby); and it surfaces the server's refusal message instead of a generic failure, which is the useful half now that a refusal names a fixable cause.Its inline copy of the tailnet predicate is replaced by
client/src/lib/tailnetPeer.js, a faithful port ofserver/lib/tailnetPeer.jscarrying that suite's own test table. The inline copy had drifted: it range-checked only the second octet, so an address like100.64.999.1read as tailnet in the browser and as ordinary public space on the server — the picker would have offered a peer the server refuses. The server stays authoritative either way; the port only lets the picker explain an absent option.Also moved: the route's shape (
ROUTABLE_MEDIA_KINDS,sanitizeRoute,normalizeMediaRoutingConfig) out ofdefaultRouting.js— which owns enqueue-time resolution — into the newroutingPolicy.js, re-exported so existing callers keep one import site.modelKeyis now exported fromfederatedMediaConsumer.jsso the save gate and the submit gate cannot disagree about what "allowlisted" means.Test plan
cd server && npm test— 33002 passed, 19 skipped, 0 failed.cd client && npm test— 9423 passed, 0 failed.cd client && npm run lint— clean.server/services/federatedMedia/routingPolicy.test.js(20 cases): the accept path; each refusal with its code and status; engine/model treated as a pair, not a model id; every kind in a patch checked, not just the first; clearing allowed with an empty registry; a currently-offline peer and a peer with an expired snapshot both still savable.client/src/lib/tailnetPeer.test.js: the server suite's table case for case, plus the malformed-CGNAT cases the old inline copy accepted.server/routes/settings.test.js: route-level accept/refuse/clear, with the peer registry mocked (the policy loads it lazily, so an unmocked run would read the developer's owndata/instances.json).client/.../UnattendedRenderRouting.test.jsx: readiness + queue caption for a routed kind, an expired snapshot read asstalerather than repeating its ownready, an unregistered routed peer, no caption for a locally-rendered kind, and the server's reason surfaced on a refused save with a generic fallback when none is carried.agyreview caught a real CI failure this branch would otherwise have shipped: two malformed synthetic addresses in the new test table sit inside100.64.0.0/10and trippedscripts/tailnet-identity-leak.test.js. Fixed in the second commit by allowlisting them (they carry an octet > 255, so they are provably not any real address) — the same treatment the existing top-of-range fixture already gets. That guard scopes to git-tracked files, so it only goes red once the files are committed.Remaining
Still open on #4348, unchanged by this PR:
MEDIA_PROVIDER_INPUT_UNSUPPORTED. ADR-gated:docs/decisions/2026-08-20-federated-visual-prompts.mdsays widening the carve-out "requires a new ADR, not a reading of this one".Refs #4348