mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix: session loaders keep a draft's base, and a failed relocation flush stays put
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ccea81f5e9
commit
ee2c2abcb9
@@ -157,3 +157,85 @@ async fn test_save_follows_a_draft_only_move(db: Pool<Postgres>) -> anyhow::Resu
|
||||
assert_eq!(draft["draft_path"], "u/test-user/moved", "{draft}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Rename `from` to `to` the way Home does: redeploy the deployed content at the new
|
||||
/// path, keeping the deployer's own draft so it is carried rather than consumed.
|
||||
/// Returns the new head's hash.
|
||||
async fn rename(port: u16, from_hash: &str, to: &str) -> anyhow::Result<String> {
|
||||
let resp = reqwest::Client::new()
|
||||
.post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/scripts/create"
|
||||
))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN")
|
||||
.json(&json!({
|
||||
"path": to,
|
||||
"parent_hash": from_hash,
|
||||
"summary": "A",
|
||||
"description": "",
|
||||
"content": "export function main() { return 1 }",
|
||||
"language": "deno",
|
||||
"schema": {},
|
||||
"skip_draft_deletion": true
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let hash = resp.text().await?;
|
||||
assert_eq!(status, 201, "rename to {to} failed: {hash}");
|
||||
Ok(hash)
|
||||
}
|
||||
|
||||
/// Save the draft as an editor still bound to `url_path` would. Returns the path the
|
||||
/// save landed at.
|
||||
async fn save_at(port: u16, url_path: &str, content: &str) -> anyhow::Result<String> {
|
||||
let saved: Value = reqwest::Client::new()
|
||||
.post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/drafts/update/script/{url_path}"
|
||||
))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN")
|
||||
.json(&json!({
|
||||
"value": { "path": url_path, "summary": "A", "content": content, "language": "deno" }
|
||||
}))
|
||||
.send()
|
||||
.await?
|
||||
.json()
|
||||
.await?;
|
||||
assert_eq!(saved["status"], "saved", "save refused: {saved}");
|
||||
Ok(saved["path"].as_str().unwrap_or_default().to_string())
|
||||
}
|
||||
|
||||
/// A record is kept to one hop, and a move back to the path it left ends it: both are
|
||||
/// three statements whose order decides the answer.
|
||||
#[sqlx::test(fixtures("base", "drafts_save_follows_move"))]
|
||||
async fn test_move_records_stay_one_hop(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
|
||||
let b = rename(port, HEAD_HASH, "u/test-user/follow_b").await?;
|
||||
let _c = rename(port, &b, "u/test-user/follow_c").await?;
|
||||
assert_eq!(
|
||||
save_at(port, "u/test-user/follow_a", "after two moves").await?,
|
||||
"u/test-user/follow_c",
|
||||
"a save at the first path did not reach the last"
|
||||
);
|
||||
assert_eq!(own_draft_paths(port).await?, vec!["u/test-user/follow_c"]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("base", "drafts_save_follows_move"))]
|
||||
async fn test_move_back_ends_the_record(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
|
||||
let b = rename(port, HEAD_HASH, "u/test-user/follow_b").await?;
|
||||
let _a = rename(port, &b, "u/test-user/follow_a").await?;
|
||||
assert_eq!(
|
||||
save_at(port, "u/test-user/follow_a", "after moving back").await?,
|
||||
"u/test-user/follow_a",
|
||||
"a save was routed off the path the item moved back to"
|
||||
);
|
||||
assert_eq!(own_draft_paths(port).await?, vec!["u/test-user/follow_a"]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -722,6 +722,16 @@ pub async fn move_drafts_for_path(
|
||||
///
|
||||
/// Kept to one hop: records pointing at `old_path` are re-pointed, and records
|
||||
/// leaving either path are replaced, since `new_path` now holds the item.
|
||||
///
|
||||
/// **The caller must have authorized the move first.** A record routes every later
|
||||
/// draft write at `old_path` (any owner's, for an item move), and enforces nothing
|
||||
/// itself.
|
||||
///
|
||||
/// A record outlives the editors that need it: it ends when a later move touches
|
||||
/// either path or an item is deployed at `old_path`, so a save that means to start
|
||||
/// a NEW draft at a vacated path would be routed instead. Nothing does that today —
|
||||
/// every surface parks a new item at a minted `u/<user>/draft_<uuid>` key
|
||||
/// (`mintDraftPath.ts`) and carries the user-typed name inside the value.
|
||||
pub async fn record_draft_move(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
w_id: &str,
|
||||
@@ -771,6 +781,9 @@ pub async fn record_draft_move(
|
||||
|
||||
/// Drop the move records leaving `path`: an item was just created there, and saves
|
||||
/// addressed to it are its own.
|
||||
///
|
||||
/// **The caller must have authorized the deploy that created the item first.**
|
||||
/// Dropping a record sends later draft writes at `path` back to `path`.
|
||||
pub async fn clear_draft_moves_from(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
w_id: &str,
|
||||
|
||||
@@ -148,6 +148,16 @@
|
||||
relocating = true
|
||||
await onBeforeRelocate?.()
|
||||
await UserDraftDbSyncer.flush(query)
|
||||
// `flush` resolves on a failed or rejected save as well, and leaving the
|
||||
// route drops what it was carrying: stay, so the editor keeps the edits
|
||||
// and its own failure indicator.
|
||||
if (
|
||||
UserDraftDbSyncer.getState(query).failureMessage ||
|
||||
UserDraftDbSyncer.getConflict(query).conflict
|
||||
) {
|
||||
relocating = false
|
||||
return
|
||||
}
|
||||
sendUserToast(`This item was moved to ${newPath}. You are now editing it there.`)
|
||||
await goto(`${base}/${seg}/${newPath}`)
|
||||
})
|
||||
|
||||
@@ -607,7 +607,9 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
saved.val = undefined
|
||||
}
|
||||
await initFlow(aiDraft, store, stateStore, workspace)
|
||||
if (deployedVersionId != null && store.val) store.val.version_id = deployedVersionId
|
||||
// Only a draft with no base takes the head; see loadScript.
|
||||
if (deployedVersionId != null && store.val && store.val.version_id == null)
|
||||
store.val.version_id = deployedVersionId
|
||||
slot.loadedPath = path
|
||||
slot.loadedWorkspace = workspace
|
||||
return
|
||||
@@ -629,7 +631,8 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
)
|
||||
UserDraft.save('flow', path, flow, { workspace })
|
||||
await initFlow(flow, store, stateStore, workspace)
|
||||
if (deployedVersionId != null && store.val) store.val.version_id = deployedVersionId
|
||||
if (deployedVersionId != null && store.val && store.val.version_id == null)
|
||||
store.val.version_id = deployedVersionId
|
||||
slot.loadedPath = path
|
||||
slot.loadedWorkspace = workspace
|
||||
} catch (err) {
|
||||
@@ -694,7 +697,10 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
schema: emptySchema(),
|
||||
language: (aiDraft.language ?? 'bun') as any
|
||||
}
|
||||
if (saved.val?.hash) {
|
||||
// Only a draft with no base takes the head: `draft.base` is derived from
|
||||
// `parent_hash` on every save, so stamping the head over a fork base
|
||||
// tells the server this draft is up to date when it is not.
|
||||
if (saved.val?.hash && baseline.parent_hash == null) {
|
||||
baseline.parent_hash = saved.val.hash
|
||||
}
|
||||
baseline.content = aiDraft.content
|
||||
@@ -714,7 +720,8 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
const baseline = structuredClone(
|
||||
((result as SavedScript).draft as NewScript | undefined) ?? (result as NewScript)
|
||||
)
|
||||
baseline.parent_hash = result.hash
|
||||
// See the ai-draft branch above: the head only seeds a draft that has no base.
|
||||
if (baseline.parent_hash == null) baseline.parent_hash = result.hash
|
||||
// Seed the per-tab last_sync from the server draft's timestamp so the
|
||||
// seeding save below attaches a matching last_sync and the server can
|
||||
// reject stale writes (see loadRawApp). Without this a server draft —
|
||||
@@ -842,7 +849,11 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
path: result.path,
|
||||
custom_path: draftValue?.custom_path ?? result.custom_path,
|
||||
draft_path: draftValue?.draft_path,
|
||||
parent_version: draftValue?.parent_version
|
||||
// The draft's own base, else the head this checkout forks from (the
|
||||
// standalone editor's loader does the same).
|
||||
parent_version:
|
||||
draftValue?.parent_version ??
|
||||
(Array.isArray(result.versions) ? result.versions[result.versions.length - 1] : undefined)
|
||||
}
|
||||
// Seed the per-tab last_sync from the server draft's timestamp so
|
||||
// later saves attach a matching last_sync and the server can reject
|
||||
|
||||
Reference in New Issue
Block a user