fix(forking): hide satisfied dependent configuration (#6723)

* fix(forking): hide satisfied dependent configuration

* fix(forking): keep dependent chains configurable

* fix(forking): invalidate stale dependent selectors
This commit is contained in:
Vikhyath Mondreti
2026-08-15 18:36:53 -07:00
committed by GitHub
parent 9e67655b23
commit be20df9257
5 changed files with 441 additions and 36 deletions
@@ -4,9 +4,12 @@
import { describe, expect, it } from 'vitest'
import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork'
import {
applyDependentRepick,
dependentKey,
effectiveCopyDependentValue,
effectiveDependentValue,
getActionableDependentFields,
isDependentConfigurationActionable,
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
const field = (overrides: Partial<ForkDependentReconfig> = {}): ForkDependentReconfig => ({
@@ -97,3 +100,245 @@ describe('effectiveCopyDependentValue', () => {
expect(effectiveCopyDependentValue(f, {})).toBe('')
})
})
describe('applyDependentRepick', () => {
it('clears direct and transitive descendants without touching unrelated fields', () => {
const site = field({
subBlockKey: 'siteId',
currentValue: 'site-old',
providesContextKey: 'siteId',
})
const drive = field({
subBlockKey: 'driveId',
currentValue: 'drive-old',
providesContextKey: 'driveId',
consumesContextKeys: ['siteId'],
})
const spreadsheet = field({
subBlockKey: 'spreadsheetId',
currentValue: 'spreadsheet-old',
providesContextKey: 'spreadsheetId',
consumesContextKeys: ['driveId'],
})
const sheet = field({
subBlockKey: 'sheetName',
currentValue: 'Sheet1',
consumesContextKeys: ['spreadsheetId'],
})
const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' })
const previous = {
[dependentKey(drive)]: 'drive-repicked',
[dependentKey(spreadsheet)]: 'spreadsheet-repicked',
[dependentKey(sheet)]: 'Sheet2',
[dependentKey(unrelated)]: 'still-keep-me',
}
const next = applyDependentRepick(
previous,
site,
[site, drive, spreadsheet, sheet, unrelated],
'site-new'
)
expect(next).toEqual({
[dependentKey(site)]: 'site-new',
[dependentKey(drive)]: '',
[dependentKey(spreadsheet)]: '',
[dependentKey(sheet)]: '',
[dependentKey(unrelated)]: 'still-keep-me',
})
expect(effectiveDependentValue(drive, next, false)).toBe('')
expect(effectiveCopyDependentValue(sheet, next)).toBe('')
})
it('only changes the selected field when it provides no selector context', () => {
const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' })
const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' })
expect(
applyDependentRepick(
{ [dependentKey(unrelated)]: 'still-keep-me' },
leaf,
[leaf, unrelated],
'ISSUE-2'
)
).toEqual({
[dependentKey(leaf)]: 'ISSUE-2',
[dependentKey(unrelated)]: 'still-keep-me',
})
})
})
describe('isDependentConfigurationActionable', () => {
it('hides stored values when the mapped parent is unchanged', () => {
expect(
isDependentConfigurationActionable(
field({ required: true, currentValue: 'INBOX' }),
{},
{
parentResolved: true,
parentChanged: false,
copying: false,
}
)
).toBe(false)
})
it('shows a required value that is missing under an unchanged mapped parent', () => {
expect(
isDependentConfigurationActionable(
field({ required: true, currentValue: '' }),
{},
{
parentResolved: true,
parentChanged: false,
copying: false,
}
)
).toBe(true)
})
it('hides a missing optional value under an unchanged mapped parent', () => {
expect(
isDependentConfigurationActionable(
field({ required: false, currentValue: '' }),
{},
{
parentResolved: true,
parentChanged: false,
copying: false,
}
)
).toBe(false)
})
it('shows every dependent when the mapped parent changed', () => {
expect(
isDependentConfigurationActionable(
field({ required: false, currentValue: 'INBOX' }),
{},
{
parentResolved: true,
parentChanged: true,
copying: false,
}
)
).toBe(true)
})
it('shows every dependent when the parent will be copied', () => {
expect(
isDependentConfigurationActionable(
field({ required: false, currentValue: 'INBOX' }),
{},
{
parentResolved: true,
parentChanged: false,
copying: true,
}
)
).toBe(true)
})
it('hides dependents until their parent is resolved', () => {
expect(
isDependentConfigurationActionable(
field({ required: true, currentValue: '' }),
{},
{
parentResolved: false,
parentChanged: false,
copying: false,
}
)
).toBe(false)
})
})
describe('getActionableDependentFields', () => {
const unchangedMappedParent = {
parentResolved: true,
parentChanged: false,
copying: false,
}
it('includes the context provider for a required missing child', () => {
const spreadsheet = field({
subBlockKey: 'spreadsheetId',
title: 'Spreadsheet',
currentValue: '',
providesContextKey: 'spreadsheetId',
})
const sheet = field({
subBlockKey: 'sheetName',
title: 'Sheet',
currentValue: '',
required: true,
consumesContextKeys: ['spreadsheetId'],
})
expect(
getActionableDependentFields([spreadsheet, sheet], {}, unchangedMappedParent).map(
(dependent) => dependent.subBlockKey
)
).toEqual(['spreadsheetId', 'sheetName'])
})
it('keeps a saved context provider visible while its child needs configuration', () => {
const spreadsheet = field({
subBlockKey: 'spreadsheetId',
title: 'Spreadsheet',
currentValue: 'spreadsheet-target',
providesContextKey: 'spreadsheetId',
})
const sheet = field({
subBlockKey: 'sheetName',
title: 'Sheet',
currentValue: '',
required: true,
consumesContextKeys: ['spreadsheetId'],
})
expect(
getActionableDependentFields([spreadsheet, sheet], {}, unchangedMappedParent).map(
(dependent) => dependent.subBlockKey
)
).toEqual(['spreadsheetId', 'sheetName'])
})
it('walks transitive providers and leaves unrelated optional fields hidden', () => {
const unrelated = field({
subBlockKey: 'optionalLabel',
title: 'Optional label',
currentValue: '',
})
const site = field({
subBlockKey: 'siteId',
title: 'Site',
currentValue: '',
providesContextKey: 'siteId',
})
const drive = field({
subBlockKey: 'driveId',
title: 'Drive',
currentValue: '',
providesContextKey: 'driveId',
consumesContextKeys: ['siteId'],
})
const spreadsheet = field({
subBlockKey: 'spreadsheetId',
title: 'Spreadsheet',
currentValue: '',
required: true,
consumesContextKeys: ['driveId'],
})
expect(
getActionableDependentFields(
[unrelated, site, drive, spreadsheet],
{},
unchangedMappedParent
).map((dependent) => dependent.subBlockKey)
).toEqual(['siteId', 'driveId', 'spreadsheetId'])
})
})
@@ -5,6 +5,40 @@ export function dependentKey(dependent: ForkDependentReconfig): string {
return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}`
}
/**
* Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string
* overrides are intentional: an absent override means "fall back to the stored value", while a
* changed provider makes every stored descendant stale for both mapped and copied parents.
*/
export function applyDependentRepick(
reconfig: Record<string, string>,
changedField: ForkDependentReconfig,
blockFields: ForkDependentReconfig[],
value: string
): Record<string, string> {
const changedKey = dependentKey(changedField)
const nextState = { ...reconfig, [changedKey]: value }
if (!changedField.providesContextKey) return nextState
const pendingContextKeys = [changedField.providesContextKey]
const visitedFields = new Set([changedKey])
for (let index = 0; index < pendingContextKeys.length; index += 1) {
const contextKey = pendingContextKeys[index]
if (!contextKey) continue
for (const field of blockFields) {
const fieldKey = dependentKey(field)
if (visitedFields.has(fieldKey) || !field.consumesContextKeys.includes(contextKey)) continue
visitedFields.add(fieldKey)
nextState[fieldKey] = ''
if (field.providesContextKey) pendingContextKeys.push(field.providesContextKey)
}
}
return nextState
}
/**
* The value sent + displayed for a dependent: the user's in-session re-pick if present, else the
* stored value (`currentValue`). Blank when the parent target changed in-session, since the old
@@ -37,3 +71,57 @@ export function effectiveCopyDependentValue(
if (repicked !== undefined) return repicked
return field.currentValue || field.sourceValue
}
export interface DependentConfigurationState {
parentResolved: boolean
parentChanged: boolean
copying: boolean
}
/**
* Whether a dependent selector needs to be shown. A changed or copied parent requires review
* because its children resolve in a different scope. An unchanged mapping only needs a selector
* when a required value is missing; its stored values are already valid and sync-ready.
*/
export function isDependentConfigurationActionable(
field: ForkDependentReconfig,
reconfig: Record<string, string>,
state: DependentConfigurationState
): boolean {
if (!state.parentResolved) return false
if (state.parentChanged || state.copying) return true
return field.required && effectiveDependentValue(field, reconfig, false) === ''
}
/**
* Actionable fields plus the transitive in-block providers that scope them. A provider belongs
* in the configuration UI whenever one of its descendants needs action, even if its saved value
* is present, so the user can see and change the context in which the child is selected.
*/
export function getActionableDependentFields(
fields: ForkDependentReconfig[],
reconfig: Record<string, string>,
state: DependentConfigurationState
): ForkDependentReconfig[] {
const actionable = new Set(
fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state))
)
const providersByContextKey = new Map<string, ForkDependentReconfig>()
for (const field of fields) {
if (field.providesContextKey) providersByContextKey.set(field.providesContextKey, field)
}
const pending = Array.from(actionable)
for (let index = 0; index < pending.length; index += 1) {
const field = pending[index]
if (!field) continue
for (const contextKey of field.consumesContextKeys) {
const provider = providersByContextKey.get(contextKey)
if (!provider || actionable.has(provider)) continue
actionable.add(provider)
pending.push(provider)
}
}
return fields.filter((field) => actionable.has(field))
}
@@ -34,9 +34,12 @@ import {
import { forkRefKey } from '@/ee/workspace-forking/components/fork-sync/copy-reconciliation'
import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-sync/dependent-field-selector'
import {
applyDependentRepick,
type DependentConfigurationState,
dependentKey,
effectiveCopyDependentValue,
effectiveDependentValue,
getActionableDependentFields,
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
import type {
ForkKindSummary,
@@ -89,6 +92,7 @@ interface DependentBlock {
targetBlockId: string
blockName: string
fields: ForkDependentReconfig[]
configurableFields: ForkDependentReconfig[]
}
interface WorkflowDependents {
@@ -103,7 +107,9 @@ interface WorkflowDependents {
*/
function groupDependentsByWorkflow(
workflows: ForkResourceUsage['workflows'],
dependents: ForkDependentReconfig[]
dependents: ForkDependentReconfig[],
reconfig: Record<string, string>,
state: DependentConfigurationState
): WorkflowDependents[] {
const byWorkflow = new Map<string, ForkDependentReconfig[]>()
for (const dependent of dependents) {
@@ -116,7 +122,12 @@ function groupDependentsByWorkflow(
for (const field of byWorkflow.get(workflow.workflowId) ?? []) {
let block = byBlock.get(field.targetBlockId)
if (!block) {
block = { targetBlockId: field.targetBlockId, blockName: field.blockName, fields: [] }
block = {
targetBlockId: field.targetBlockId,
blockName: field.blockName,
fields: [],
configurableFields: [],
}
byBlock.set(field.targetBlockId, block)
}
block.fields.push(field)
@@ -124,7 +135,13 @@ function groupDependentsByWorkflow(
return {
workflowId: workflow.workflowId,
workflowName: workflow.workflowName,
blocks: Array.from(byBlock.values()).sort((a, b) => a.blockName.localeCompare(b.blockName)),
blocks: Array.from(byBlock.values())
.map((block) => ({
...block,
configurableFields: getActionableDependentFields(block.fields, reconfig, state),
}))
.filter((block) => block.configurableFields.length > 0)
.sort((a, b) => a.blockName.localeCompare(b.blockName)),
}
})
}
@@ -146,28 +163,6 @@ function blockChainState(
return { providedValues, providedContextKeys }
}
/** Store a re-pick and invalidate in-block children chained off the changed field. */
function applyDependentRepick(
setReconfig: Dispatch<SetStateAction<Record<string, string>>>,
field: ForkDependentReconfig,
blockFields: ForkDependentReconfig[],
value: string
) {
setReconfig((prev) => {
const nextState = { ...prev, [dependentKey(field)]: value }
// A changed parent invalidates its children's stale re-picks.
const providedKey = field.providesContextKey
if (providedKey) {
for (const sibling of blockFields) {
if (sibling.consumesContextKeys.includes(providedKey)) {
delete nextState[dependentKey(sibling)]
}
}
}
return nextState
})
}
interface DependentSelectorProps {
field: ForkDependentReconfig
block: DependentBlock
@@ -225,7 +220,9 @@ function DependentSelector({
}}
enabled={parentValue !== '' && ready}
value={effectiveValue(field)}
onChange={(value) => applyDependentRepick(setReconfig, field, block.fields, value)}
onChange={(value) =>
setReconfig((current) => applyDependentRepick(current, field, block.fields, value))
}
title={field.title}
/>
)
@@ -260,7 +257,7 @@ function DependentWorkflowCard({
setReconfig,
}: DependentWorkflowCardProps) {
const [collapsed, setCollapsed] = useState(
() => !workflow.blocks.some((block) => block.fields.some((field) => field.required))
() => !workflow.blocks.some((block) => block.configurableFields.some((field) => field.required))
)
return (
<CollapsibleCard
@@ -270,9 +267,9 @@ function DependentWorkflowCard({
>
<div className='flex flex-col gap-3'>
{workflow.blocks.map((block) => {
const topLevel = block.fields.filter((field) => !field.toolName)
const topLevel = block.configurableFields.filter((field) => !field.toolName)
const byTool = new Map<string, ForkDependentReconfig[]>()
for (const field of block.fields) {
for (const field of block.configurableFields) {
if (!field.toolName) continue
const list = byTool.get(field.toolName)
if (list) list.push(field)
@@ -357,11 +354,14 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
const usages = controller.usagesForEntry(entry)
const dependents = controller.dependentsForEntry(entry)
// Group once per (usages, dependents) change - both keep stable references from the
// controller's memoized maps, so this skips recompute across the page's frequent re-renders.
const workflows = useMemo(
() => groupDependentsByWorkflow(usages, dependents),
[usages, dependents]
() =>
groupDependentsByWorkflow(usages, dependents, controller.reconfig, {
parentResolved: target !== '' || copying,
parentChanged,
copying,
}),
[usages, dependents, controller.reconfig, target, parentChanged, copying]
)
const configurable = workflows.filter((workflow) => workflow.blocks.length > 0)
const usedOnly = workflows.filter((workflow) => workflow.blocks.length === 0)
@@ -20,10 +20,19 @@ const blockWith = (subBlocks: SubBlockConfig[]): BlockConfig =>
const sourceState = (
blockType: string,
subBlocks: Record<string, { value: unknown }>
subBlocks: Record<string, { value: unknown }>,
data?: Record<string, unknown>
): WorkflowState =>
({
blocks: { 'block-1': { id: 'block-1', type: blockType, name: 'Block', subBlocks } },
blocks: {
'block-1': {
id: 'block-1',
type: blockType,
name: 'Block',
subBlocks,
...(data && { data }),
},
},
edges: [],
loops: {},
parallels: {},
@@ -325,6 +334,67 @@ describe('collectForkDependentReconfigs', () => {
expect(sheet?.context.spreadsheetId).toBe('ss-src')
})
it('uses the persisted canonical mode when building a dependent selector context', () => {
vi.mocked(getBlock).mockReturnValue(
blockWith([
{ id: 'credential', title: 'Credential', type: 'oauth-input' },
{ id: 'domain', title: 'Domain', type: 'short-input' },
{
id: 'projectId',
title: 'Project',
type: 'project-selector',
canonicalParamId: 'projectId',
mode: 'basic',
selectorKey: 'jira.projects',
dependsOn: ['credential', 'domain'],
},
{
id: 'manualProjectId',
title: 'Project ID',
type: 'short-input',
canonicalParamId: 'projectId',
mode: 'advanced',
dependsOn: ['credential', 'domain'],
},
{
id: 'issueKey',
title: 'Issue',
type: 'file-selector',
selectorKey: 'jira.issues',
dependsOn: ['credential', 'domain', 'projectId'],
required: true,
},
])
)
const states = new Map<string, WorkflowState>([
[
'wf-src',
sourceState(
'jira',
{
credential: { value: 'cred-src' },
domain: { value: 'example.atlassian.net' },
projectId: { value: 'project-basic-stale' },
manualProjectId: { value: 'project-advanced' },
issueKey: { value: 'ADV-1' },
},
{ canonicalModes: { projectId: 'advanced' } }
),
],
])
const result = collectForkDependentReconfigs([replaceItem], states, resolve)
expect(result).toHaveLength(1)
expect(result[0]).toMatchObject({
subBlockKey: 'issueKey',
context: {
domain: 'example.atlassian.net',
projectId: 'project-advanced',
},
})
})
it('emits a credential-dependent selector nested inside a tool-input tool', () => {
vi.mocked(getBlock).mockImplementation((type) => {
if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }])
@@ -109,7 +109,9 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
chaining,
out,
} = params
const fullContext = buildSelectorContextFromBlock(contextBlockType, contextSubBlocks)
const fullContext = buildSelectorContextFromBlock(contextBlockType, contextSubBlocks, {
canonicalModes,
})
const canonicalIndex = buildCanonicalIndex(config.subBlocks)
const gates = createCanonicalModeGates(config.subBlocks, values, canonicalModes)
const configById = new Map(config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))