UX-179 - rpcn: save pipelines as drafts

[SpicyPete]

Summary

Save no longer means deploy. A pipeline can be saved as a draft: stored as written, never validated, never running, and started later once it lints. Around that the editor gets crash recovery and a way to see what a save will change.

• Proto (dataplane/v1/pipeline.proto, additive): STATE_DRAFT, PipelineCreate.draft, PipelineUpdate.draft, list filters states / include_drafts, and output-only created_by, create_time, update_time. Drafts stay out of ListPipelines unless asked for, so older clients never see a state they can't render.
• Editor: the primary button is Save draft for new pipelines and drafts, Save for stopped ones, Apply and restart for running ones. Save and start / Save and stop live in the split menu. Starting a draft validates it first; if that fails, the editor opens with the lint hints and the draft is untouched.
• Unsaved changes lane: a diff of saved vs edited config next to a list of the components and settings that changed.
• Autosave: edits mirror to localStorage, so a crash or a closed tab doesn't lose work. The next visit offers to restore them and warns if someone saved the pipeline in the meantime.
• List: a Drafts tab, a Draft badge with who edited it and when, and row actions to continue editing, start, or delete a draft.

The reasoning behind the design choices is in frontend/specs/rp-connect-pipeline-drafts.md.

Rollout

Behind enable-rpcn-pipeline-drafts, off by default here and in cloud-ui. The service side is redpanda-data/cloudv2#29544.

Deploy order doesn't matter while the flag is off. Once it's on, every hop (console, console-enterprise, redpanda-connect-api) has to be new: an old one drops draft and deploys the pipeline for real. The UI checks the state that comes back and tells the user if that happened. Flip the flag last.

backend/pkg/protogen and proto/gen/openapi are regenerated because console-enterprise proxies this service through the Go types and would otherwise strip the new fields.

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf CI / validate (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Sep 4, 2026, 1:59 PM

[github-actions]

✅ Clean — no registry drift, off-token colours, or ad-hoc classes

App: frontend · Scope: diff vs origin/master · Files: 33

	Count
⚠️ Outdated registry components	0
🛠 Locally-modified components	0
❓ Unknown to registry	0
🎨 Off-token palette colours	0
🔢 Ad-hoc utility classes	0

Generated by lookout audit-changes.

[eblairmckee]

Adversarial review — findings

Went through this looking for what breaks rather than whether it reads well. Findings only, HIGH → LOW. A lot of this is careful work — the lane-switch commit-before-unmount, revealing by store request instead of poking a disposed editor, the deliberate choice to compare server clocks only for staleness, the exhaustive Record that breaks the build on a new state. What follows is concentrated in two untested files and one theme file.

---

HIGH

H1. basedOnUpdateTime is re-read live, so the "someone else saved" warning is defeated by continuing to type

Where: use-editor-autosave.ts:44-46, 64 and index.tsx:1367-1371

useEditorAutosave refreshes savedUpdateTimeRef.current = savedUpdateTime on every render, and write() stamps basedOnUpdateTime from that ref. The pipeline query polls. isAutosaveStale then compares savedUpdateTime !== recoverableEntry.basedOnUpdateTime.

Failure mode: you open a draft at update_time = T1 and start editing. A colleague saves; update_time becomes T2. The poll lands, savedUpdateTime becomes T2, and your next keystroke fires the debounce and rewrites the buffer with basedOnUpdateTime = T2. The buffer has silently adopted a baseline you never saw. isAutosaveStale is now false, so AutosaveRestoreNotice renders the reassuring informative copy ("You left this editor without saving these edits") instead of the warning copy ("This pipeline has been saved by someone since you were editing") — and restoring overwrites the colleague's config with edits based on the pre-T2 state.

Quantified risk: the window is the whole editing session, not a moment — any poll landing while the user types destroys the signal. Probability that a concurrent save is missed rather than caught is close to 1 for an actively-edited draft. So the check works in a test and not in use, and the impact is silent loss of a colleague's saved configuration, which is the exact failure this mechanism exists to prevent.

Recommendation: pin the baseline once when the editor hydrates, next to capturedTargetRef / initialYaml, and keep it for the life of that target. basedOnUpdateTime should answer "what was on screen when I started" — immutable for the session — not "what does the server say right now", which is the thing you're trying to detect a change in.

use-editor-autosave.ts is also the one file here with no test, which is why nothing caught this.

H2. The Unsaved-changes diff is hardcoded light-theme, bypassing the registry mechanism that already solves this

Where: changes-diff-theme.ts

defineDiffTheme calls monaco.editor.defineTheme with base: 'vs' and literal light hex values (#cd372c, #25855a, #c3c4c6), with the comment "Light-theme hex copies of the semantic tokens (Monaco can't read CSS vars)". That premise is already false in this repo — components/redpanda-ui/lib/editor-theme.tsx resolves design tokens by painting a probe element and types base: 'vs' | 'vs-dark', and components/ui/yaml/yaml-editor.tsx re-resolves on a theme flip, its own comment reading "defineTheme is global, so re-resolving on a flip re-themes every mounted editor at once." The new panel opts out of all of it.

Failure mode: in dark mode the diff renders a vs-based (light) theme with editor.background: transparent over the app's dark surface. inherit: true on a light base gives dark syntax colors on a dark ground, and the change highlights are 8%-alpha light red/green — removedLineBackground at #cd372c14 over a near-black surface is essentially invisible. Seeing what a save will change is the lane's entire purpose.

Quantified risk: deterministic, not probabilistic — wrong for 100% of dark-mode sessions. changes-diff-theme.test.ts tests the withAlpha hex arithmetic, so it passes green while the defect stands; that's a test asserting an implementation detail instead of the behavior.

Recommendation: build the diff colors through editorTheme() so they come from the tokens, select vs / vs-dark from the resolved theme, and re-resolve on flip the way yaml-editor.tsx does. Then have the test assert that a dark theme yields a dark base.

---

MEDIUM

M1. The mixed-version guard is implemented on create only

Where: index.tsx:390-392 (draftWasIgnored) vs :429-451

On create the code trusts the response state — isDraftSave && createdPipeline.state !== Pipeline_State.DRAFT → DRAFT_UNSUPPORTED_MESSAGE. On update it relies on the server refusing the draft: true assertion. But a pre-drafts hop drops the unknown draft field, so no assertion reaches the server and nothing is refused.

Failure mode: an old redpanda-connect-api handling an update to an existing draft lints the config, and its buildPipeline rebuilds metadata.annotations from scratch — discarding connect.redpanda.com/draft, which it has never heard of. Its mapState reads spec.paused: true and reports STATE_STOPPED, so the write stores paused: true. The draft silently becomes an ordinary stopped pipeline while the UI reports "Draft saved". Not catastrophic — it stays paused, and the config had to lint to get there — but the state the user thinks they're in is gone, and the PR body's "The UI checks the state that comes back and tells the user if that happened" only holds for create.

Recommendation: apply the same check on the update path. response.response?.pipeline is already in hand and currently used only for warnIfResized.

M2. The "N issues to fix" pill and the Start gate are different linters

Where: index.tsx:1546 (draftIssueCount={Object.keys(lintHints).length}), pipeline-header.tsx:414

lintHints comes from usePipelineLint — the browser's linter. Starting a draft is gated by the server's LintYAML inside StartPipeline. Different implementations, potentially different Connect versions, different visibility into cloud schema restrictions and secret resolution.

So a draft header can read "no issues" and Start pipeline is refused with lint hints, or read "2 issues to fix" and start fine. The warning triangle is asserting something it can't know.

Recommendation: label it as a local check, or seed the count from the last server refusal (see M3) so the number shown is the number that gated them.

M3. Starting a draft from a list row extracts the server's lint hints, counts them, and throws them away

Where: use-start-draft.ts:37-46

toast.error(startBlockedMessage(Object.keys(extractLintHintsFromError(error)).length));
navigate({ to: '/rp-connect/$pipelineId/edit', ... });

The hints are computed purely for the count. Nothing carries them into the editor, which mounts with errorLintHints = {}.

Failure mode: the toast says "This draft has 2 issues to fix before it can start. We've opened the editor on them." The editor then highlights whatever the local linter independently finds — possibly nothing, possibly different lines. The in-editor save-and-start path does this correctly (index.tsx:454-457 calls setErrorLintHints(hints)), so the same user intent produces different quality of feedback depending on where it was clicked. This is the primary way to start a draft you weren't already editing.

Recommendation: pass the hints through router state or a small store and seed errorLintHints on mount. The toast copy is a promise — worth keeping.

M4. A failed writeAll in clear() is ignored, so a saved-away buffer can resurrect and offer to overwrite the save

Where: state/rpcn-editor-autosave.ts:143-148 and :113-117

writeAll returns false on a QuotaExceededError. save() checks it; clear() doesn't — it calls writeAll(next) then unconditionally set({ entries: next }). In-memory state says the buffer is gone; localStorage still has it.

Failure mode: save the pipeline → markSaved → clear(target) → storage write fails → next page load readAll() brings the stale buffer back → AutosaveRestoreNotice offers to restore edits you already saved, replacing the current correct config with the older one.

Reachability: pruneForCluster caps at MAX_AUTOSAVE_BUFFERS = 10 per cluster and passes every other cluster's entries through untouched:

const others = entries.filter((e) => e.clusterId !== clusterId);
return [...mine.slice(0, MAX_AUTOSAVE_BUFFERS), ...others];

Total is unbounded across clusters. At MAX_AUTOSAVE_YAML_BYTES = 256 KB, one cluster at cap is ~2.5 MB; two or three exhausts a typical 5–10 MB origin budget, which is also shared with everything else console persists there.

Recommendation: check writeAll's return in clear() and don't update in-memory state on failure; cap total entries rather than per-cluster. A per-cluster cap with an uncapped tail is the same as no cap.

M5. hasWrittenRef is not reset when the autosave target changes, defeating its own guard

Where: use-editor-autosave.ts:48, 53-56, 58

The ref exists for one stated purpose: "Only clear a buffer this editor wrote; an earlier session's must survive the load settling." It is never reset when targetKey changes — the effect's cleanup drops the pending timer and re-subscribes, write is re-created, the ref keeps its value.

Failure mode: navigate from draft A's /edit to draft B's /edit in the same mounted component (browser back/forward between two edit URLs, or a pasted link — same file route, so a param-only change re-renders rather than remounts). Hydration changes yamlContent, scheduling a write. A second later write() runs against draft B: form clean, documentChanged false, hasWrittenRef.current still true from draft A — so it calls rpcnEditorAutosave.clear(B), destroying B's recovery buffer this editor never wrote. hasStoredBuffer goes false, showAutosaveRestore requires it, and the restore notice disappears about a second after the page settles.

Loss of crash-recovery data on exactly the flow the feature exists for. The create → /edit transition is safe (separate file routes, so it remounts) and a fresh page load is safe (hasWrittenRef starts false).

Recommendation: useEffect(() => { hasWrittenRef.current = false }, [targetKey]).

[Confidence: medium — rests on TanStack Router not remounting on a param-only change within the same file route. If it does remount this is a non-issue; worth 30 seconds to confirm before acting.]

M6. Autosave silently does nothing above 256 KB, then blocks the exit that depends on it

Where: state/rpcn-editor-autosave.ts:57, 126-128; index.tsx:1381-1387

save() returns false for a config over MAX_AUTOSAVE_YAML_BYTES with no UI signal anywhere, so the user believes crash recovery is on for the whole session.

Compounded at the exit: handleLeaveAndKeepEdits treats a false flush as a hard block — "Your edits could not be kept in this browser. Save them, or discard them to leave." For a running pipeline the offered save is Apply and restart, which unsavedChangesCopy itself describes as dropping in-flight messages. So a large-config user's only two exits are a production restart or losing the work, and they find out at the moment they try to leave.

256 KB of YAML is large but not absurd for a many-component pipeline with inline schemas or mappings. The failure is silent right up to the point it's blocking.

Recommendation: surface the "too large to keep in this browser" state in the editor while it's true, not at the exit. And don't make Leave for now refuse — leaving without a buffer is worse than leaving with one, but better than a forced restart.

M7. The persisted schema has no version, and the type guard won't catch the next shape change

Where: state/rpcn-editor-autosave.ts:59-73

isAutosaveEntry validates targetKey, clusterId, name, configYaml, updatedAt, and Array.isArray(tags). Not description, not computeUnits, not basedOnUpdateTime, and not any tag element.

So { tags: [null] } or a missing computeUnits passes the guard and reaches applyAutosave → form.reset({ computeUnits: undefined, tags: [null] }) — a render crash in the tags field (tag.key on null) and a controlled→uncontrolled flip on the number input. The shape has already changed once (LEGACY_DRAFTS_STORAGE_KEY being removed on read is evidence of exactly this migration problem) with nothing recording which version wrote an entry.

Recommendation: add a version field, discard entries that don't match, validate tag elements. A structure this loose feeding straight into form.reset is the crash you get one refactor from now, on data you can't reproduce.

M8. clearAll() is wired only to console's own logout(), which the cloud deployment may never call

Where: state/backend-api.ts:483-486

The stated mitigation for storing configs verbatim — the store's comment says "Configs are stored verbatim, so a pasted credential must not outlive the week", and clearAll is documented /** For logout. */ — fires from _apiCreator's logout(), right after appConfig.fetch('./auth/logout').

Sessions that end by token expiry, tab close, browser quit, or an IdP-side sign-out never reach it. And in cloud, console is federated into cloud-ui, whose sign-out is a different code path — so on the deployment this feature actually targets, clearAll plausibly never runs. That leaves the 7-day TTL as the only eviction, and the TTL is itself only applied when readAll() runs, i.e. when something imports the module. A user who pastes a credential into a draft and then stops using RPCN keeps it in localStorage indefinitely, readable by any XSS on the console origin for as long as it sits there.

Recommendation: verify the cloud-ui sign-out path reaches this. Add eviction on visibilitychange/beforeunload or a timer so the TTL is enforced rather than opportunistic. And reconsider whether 7 days is right for verbatim configuration — "must not outlive the week" is a long week for a pasted password.

[Confidence: medium on the cloud-ui logout path — I didn't trace cloud-ui's sign-out into the federated console.]

M9. Recovery buffers are not scoped to a user

Where: state/rpcn-editor-autosave.ts:154-163 (selectAutosaveEntry), :120-121

The key is clusterId + targetKey. No user identity anywhere in the entry. On a shared browser profile, or after an account switch that doesn't go through console's logout(), user B opening the same pipeline is offered user A's unsaved configuration — including anything A pasted into it. Combined with M8, this is the concrete path by which a pasted secret reaches someone else.

Recommendation: include the user id in the target key. It also makes M8 less load-bearing.

M10. The Drafts tab can bounce you off itself while the list is still loading

Where: list.tsx:625, 709-726

draftCount is derived from the pages fetched so far, and the auto-switch effect has no loading guard:

useEffect(() => {
  if (activeTab === 'draft' && draftCount === 0) { handleTabChange('all'); }
}, [activeTab, draftCount, handleTabChange]);

isLoading and hasNextPage are both destructured in the same component and both unused here.

Failure mode: deep-link or refresh on the Drafts tab, first page contains no drafts, draftCount is 0, you're switched to All — then the drafts arrive and you're on the wrong tab with no indication why. Same effect fires transiently during any refetch that momentarily empties the set.

Recommendation: gate on !isLoading && !hasNextPage.

M11. Two independently derived SaveContext objects decide the button's label and the button's action

Where: index.tsx:1134-1137 vs pipeline-header.tsx:429

index.tsx builds saveContext and hands it to usePipelineSave, where handleSave() with no intent computes primaryRunIntent(saveContext). PipelineEditHeader separately builds const context: SaveContext = { mode, state: pipelineState, draftsEnabled } and calls primaryRunIntent(context) to label the button, plus saveRunHint and runIntentLabel.

They agree today because the inputs are the same. The failure mode when they diverge is specific and bad: the button says Save draft while handleSave computes keep or stopped and deploys. Note index.tsx normalizes mode === 'create' ? 'create' : 'edit' while the header receives mode directly — the normalization exists in one place and not the other, which is how these drift.

Recommendation: pass the single saveContext down. The header already takes three props to reconstruct it; one prop replaces them.

M12. relativeAgeLabel mixes clock domains at both display sites

Where: draft-copy.ts:51-57; called with server timestamps at list.tsx:491 and pipeline-header.tsx:302

relativeAgeLabel computes Date.now() - at — browser clock. Both display sites feed it timestampToMillis(pipeline.updateTime) — server clock. index.tsx was explicitly careful about this for staleness ("Both sides are the dataplane's update_time, so clock skew can't affect this"); these two aren't, and the helper's doc comment says nothing about which domain it expects.

A browser clock ahead by hours renders "5 hours ago" on a draft saved a second ago. A clock behind gives a negative elapsed, which lands in the < 60_000 branch and shows "just now" — benign by luck, not design.

Recommendation: compute display age from a server-provided "now", or type the helper so browser-clock and server-clock values can't be passed interchangeably. The autosave notice's use of it is correct; only these two are wrong, which is what makes it worth typing rather than commenting.

M13. The weakest delete confirmation is on the least recoverable artifact

Where: delete-draft-dialog.tsx:28, list.tsx:415-422

DeleteDraftDialog drops the type-to-confirm that DeleteResourceAlertDialog requires, justified as "Lighter than DeleteResourceAlertDialog: no type-to-confirm, since nothing is deployed."

That rationale reasons about operational risk and skips data risk. A deployed pipeline's config is recoverable — it's readable from the running resource. A draft is the only copy of work that may represent hours of editing, and it's now two clicks from a row dropdown, described in the dialog's own copy as "deleted for everyone" and "This can't be undone." The confirmation strength is inverted relative to what's actually losable.

Separately: list.tsx renders DeleteDraftDialog without the hasUnsavedChanges prop, so deleting a draft from the list never shows "Your unsaved changes go with it" — even when a localStorage buffer exists for that draft, which handleDelete then clears.

Recommendation: pass hasUnsavedChanges from the list (rpcnEditorAutosave.get(id) !== null). Reconsider the asymmetry — at minimum keep type-to-confirm for a draft with an unsaved buffer.

M14. nextUntitledName() puts an un-spinnered network call in the save path and samples only the first 100 names

Where: index.tsx:310-316, react-query/api/pipeline.tsx:154-181, draft-copy.ts:38-49

Three small things that compound:

• nextUntitledName() awaits a ListPipelines call inside handleSave before any mutation starts, so isSaving (derived from mutation pending states) is still false. Clicking Save draft on an unnamed pipeline shows no feedback while it runs.
• NAME_LOOKUP_PAGE_SIZE = 100 with no pagination, so past 100 matching names the generated "Untitled pipeline N" can collide.
• The server's name_contains is a case-sensitive strings.Contains; untitledPipelineName lowercases for comparison. The two disagree about what's taken.

display_name isn't unique-constrained so a collision is cosmetic, but the un-spinnered await is a real perceived-latency bug on the feature's primary button.

Recommendation: start the name lookup optimistically when the editor opens, or set a pending state around it.

---

LOW

• use-start-draft.ts:41 — params: { pipelineId: encodeURIComponent(pipelineId) } double-encodes; TanStack Router encodes path params itself. Harmless for k8s-shaped ids, wrong in principle, and it bites whoever relaxes the id format.
• ui/pipeline/constants.ts:35 — PIPELINE_STATE_STATUS_VARIANT is an exhaustive Record<Pipeline_State, …>, so a new proto state breaks the build. Good. PIPELINE_STATE_LABELS immediately above stays Partial<Record<…>>, so a new state silently renders "Unknown". Worth making consistent while you're in the file.
• Two notions of "startable" — STARTABLE_STATES includes STOPPING ("so a row menu can rescue a stuck stop") and save-actions.ts:81 re-excludes it locally. Pre-existing constant, new workaround; two definitions of one predicate in the same feature.
• list.tsx:489-496 — draft rows replace the id line with "Edited … by …", so a draft's id can't be read or select-all-copied from the list. The non-draft row's select-all affordance is a deliberate nicety being dropped for drafts specifically.
• draft-copy.ts:15 — areDraftsEnabled() is a plain call at render, not a hook, so a live flag flip needs a reload. Consistent with the rest of isFeatureFlagEnabled usage; noting it because this flag is the rollout control and someone will flip it expecting it to take.
• pipeline-header.tsx:403 — onSave: (intent?: SaveIntent) => void receives an async handleSave. Anything thrown outside its internal try/catch (pendingEditCommit?.(), form.trigger()) becomes an unhandled rejection rather than a toast.
• proto/gen/openapi/openapi.json is a single minified line, so its diff is -1/+1 and unreviewable while openapi.yaml shows +98. Worth confirming both came from the same generator run rather than assuming it.

---

Two candidates I chased and dropped, recorded so nobody re-derives them:

• Monaco instance leak from the new lane. ChangesPanel mounts a DiffEditor, but isEditChangesLane also unmounts EditorPanel, so only one Monaco lives at a time, and onEditorMount's cleanup calls setEditorInstance(null). goToYamlNode reveals through a store request consumed on mount rather than calling into a disposed editor. Clean.
• XSS via created_by or draft names. All rendered as text children; React escapes. toNameContainsFilter even strips characters the server's ^[A-Za-z0-9-_ /]+$ would reject rather than eating a 400. No injection surface found.

[SpicyPete]

Thanks for the review, addressed/fixed most feedback.
M8 and M9 require changes to how cloud-ui does auth, so I think these edge cases are okay for now.
M13 is deliberate, a draft isn't running, so I think the delete dialog should be easier to use (still 2 step deletion)

For M12 this will require a backend change, and this branch is following the convention used elsewhere in the app. In utils/tsx-utils.tsx computes exactly Date.now() - serverTs and is used across topics, brokers. The branch matched the existing convention. We could do a follow up to fix this throughout, but it is a bit of an edge case.

[malinskibeniamin]

can you have a draft that's not technically valid YAML so that you can leave it and fix it before it can move to a non-draft state?

[SpicyPete]

Yes, you can save invalid YAML as a draft