mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 08:04:25 +00:00
Ruben FiszelandClaude Opus 4.8
4dbf873723
* fix(frontend): stop flow step id generation from being poisoned by non-canonical keys
nextId computed the next step id from the max of charsToNumber over every
module id and flowState key. Only canonical auto-ids (a, b, ... aa, ab) have a
meaningful charsToNumber value, but flowState also holds copy ids ("z2"),
subflow result keys ("subflow:..."), reserved keys ("failure"/"preprocessor")
and user-renamed ids. The old `length >= 4` guard filtered long junk but let
short junk through, so e.g. duplicating step "z" (key "z2", charsToNumber 629)
made the next new step jump to "xg" and escalate from there.
nextId now only counts a key if it round-trips through numberToChars and is not
reserved, and the broken length cap is removed so large flows still get correct
ids.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(frontend): keep length cap in nextId to avoid regressing long renames
Address CI review: removing the length cap made all-lowercase renamed step
ids (e.g. "process", which round-trips through numberToChars) feed into the
max and poison id generation again — a regression versus the prior behavior,
since step ids can be renamed to ^[a-zA-Z][a-zA-Z0-9_]*$.
Restore the length>=4 skip and pair it with the round-trip canonical check,
so short non-canonical keys (copy ids "z2"/"c10", reserved/renamed short ids)
no longer poison the max while long renames stay out of the sequence. Update
the tests to reflect the actual coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
54 lines
1.9 KiB
TypeScript
54 lines
1.9 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
|
|
import type { OpenFlow } from '$lib/gen'
|
|
import type { FlowState } from './flowState'
|
|
import { nextId } from './flowModuleNextId'
|
|
|
|
function flowWith(ids: string[]): OpenFlow {
|
|
return {
|
|
summary: '',
|
|
value: {
|
|
modules: ids.map((id) => ({ id, value: { type: 'identity' } as any }))
|
|
}
|
|
} as OpenFlow
|
|
}
|
|
|
|
function stateWith(keys: string[]): FlowState {
|
|
return Object.fromEntries(keys.map((k) => [k, {}])) as FlowState
|
|
}
|
|
|
|
describe('nextId', () => {
|
|
it('produces a, b, c, ... for a fresh flow', () => {
|
|
expect(nextId(stateWith(['failure']), flowWith([]))).toBe('a')
|
|
expect(nextId(stateWith(['a', 'failure']), flowWith(['a']))).toBe('b')
|
|
expect(nextId(stateWith(['a', 'b', 'c', 'failure']), flowWith(['a', 'b', 'c']))).toBe('d')
|
|
})
|
|
|
|
it('ignores the reserved failure/preprocessor keys always present in flowState', () => {
|
|
expect(nextId(stateWith(['failure', 'preprocessor']), flowWith([]))).toBe('a')
|
|
})
|
|
|
|
// Regression: copy ids ("z2"), subflow result keys and other non-canonical keys land in
|
|
// flowState; charsToNumber on them used to leak into the max and made new steps jump to
|
|
// garbage ids like "bzw".
|
|
it('is not poisoned by copy ids', () => {
|
|
const ids = ['a', 'b', 'c']
|
|
const state = stateWith([...ids, 'c2', 'a2', 'z2', 'c10', 'failure'])
|
|
expect(nextId(state, flowWith(ids))).toBe('d')
|
|
})
|
|
|
|
it('is not poisoned by subflow result keys', () => {
|
|
const ids = ['a', 'b']
|
|
const state = stateWith([...ids, 'subflow:abcd', 'Result', 'failure'])
|
|
expect(nextId(state, flowWith(ids))).toBe('c')
|
|
})
|
|
|
|
// A step renamed to a long lowercase word ("process") is a valid base-26 string and would
|
|
// otherwise inflate the max; the length cutoff keeps such renames out of the sequence.
|
|
it('is not poisoned by renames to long lowercase words or underscored ids', () => {
|
|
const ids = ['a', 'b']
|
|
const state = stateWith([...ids, 'process', 'my_step', 'failure'])
|
|
expect(nextId(state, flowWith(ids))).toBe('c')
|
|
})
|
|
})
|