Repository navigation
fix(workflow): a plain save no longer clobbers is_public - #8498
Merged
Merged
Conversation
Since apache#8125 the persist endpoint wrote is_public from the request. The frontend feeds the saved row back as its metadata, and that row names the flag isPublic while the rest of the frontend calls it isPublished, so the very next autosave went out without the flag, the update wrote NULL into a NOT NULL column, and every second save failed with 500 (silently on the canvas, "Could not save" on the Form View). A stale false on a save could likewise un-publish a workflow. persist now writes name, description and content only; publishing stays with /public and /private, and default_view with /set-default-view. The frontend stops sending the flag on a save, and parseWorkflowInfo carries a persist response's isPublic over to isPublished so metadata fed back from a save keeps the publish state. Closes apache#8496. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
github-actions
Bot
added
engine
fix
frontend
Yicong-Huang
added
release/v1.3
Contributor
Backport auto-label reportThis
|
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The backend and frontend changes consistently address the regression with focused coverage for the affected save paths.
Pull request overview
Fixes workflow autosave failures and prevents ordinary saves from overwriting publication state.
Changes:
- Restricts backend saves to name, description, and content.
- Normalizes persisted
isPublicresponses to frontendisPublished. - Adds regression coverage for missing and stale publication flags.
File summaries
| File | Description |
|---|---|
WorkflowResource.scala |
Preserves publication state during saves. |
WorkflowResourceSpec.scala |
Adds backend regression tests. |
workflow-persist.service.ts |
Omits publication state from save requests. |
workflow-persist.service.spec.ts |
Verifies request and response behavior. |
workflow-util.service.ts |
Normalizes the publication field name. |
workflow-util.service.spec.ts |
Tests publication metadata normalization. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 377 | 0.23 | 26,406/31,863/31,863 us | 🟢 -32.7% / 🔴 +103.8% |
| 🔴 | bs=100 sw=10 sl=64 | 781 | 0.476 | 124,770/162,311/162,311 us | 🔴 +20.2% / 🔴 +49.3% |
| ⚪ | bs=1000 sw=10 sl=64 | 919 | 0.561 | 1,086,997/1,157,494/1,157,494 us | ⚪ within ±5% / 🔴 +11.5% |
Baseline details
Latest main 0e549fa from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 377 tuples/sec | 415 tuples/sec | 755.28 tuples/sec | -9.2% | -50.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.23 MB/s | 0.253 MB/s | 0.461 MB/s | -9.1% | -50.1% |
| bs=10 sw=10 sl=64 | p50 | 26,406 us | 20,222 us | 12,957 us | +30.6% | +103.8% |
| bs=10 sw=10 sl=64 | p95 | 31,863 us | 47,368 us | 16,134 us | -32.7% | +97.5% |
| bs=10 sw=10 sl=64 | p99 | 31,863 us | 47,368 us | 20,333 us | -32.7% | +56.7% |
| bs=100 sw=10 sl=64 | throughput | 781 tuples/sec | 850 tuples/sec | 980.1 tuples/sec | -8.1% | -20.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.476 MB/s | 0.519 MB/s | 0.598 MB/s | -8.3% | -20.4% |
| bs=100 sw=10 sl=64 | p50 | 124,770 us | 115,658 us | 101,894 us | +7.9% | +22.5% |
| bs=100 sw=10 sl=64 | p95 | 162,311 us | 135,032 us | 108,718 us | +20.2% | +49.3% |
| bs=100 sw=10 sl=64 | p99 | 162,311 us | 135,032 us | 122,482 us | +20.2% | +32.5% |
| bs=1000 sw=10 sl=64 | throughput | 919 tuples/sec | 929 tuples/sec | 1,011 tuples/sec | -1.1% | -9.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.561 MB/s | 0.567 MB/s | 0.617 MB/s | -1.1% | -9.1% |
| bs=1000 sw=10 sl=64 | p50 | 1,086,997 us | 1,068,865 us | 996,422 us | +1.7% | +9.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,157,494 us | 1,193,661 us | 1,037,670 us | -3.0% | +11.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,157,494 us | 1,193,661 us | 1,072,152 us | -3.0% | +8.0% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,530.08,200,128000,377,0.230,26406.34,31863.05,31863.05
1,100,10,64,20,2561.98,2000,1280000,781,0.476,124769.78,162311.19,162311.19
2,1000,10,64,20,21770.03,20000,12800000,919,0.561,1086997.12,1157494.01,1157494.01
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8498 +/- ##
============================================
+ Coverage 93.69% 93.90% +0.21%
- Complexity 4825 4944 +119
============================================
Files 1209 1213 +4
Lines 49871 53030 +3159
Branches 6099 6804 +705
============================================
+ Hits 46726 49800 +3074
- Misses 1652 1705 +53
- Partials 1493 1525 +32
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Backport to |
Contributor
|
Backported to |
This was referenced Sep 11, 2026
aglinxinyuan
pushed a commit
to aglinxinyuan/texera
that referenced
this pull request
Oct 10, 2026
### What changes were proposed in this PR?
A public workflow follows the author's latest content, as publishing has
always done: every
save reaches the Hub immediately. This adds the other state — the author
pins the version
they have now, and what the public sees stops moving until they pin
again.
- **`WorkflowPublishService`** owns the two states and the moves between
them: `publish`,
`pinLatest`, `unpin`, `unpublish`, `statusOf`. `is_public` stays the
on/off switch;
`published_content` is the pin, NULL while following.
- **A pin freezes everything on public show** — the graph, the title,
the description and the
view the workflow opens in — because a copy that froze only its graph
would still advertise
a title nobody published. The default view matters because a form's
definition rides inside
the content: serving the author's live preference over a frozen graph
would open a form on a
copy that has none.
- **A pin is one statement.** Each column is copied from its own row
(`published_content = content`
and so on) rather than from a workflow read a moment earlier, so there
is no window in which the
author's next save lands and the pin freezes the version before it --
which would have left them
reading "you have unpublished changes" the instant after they pinned.
- **Three endpoints**:
| Endpoint | Does |
|---|---|
| `POST /workflow/pin/{wid}` | freezes the author's current version as
the public copy; called again, moves the pin forward |
| `DELETE /workflow/pin/{wid}` | drops the pin, back to following the
latest |
| `GET /workflow/publish-status/{wid}` | published, pinned, and whether
a pin is holding edits back |
- **Publishing and unpublishing move through the same service**, so
unpublishing drops the pin
rather than leaving a private workflow carrying one. Re-publishing
starts in the following
state: coming back should not silently put old public content on show
again.
- **One writer for the publish columns.** Two paths could still write
them by accident, because
each read a whole row (or took a whole `Workflow` from the client) and
wrote it all back:
- a **rename or description edit** wrote every column from a row it had
read a moment
earlier, so anything landing in between was silently rewritten to what
that read had seen:
a publish undone, a pin dropped, or — worst — a workflow the author had
just unpublished
put back on public show under its frozen copy. Each now writes only its
own column.
- **creating** a workflow clears the publish columns, so a request body
cannot seed a
published copy of its own choosing.
A plain **save** was the third, and apache#8498 has since narrowed it on main
for its own reasons,
so nothing is needed here — the tests below still pin the behaviour from
this feature's side.
Together with `/set-default-view`, which already wrote only its column,
every endpoint now
writes what it owns and nothing else: outside creation,
`WorkflowPublishService` is the only
writer of `is_public` and the frozen copy.
**Nothing on screen changes.** No read path consults the pinned copy yet
and there is no UI:
every workflow stays in the following state it is in today. The
endpoints answer, and nothing
calls them.
### Any related issues, documentation, discussions?
Closes apache#7938
Part of apache#7828. Design discussion: apache#7128. Schema: apache#7851.
### How was this PR tested?
32 cases in `WorkflowPublishSpec`:
- the state machine — follow → pin → re-pin → unpin → unpublish →
re-publish, and that a
re-publish does not resurrect the previous pin;
- what a pin freezes — the name, the description and the default view
alongside the content,
and that each of them moving afterwards is reported as an unpublished
change;
- the guards — no write access, not published, and a workflow that does
not exist;
- that a create cannot inject publish columns, and that a save, a
collaborator's save and a
rename all leave the publish state alone;
- that a publish or an unpublish landing while a rename is in flight
survives it, and that a save
landing while a pin is in flight is the version that gets pinned. These
three drive the
interleaving off the statement itself rather than off a thread, so the
ordering is the same on
every run;
- the ordinary states that are easy to forget: content that is not valid
JSON is reported as
drift rather than throwing, a workflow with no description pins like any
other,
unpinning one that is already following is a no-op rather than an error,
and publishing one that
is already public leaves its pin alone.
`hasUnpublishedChanges` compares the two copies as parsed JSON rather
than as strings — the
same graph can come back with its keys in another order, and reporting
that as an edit is an
alarm the author cannot clear. One case covers it.
The narrowed write paths were checked by mutation: restoring `is_public`
to the save statement
turns three cases red, one of them on the CHECK constraint itself, and
restoring the
read-modify-write to the rename path turns its two interleaving cases
red — one showing the
publish reverted, the other showing the unpublished workflow back on
public show. Restoring the
read-then-write to the pin turns the third red, showing the pin frozen
one save behind.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-5)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
What changes were proposed in this PR?
Closes #8496. A regression from #8125: since then, every workflow's second autosave (and every one after it) fails with 500, silently on the operator canvas and as "Could not save" on the Form View.
persistWorkflowwroteis_publicfrom the request. The frontend feeds the saved row straight back as its metadata, and that row names the flagisPublicwhile the rest of the frontend calls itisPublished, so the very next save went out without the flag, the update wrote NULL into a NOT NULL column, and the request failed. A staleisPublic: falseon a save could likewise un-publish a published workflow.saveWorkflowFieldsnow writes name, description and content only. Publishing stays with/publicand/private,default_viewwith/set-default-view, and the timestamps are not rewritten, so a save can never clobber a concurrent change to any of them.WorkflowPersistService.persistWorkflowno longer sendsisPublic(the endpoint does not read it, and the value is not reliably known after the first save), andWorkflowUtilService.parseWorkflowInfocarries a persist response'sisPublicover toisPublished, so metadata fed back from a save keeps the publish state instead of dropping it.The Form View stack is not blocked by this: #8455 touches none of these files, and #8456 touches
workflow-persist.service.tsonly increateWorkflow(addingdefaultView), a different function; a dry-run merge of the two is clean.Any related issues, documentation, discussions?
Closes #8496. Found while verifying #8455 on a flag-on instance (parent #8011).
How was this PR tested?
Backend:
WorkflowResourceSpecgains two tests, a save carrying no flag neither fails nor changesis_publicafter/public, and a save carryingfalsedoes not un-publish; the existing default-view save test was updated to send no flag, as the frontend does.WorkflowResourceSpecandPublishedCopySchemaSpecpass (87 tests), scalafmt clean.Frontend: the persist spec asserts the save payload carries no
isPublicand that the response'sisPubliccomes back asisPublished;parseWorkflowInfogains tests for the carry-over and for leaving a presentisPublishedalone. Full suite passes (5777), changed lines fully covered, eslint, prettier and the production (AOT) build pass.End to end, against a running stack rebuilt with this change: the exact second-save payload that returned 500 now returns 200 with
is_publicunchanged; a create-through-persist with the new payload inserts withis_public = false; and in a headless browser the Form View renames a workflow twice with every/api/workflow/persistanswering 200, noisPublickey in any request body, and no "Could not save".Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY