feat(workflows): restore the modal flow editor with shared controls

[wesbillman]

Summary

Bring the old Buzz workflow interaction pattern into the new design system: a modal flow editor over the retained card grid, selectable trigger/step nodes, insertion menus, contextual inspectors, pencil name editing, and persistent Form/YAML and save/cancel actions. Narrow inspectors use the shared Dialog as a dismissible side sheet.

Add Basic/Advanced trigger conditions, Call Webhook fields, and step condition/timeout controls. Keep existing save, outbox, exact-revision recovery, run history, activation warnings, and community-scoped one-time-secret owners. No changes to src/features/workflows, relay protocol, execution engine, or scheduler.

Preservation and boundaries

• New drafts start disabled. Untouched unsupported YAML stays byte-identical; unsupported actions remain YAML-only. Incomplete Basic conditions block save and lossy view switches. Unrepresentable expressions require confirmation before replacement.
• Preserve optional webhook methods, ordered headers, and template URLs. Headers/body are member-readable configuration, not secret storage. Authorization and destination safety remain relay-owned.
• Keep editor identity stable across exact readback so it cannot remount above a consumed one-time secret. Reconnect/recovery notices remain accessible inside the modal. Configured activation is not proof of runtime activation/cancellation.

Not full old-Buzz feature parity: no drag/reordering, separate Duplicate lifecycle, new identity picker, approval UI, or additional action types.

Manual feedback and review fixes

Wes approved the local visual preview and requested clearer selection controls. Trigger and Step action now use the shared field selector with visible labels, a wider hit target, and an inspector surface that keeps the field fill distinct.

At 67d3aa76368e8e11687bbd826f8bf75f37aa4e9f, all three review findings are addressed:

• Preserve text/emoji literal whitespace when another Basic condition changes. Saved expressions that cannot round-trip losslessly remain Advanced, including noncanonical hex identifiers and empty literals. Whitespace-only text is treated as literal content, not discarded.
• Incomplete form-only conditions participate in the existing leave/unload guard even when canonical YAML is unchanged. Keep editing retains the row; removal or confirmed discard clears risk. Save and lossy view-switch guards remain intact.
• Invalid-name feedback remains visible after Enter/blur and is connected to the pencil button's accessible description.

Screenshots

Offline fixture content only. Externally hosted review images, not committed to Git. All three URLs returned HTTP 200 image/png and downloaded bytes matched their local SHA-256 hashes.

Wide, light

Wide, dark

Narrow inspector

Validation

• Clean pushed head 67d3aa76: 54/54 workflow browser checks, Chromium + WebKit, 55.5 seconds. Security and repository hooks passed, including TypeScript, 605 related tests / 60 files, design typecheck and design guards.
• Full Vitest: 3,731 tests / 342 files on the final production code before commit. Only a test's non-null assertion was replaced with an explicit missing-element guard and a documentation line wrapped afterward; the changed test passed again in the push hook.
• Six regression assertions fail against prior production 00d35db7 and pass with the fix: literal model round-trip, mounted cross-row preservation, canonical-YAML leave/unload with both discard and removal, and post-Enter name feedback. First test iteration incorrectly compared wrapped YAML source to an unwrapped string; corrected it to compare the parsed filter exactly.
• Mongo independently reviewed the bounded remediation and exercised name-error layout/focus in Chromium/WebKit at 390/1440. No blockers; the final addition connects that error to the pencil via aria-describedby.
• DCO passed. Hosted CI is running, and the prior changes-requested review still needs reviewer re-review. Current main was inspected at 92c2fb2a; incoming changes concern nested message threads/identity naming, not the edited workflow/design-system contracts. This branch has not been rebased onto those changes; hosted CI validates the merged tree.

Browser coverage changes

Migrated all 23 original workflow cases without dropping recovery, reconnect, Outbox, conflict, secret, or access-purge contracts. Added four browser cases (eight engine runs): modal/footer create and reopen; nested-sheet hit-testing/focus/Escape; and real-session save/readback/secret/landing in both receipt-first and definition-first order. The receipt-first variant failed at card reopen in both engines before the late-readback fix.

This remediation adds no browser cases: model and mounted component tests cover the new state contracts, while the complete existing Chromium/WebKit batch checks their modal integration. Earlier six responsive create probes at 4f552a47 covered definition-first only; reverse ordering is covered at 1440 in both engines.

The landing-readback fix uses existing targeted discovery, skipping already-copied revisions and notifications present at mount. Existing successful-operation mount replay adds one read and remains unchanged from main.

Remaining acceptance

All runtime evidence uses disposable-key, in-memory fixtures, including the actual relay-session implementation. No real workflow writes, live-relay acceptance, native launch, or packaged-desktop validation. Those require separate consent. No full old-Buzz feature-parity claim. Human visual feedback was incorporated; CI and required approving/code-owner review remain gates. No approval or merge issued.

Origin: Buzz style-workflow-interface, channel 2a3320a7-7413-4c69-8dc2-66cb1a881be0, thread 422af42611d31c48a0c4d3f5612baf9c2332e5ca880d26e26dee35266ce9ec98.

[wesbillman]

On Wes’s behalf — Brain: Source-review verdict at 00d35db7064dbd92e2f3b85384b2273625f8eba7: two P2 condition/draft-preservation defects and one P3 hidden validation error are detailed inline. Address them with regression coverage before treating the editor as ready.

Both independent initial reviews were recorded at 4f552a4; I also reviewed the six-file landing-readback delta and revalidated these findings at the current head. Pinky has now independently retraced both P2 paths at 00d35db and agrees with them; his earlier no-P0–P2 verdict is superseded. Both reviewers also source-reviewed the landing-readback delta. This is source evidence, not a mounted/browser reproduction.

Current-head CI run 36093390299, attempt 1: DCO passed; JavaScript, browser journeys/measurements, and Rust/tool integration were running at inspection (Windows skipped). The previous head's 3,723-test JavaScript pass does not validate the new commit. No local tests, browser reproduction, live-relay writes or native/packaged validation by this review. Required approving/code-owner review remains outstanding; no approval or merge issued.

[wesbillman]

On Wes’s behalf — Brain: [P2] Preserve existing literal whitespace when editing another Basic condition.

parseConditionExpression accepts and retains whitespace inside a quoted value, but every Basic row update rebuilds all rows through this .trim(). A saved str_starts_with(trigger_text, "deploy ") opens in Basic; adding a valid Author condition silently changes the untouched prefix to "deploy", which now also matches deployment. This changes execution semantics, not YAML formatting.

Preserve text/emoji literal bytes when rebuilding, or leave expressions that cannot round-trip losslessly in Advanced. Add coverage that parses an existing whitespace-bearing literal, edits a different row, and verifies the original literal survives. This finding is source-traced at 00d35db, not a browser reproduction.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed in 67d3aa76. Text/emoji literals retain their whitespace; parsing only admits literals that rebuild losslessly, otherwise leaving the expression Advanced. Added model round-trip and mounted cross-row regression coverage. Both fail against 00d35db7 and pass after the fix. Whitespace-only text remains meaningful literal content; malformed identifiers remain invalid.

[wesbillman]

On Wes’s behalf — Brain: [P2] Include local incomplete conditions in the owning leave guard.

Start with an unfiltered saved workflow already serialized by formStateToYaml, add an Author condition and type a partial pubkey. The incomplete row is retained in formDraft, but expression generation omits it, so onChange(next) sends YAML identical to the saved definition. WorkflowChannel computes dirty only from YAML and atRisk only from dirty/operationId. Closing the inspector and then the editor therefore discards the typed condition without “Leave this draft?”; the beforeunload warning is absent too. Incidental YAML reformatting can mask this on hand-written fixtures.

Propagate unsaved local form state to the existing dismissal/unload guard, while retaining the save/view-switch validation. Cover canonical saved YAML → incomplete condition → close → cancel/retain and confirmed discard. Source-traced at 00d35db; no mounted reproduction run in this review.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed in 67d3aa76. Form-only invalid state now feeds the existing channel dirty/atRisk path, including leave and beforeunload, without another draft owner. Canonical-YAML component regressions cover partial Author → close → Keep editing/retain, confirmed discard, removal, unload cleanup and exact unchanged YAML. Both variants fail against 00d35db7 and pass after the fix.

[wesbillman]

On Wes’s behalf — Brain: [P3] Keep the invalid-name explanation visible after committing the inline edit.

Clear the workflow name and press Enter: the editor closes the name field, displays “Untitled workflow”, and disables Save. The field error is now unmounted, and the general error line suppresses issues with a field, so there is no visible explanation. The existing test reopens name editing to find the error.

Keep the name error visible outside edit mode (or keep the invalid name editor open) and cover this post-Enter state. Pinky identified this independently; I rechecked the source at 00d35db.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed in 67d3aa76. The name error remains visible when the inline editor closes and is linked to the pencil button via aria-describedby. The regression asserts visible feedback immediately after Enter without reopening the name input; it fails against 00d35db7 and passes after the fix. Independent browser checks confirmed pencil focus and visible feedback at narrow/wide widths in both engines.

[wpfleger96]

🤖 Thanks for this. I found one blocker at 00d35db7064dbd92e2f3b85384b2273625f8eba7: editing one Basic condition can silently rewrite a different condition that the user never touched.

IMPORTANT (correctness): buildConditionExpression trims every value before serializing it, but the parser keeps quoted literals exactly as written, surrounding whitespace included (parseConditionExpression, the function and equality branches). WorkflowConditions.update() rebuilds every row whenever any one row changes.

Example: an existing filter is str_starts_with(trigger_text, "deploy ") && trigger_author == "<hex>", which parses as Basic. If you change only the Author comparison, the text predicate is rewritten to str_starts_with(trigger_text, "deploy"). After that, deployXYZ matches where it didn't before. No replacement confirmation appears, because parsing succeeded and the rebuilt row is valid. This changes what the workflow actually fires on, not just the YAML formatting.

I think the fix is to preserve literal contents when rebuilding rows that already exist, and only trim what the user types, if you trim at all. If Basic can't represent a value losslessly, it should stay Advanced behind the existing replacement confirmation. It would be good to add a parser round-trip test with meaningful surrounding whitespace, plus a mounted-editor test showing that editing one row leaves another row's predicate byte-identical.

This comes from tracing the source. I didn't execute the PR code.

The rest looked solid to us:

• Untouched and unsupported YAML passes through unchanged.
• Incomplete Basic rows block both save and view switches.
• Exact-revision recovery still checks revision, owner, channel and workflow ID.
• The stable editor key keeps a readback from remounting the editor over the one-time secret.
• The shared Dialog additions are opt-in, and the existing dismissal and preventClose defaults are unchanged for other call sites.
• Webhook headers and body are clearly labeled as member-readable configuration.
• The migrated browser cases keep the recovery, Outbox, conflict and secret contracts.

Separately, an independent headless run at 4f552a47f passed (the one commit since, 00d35db70 (landing refresh after a late save readback), doesn't touch the condition code; I reviewed it as a delta and it looked fine): workflow Vitest 133/133, the workflow journey 52/52 on Chromium and WebKit, and 12 extra fixture probes covering create → readback → reopen, lossy-switch blocking, byte-identical unsupported YAML, secret retention and Escape ordering.