Skip to content

feat(web): yes/no condition nodes on the campaign canvas - #245

Draft
Ahmustufa wants to merge 9 commits into
mainfrom
feature/canvas-condition-nodes
Draft

feat(web): yes/no condition nodes on the campaign canvas#245
Ahmustufa wants to merge 9 commits into
mainfrom
feature/canvas-condition-nodes

Conversation

@Ahmustufa

@Ahmustufa Ahmustufa commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Draft. Merge order: #242 (branching) → #248 (branch 409 check) → this. This branch is rebuilt on main + #242 + #248 plus its own three commits. After those two merge, its diff is only the condition-node work. It targets main directly (no stacking).

Phase 2 of the campaign canvas: the branching API, in the UI.

  • Add a condition after any step (running campaigns included). It draws as a diamond with Yes/No exits, the condition in words, wait chips, and a Stop per empty exit.
  • Condition editor, which mirrors the server's rules exactly:
    • within_days is 1–90, and "always" has a single exit;
    • only non-stopping reply labels are offered, with the reason why;
    • open/click conditions need tracking and an HTML body;
    • an exit can't point at its own step.
  • Drag a Yes/No handle onto a step to set that exit. It is pre-validated, and a problem opens the editor instead of sending.
  • Concurrency:
  • Loop refused (422): the looping steps are highlighted. Unreachable steps are labelled. If the graph fails to load, it falls back to the linear view, and condition editing and reorder are held.
  • Layout: edges follow dagre bend points, and Yes stays left of No through DFS insertion order. That is pinned by a two-condition / converging / back-jump test. The dead relay and constraint code is removed.

Verification (on the rebuilt branch)

  • oxlint 1.83, tsc -b and build pass.
  • vitest: 165 files / 1823 tests.
  • Playwright: 47, including a real mouse drag re-routing an exit.
  • Review: changes requested; every must, minor and nit is fixed. Each fix was mutation-checked.

Not verified: the real backend (the fake server enforces the contract as written), and heavily cross-linked graphs.

🤖 Generated with Claude Code

@Ahmustufa Ahmustufa mentioned this pull request Sep 23, 2026
Draft
Ahmustufa and others added 9 commits September 24, 2026 00:59
@Ahmustufa @claude
feat(sequences): conditional branching for campaign steps
aec1d74 
A step can now carry one branch that routes each enrollment after the step
is sent: always / opened / clicked / replied / not_opened / not_replied,
within N days (1-90), optionally narrowed to a reply label, with a yes exit
and a no exit (a null exit ends the path). Steps without a branch keep the
original linear code path unchanged, and a test pins that.

- New table sequence_step_branches, one row per source step. Composite FKs
  keep both exits in the same campaign and tenant. Deleting a target step
  nulls only that exit.
- The route is recomputed from events inside a fixed window measured from
  the step's own send, so a decided answer can't flip. Opens and clicks
  count human events only. Replies look at inbox_messages (the other leg)
  and exclude auto-replies unless the branch names that label.
- Cycles are refused at save time under a per-campaign lock, including for
  step delete and reorder. A runtime backstop ends the path if a loop ever
  gets in.
- Mid-flight edits use the graph as it is when the decision is made. The
  rule is documented in branchroute.go.
- GET /campaigns/{id}/graph, PUT/DELETE /campaigns/{id}/steps/{stepId}/branch.
- Threading replies to the most recently sent email, not the
  highest-numbered step, since a path can go 1 -> 3 -> 2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
fix(api): quote the yes_step_id description so the contract lints
aadd279 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
fix(sequences): branching review and security fixes
b39f88d 
Reply branches (product decision: stop-on-reply stays, so labels win):
- A reply branch on a label that stops the sequence is refused at save
  (400 reply_label_stops_sequence). The API says every default human label
  stops, so a default-label "replied" branch never fires. The replacement
  tests go through the real poll -> classify -> dispatch path.
- A reply nudge only pulls forward a due time that a condition wait set,
  never an out-of-office deferral.

Routing:
- The loop backstop compares against the current step's own send row, not
  enrollment.last_sent_at, which a recover-forward re-stamps.
- Paused and done campaigns neither finish nor park enrollments; the
  status gate now runs before routing.
- opened/clicked/not_opened are refused without tracking or an HTML body
  (400 tracking_required).
- Condition waits no longer count as "deferred" sends.
- Only a real miss is a 404.
- LatestSentForContact is workspace-pinned.
- New test: a routed send still honours suppression.

Migration: the inbox_threads index is its own single-statement
CONCURRENTLY migration (asserted indisvalid on a scratch DB), so the
branching migration no longer blocks the send path.

Docs: Postgres 15+ minimum; mid-flight edit rules corrected; invariant 82
notes that reply evidence is thread-scoped.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
docs(sequences): cite the branching invariant as 84 after the rebase
0c0eef5 
main now has the audit log at 82 and inbox search at 83 (#252).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
fix(db): serialize migrations with a polling lock so CONCURRENTLY bui…
940b1af 
…lds can't deadlock

golang-migrate's pgx5 driver waits for its advisory lock inside a
running statement, which holds a snapshot. CREATE INDEX CONCURRENTLY
waits for every snapshot, so a second migrator waiting on the lock
deadlocks the build, and the migration is left dirty. This reproduced 3
of 3 times with 6 concurrent migrators on a fresh DB, and it hits both
CI (-p 4) and multi-replica deploys.

Migrate, MigrateDown, MigrateTo and Version now take a separate
session-level lock by polling pg_try_advisory_lock on a dedicated
connection. A waiter is idle between tries and holds no snapshot. The
wait is bounded (15 min, ErrMigrationLockTimeout), and unlock/close run
on a detached context.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
feat(sequences): optimistic concurrency for step branch writes
66d904f 
PUT /branch takes an optional expected_updated_at: absent means last
writer wins (as before), null means create-only, and a timestamp means
replace only if unchanged. DELETE takes it as a query param. A mismatch
returns 409 branch_changed with the current branch (or null). It is
enforced atomically in SQL under the existing per-campaign graph lock.

Two latent bugs made the token unusable, and both are fixed:
- updated_at was serialized to whole seconds (RFC3339), so it could
  never match exactly. It is now RFC3339Nano (microseconds).
- updated_at did not always advance. now() is the transaction start
  (taken before the lock), two writes in one tx collided, and ON DELETE
  SET NULL left it untouched. A trigger now sets
  greatest(clock_timestamp(), old + 1us).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
feat(web): yes/no condition nodes on the campaign canvas
58f935a 
The canvas is wired to the branching API (GET graph, PUT/DELETE branch):
- "Add a condition" after any step, including on running campaigns. It
  draws as a diamond after its step, with Yes/No exit labels, the
  condition in words, a wait chip per exit, and a Stop per empty exit.
- A condition editor in the side panel mirrors the server's rules:
  - within_days is 1-90, and "always" has no No exit;
  - only non-stopping reply labels are offered, with the reason why;
  - tracking and an HTML body are required for open/click conditions;
  - an exit can't point at its own step.
- Dragging a Yes/No handle onto a step sets that exit (allowed while
  running). A step-to-step drag still reorders and stays draft-only.
- A refused loop (422 cycle) marks the steps on it. Unreachable steps are
  labelled "Not reached". If the graph query fails, the canvas falls back
  to the linear view with a retry banner and hides condition editing, so
  a save can't silently overwrite a branch nobody can see.
- The layout routes edges through dagre bend points, and multi-exit
  nodes route through per-exit relay points, so Yes/No don't cross and a
  skip edge doesn't hide behind a step.
- New errorCode() in @/lib/rtk-error, and error copy for every
  BranchValidationError code. The Graph cache tag is invalidated by
  branch and structural step writes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
fix(web): condition-node review fixes (stale-branch overwrites, dead …
5c41d7f 
…layout code)

- Two quick exit drags no longer revert each other. A local overlay keeps
  each completed branch write applied until a graph fetched after it
  lands, because RTK shows the previous data during a refetch, so a cache
  patch wouldn't be visible. Exit drags are also refused while a write is
  in flight.
- The condition editor re-seeds from the branch's updated_at. An
  untouched form follows the change. A dirty form keeps its edits and
  shows "changed elsewhere" with a Load the latest button.
- getCampaignGraph refetches on focus and before the editor opens.
  Cross-tab writes are still last-writer-wins until a server-side
  expected_updated_at precondition exists (documented).
- Layout: with dagre's crossing heuristic off, its constraints were never
  read, so the relays and constraints were dead. They are removed, and
  the comment now says Yes-left comes from depth-first insertion order.
  A two-condition, converging, back-jump layout test pins it.
- Minor fixes:
  - HTML check matches the server exactly;
  - reorder is refused and the banner shown in both views while the
    graph is failing;
  - a failed refetch keeps the open editor read-only;
  - drags are pre-validated and open the editor on a problem;
  - errorField() in @/lib/rtk-error replaces an ad-hoc cast;
  - aria-live on validation hints;
  - no-op drops are refused;
  - loop marks clear when data changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa @claude
feat(web): send the branch concurrency token and handle 409 branch_ch…
0f34523 
…anged

Every branch PUT/DELETE from drags and the condition editor sends
expected_updated_at, echoed verbatim with microseconds and never passed
through Date. null means create-only. On 409 the server's current
version is drawn at once. The editor keeps a dirty draft, shows the
server's version, and disables Save until Load the latest. A drag is
never re-sent. A stale delete of an already-removed branch counts as
done. The editor tracks basedOn (the token) separately from observed
(the latest seen), so it never sends a token for a version the user
didn't see.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ahmustufa
Ahmustufa force-pushed the feature/canvas-condition-nodes branch from 7ff97f7 to 0f34523 Compare September 23, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Ahmustufa