mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: an unknown base stays unknown in every loader, and an owner move extends an item move
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
733ef9c3b8
commit
29e75d27d5
+18
@@ -0,0 +1,18 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "INSERT INTO draft_move (workspace_id, typ, old_path, new_path, email)\n SELECT m.workspace_id, m.typ, m.old_path, $4, $5::text\n FROM draft_move m\n WHERE m.workspace_id = $1 AND m.typ::text = ANY($2::text[])\n AND m.new_path = $3 AND m.email IS DISTINCT FROM $5::text\n AND NOT EXISTS (\n SELECT 1 FROM draft_move o\n WHERE o.workspace_id = m.workspace_id AND o.typ = m.typ\n AND o.old_path = m.old_path AND o.email = $5::text\n )",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"TextArray",
|
||||
"Text",
|
||||
"Varchar",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "dac419c05715b2cdd965dbca1221ccdb37e078a7473e554f15b9c1f2ef6cdb95"
|
||||
}
|
||||
@@ -95,7 +95,7 @@ async fn test_draft_move_refuses_the_other_app_kind(db: Pool<Postgres>) -> anyho
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(status, 400, "move onto a classic app draft was allowed: {body}");
|
||||
assert!(
|
||||
body.contains("already have a app draft"),
|
||||
body.contains("already have a draft at 'u/test-user/mvtaken_app' (app)"),
|
||||
"the refusal did not name the occupying kind: {body}"
|
||||
);
|
||||
|
||||
|
||||
@@ -269,3 +269,36 @@ async fn test_a_teammates_draft_follows_with_its_base(db: Pool<Postgres>) -> any
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// An item move and then the owner's own move of what is left: the two records have
|
||||
/// different scopes, so the owner's move has to extend the chain in its own scope or
|
||||
/// a save addressed to the first path stops at the abandoned middle one.
|
||||
#[sqlx::test(fixtures("base", "drafts_save_follows_move"))]
|
||||
async fn test_an_owner_move_extends_an_item_move(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
|
||||
rename(port, HEAD_HASH, "u/test-user/follow_b").await?;
|
||||
// Archiving the script at the new path leaves the carried draft as a draft-only
|
||||
// item, which its owner can move through `/drafts/move`.
|
||||
sqlx::query("UPDATE script SET archived = true WHERE path = 'u/test-user/follow_b'")
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let resp = reqwest::Client::new()
|
||||
.post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/drafts/move/script/u/test-user/follow_b"
|
||||
))
|
||||
.header("Authorization", "Bearer SECRET_TOKEN")
|
||||
.json(&json!({ "new_path": "u/test-user/follow_c" }))
|
||||
.send()
|
||||
.await?;
|
||||
assert!(resp.status().is_success(), "move failed: {}", resp.text().await?);
|
||||
|
||||
assert_eq!(
|
||||
save_at(port, "u/test-user/follow_a", "after both moves").await?,
|
||||
"u/test-user/follow_c",
|
||||
"a save at the first path stopped at the path the owner's move left"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -783,7 +783,8 @@ async fn move_draft(
|
||||
} else if let Some(occupant) = row.at_target.filter(|_| new_path != path) {
|
||||
// Naming the kind matters for the app pair: a classic-app draft refusing a
|
||||
// raw-app move is invisible in the raw-app list the caller is looking at.
|
||||
format!("You already have a {} draft at '{new_path}'", occupant.replace('_', " "))
|
||||
let occupant = occupant.replace('_', " ");
|
||||
format!("You already have a draft at '{new_path}' ({occupant})")
|
||||
} else {
|
||||
format!("You have no draft at '{path}'")
|
||||
}));
|
||||
|
||||
@@ -720,7 +720,8 @@ pub async fn move_drafts_for_path(
|
||||
/// addressed to `old_path` lands on them (see `update_draft`). `email` scopes the
|
||||
/// record to one user's draft-only move; `None` is a deployed item's move, for everyone.
|
||||
///
|
||||
/// Kept to one hop: records pointing at `old_path` are re-pointed, and records
|
||||
/// Kept to one hop: records pointing at `old_path` are re-pointed (an owner's move
|
||||
/// copies another scope's into its own rather than re-pointing it), 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
|
||||
@@ -765,6 +766,30 @@ pub async fn record_draft_move(
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
// An owner's move must not re-point what everyone else follows, so the records
|
||||
// ending at `old_path` in another scope are copied into this one: a save addressed
|
||||
// to the start of that chain still reaches this destination in one hop.
|
||||
if email.is_some() {
|
||||
sqlx::query!(
|
||||
"INSERT INTO draft_move (workspace_id, typ, old_path, new_path, email)
|
||||
SELECT m.workspace_id, m.typ, m.old_path, $4, $5::text
|
||||
FROM draft_move m
|
||||
WHERE m.workspace_id = $1 AND m.typ::text = ANY($2::text[])
|
||||
AND m.new_path = $3 AND m.email IS DISTINCT FROM $5::text
|
||||
AND NOT EXISTS (
|
||||
SELECT 1 FROM draft_move o
|
||||
WHERE o.workspace_id = m.workspace_id AND o.typ = m.typ
|
||||
AND o.old_path = m.old_path AND o.email = $5::text
|
||||
)",
|
||||
w_id,
|
||||
&typs as &[&str],
|
||||
old_path,
|
||||
new_path,
|
||||
email,
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
sqlx::query!(
|
||||
"INSERT INTO draft_move (workspace_id, typ, old_path, new_path, email)
|
||||
SELECT $1, t::draft_kind, $3, $4, $5 FROM unnest($2::text[]) t",
|
||||
|
||||
@@ -607,9 +607,6 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
saved.val = undefined
|
||||
}
|
||||
await initFlow(aiDraft, store, stateStore, workspace)
|
||||
// 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
|
||||
@@ -618,7 +615,8 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
// No local draft yet — seed from `result.draft ?? result`.
|
||||
const result = await FlowService.getFlowByPath({ workspace, path, getDraft: true })
|
||||
saved.val = result as SavedFlow
|
||||
const flow: Flow = ((result as SavedFlow).draft ?? (result as Flow)) as Flow
|
||||
const serverDraft = (result as SavedFlow).draft as Flow | undefined
|
||||
const flow: Flow = (serverDraft ?? (result as Flow)) as Flow
|
||||
// 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 —
|
||||
@@ -631,7 +629,10 @@ 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 == null)
|
||||
// A draft keeps the base it forked from, unknown included (it then falls
|
||||
// back to the timestamps); only a fresh checkout takes the head, which is
|
||||
// also what keeps it from always diffing. See loadScript.
|
||||
if (deployedVersionId != null && store.val && !serverDraft)
|
||||
store.val.version_id = deployedVersionId
|
||||
slot.loadedPath = path
|
||||
slot.loadedWorkspace = workspace
|
||||
@@ -697,12 +698,6 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
schema: emptySchema(),
|
||||
language: (aiDraft.language ?? 'bun') as any
|
||||
}
|
||||
// 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
|
||||
if (aiDraft.language) baseline.language = aiDraft.language
|
||||
if (aiDraft.summary !== undefined) baseline.summary = aiDraft.summary
|
||||
@@ -717,11 +712,13 @@ function createRuntime(session: Session): SessionRuntime {
|
||||
saved.val = result as SavedScript
|
||||
// Clone before mutating, else `baseline` aliases `result` and
|
||||
// `baseline.parent_hash` corrupts the diff baseline.
|
||||
const baseline = structuredClone(
|
||||
((result as SavedScript).draft as NewScript | undefined) ?? (result as NewScript)
|
||||
)
|
||||
// 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
|
||||
const serverDraft = (result as SavedScript).draft as NewScript | undefined
|
||||
const baseline = structuredClone(serverDraft ?? (result as NewScript))
|
||||
// Only a fresh checkout forks from the head. A draft keeps the base it has,
|
||||
// unknown included: `draft.base` is derived from `parent_hash` on every
|
||||
// save, so stamping the head over it would say this draft is up to date
|
||||
// when it is not. An unknown base falls back to the timestamps.
|
||||
if (!serverDraft) 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 —
|
||||
|
||||
@@ -367,6 +367,13 @@
|
||||
const effectiveFlow: Flow = draftFromBackend
|
||||
? ({ ...deployedFlow, ...draftFromBackend } as Flow)
|
||||
: (deployedFlow as Flow)
|
||||
// The merge above would hand a draft with no base the deployed one, and the next
|
||||
// autosave would persist it as `draft.base`: a draft behind the deploy would read
|
||||
// as up to date after one open. A draft keeps the base it has, unknown included
|
||||
// (staleness then falls back to the timestamps).
|
||||
if (draftFromBackend && (draftFromBackend as any).version_id == null) {
|
||||
delete (effectiveFlow as any).version_id
|
||||
}
|
||||
savedFlow = structuredClone($state.snapshot(effectiveFlow)) as Flow
|
||||
// Baseline for the autosave `discardIf`: the deployed flow WITHOUT the
|
||||
// draft overlay (matches the unedited seed when no draft exists).
|
||||
|
||||
@@ -355,11 +355,13 @@
|
||||
? { ...deployedScript, ...draftFromBackend }
|
||||
: (deployedScript as EditableScript)
|
||||
savedScript = structuredClone($state.snapshot(effectiveScript))
|
||||
// The draft's base is the version it forked from and only the user moves
|
||||
// it (by discarding or rebasing). Seeding it from the head here would let
|
||||
// the next autosave persist the head as the base, so a draft behind the
|
||||
// deploy reads as up to date after one open.
|
||||
const parentHash = topHash ?? backendScript.draft_base ?? backendScript.hash
|
||||
// The draft's base is the version it forked from and only the user moves it
|
||||
// (by discarding or rebasing). A draft keeps the base it has, unknown included
|
||||
// (staleness then falls back to the timestamps); seeding the head over it here
|
||||
// would let the next autosave persist the head as the base, so a draft behind
|
||||
// the deploy would read as up to date after one open. Only a fresh checkout
|
||||
// forks from the head.
|
||||
const parentHash = topHash ?? (hasOwnDraft ? backendScript.draft_base : backendScript.hash)
|
||||
// Baseline for the autosave `discardIf`: the deployed script with the
|
||||
// same `parent_hash` graft the seed below applies and the schema as the
|
||||
// mounted editor holds it, so the unedited draft compares equal.
|
||||
|
||||
Reference in New Issue
Block a user