Skip to content

Commit b8e5039

Browse files
author
Ricardo DeMatos
committed
fix(managed-agent): address 3 medium-severity review comments
Follow-up on Cursor Bugbot's review of the last few commits. Three related, but independent, correctness fixes. Files were silently dropped from cloud sessions - The cloud block declares `columns: ['File ID', 'Mount path']`. The table subblock stores each row's cells keyed by exactly those column strings — but `normalizeFiles` only knew about `Key`/`Value` (older shape) and flat `{fileId, mountPath}`, so every file row was dropped and no files ever reached the session. - Fix: `normalizeFiles` now recognizes `File ID` / `Mount path` (block's declared columns), `file_id` / `mount_path` (snake case), the older `Key` / `Value`, and the flat `{fileId, mountPath}` shapes. Regression test added. Stale collapsed-row labels when Claude Workspace changes - The managed-agent collapsed rows hydrate friendly names by reading the SIBLING `connection` value from `allSubBlockValues`. But `SubBlockRow`'s memo compare only checked this row's own value, so the memo would keep showing agent/env/vault/memory labels tied to the previous connection until each row's own stored value happened to change. - Fix: extend `areSubBlockRowPropsEqual` to also compare a small set of known sibling-dependency keys (`connection`, `oauthCredential`, `credential`). Any of them changing invalidates the memo so the hydration hooks re-run with the new sibling value. Empty tables re-seeded deployer defaults on remount - The table init effect early-exited when `storeValue.length > 0`, but treated `[]` as "unseeded" and re-ran `fetchDefaultRows` / `defaultRows`. A self-hosted block whose Session-parameters table was intentionally cleared to zero rows would get repopulated on every remount, clobbering the author's choice. - Fix: only seed when `storeValue === undefined || storeValue === null`. A stored `[]` — the author's explicit "no rows" — is now preserved. Tests: 92 pass (30 normalizer, up from 29; regression test for the `File ID` / `Mount path` shape).
1 parent 1a4389f commit b8e5039

4 files changed

Lines changed: 57 additions & 18 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/table/table.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,17 +265,21 @@ export function Table({
265265
)
266266

267267
/**
268-
* Initialize the table when the component mounts and the store value is
269-
* missing/empty. Precedence:
268+
* Initialize the table when the component mounts and the store value has
269+
* never been set. Precedence:
270270
* 1. `fetchDefaultRows` (async) — used when defaults live server-side
271271
* (e.g. deployer env-var read via an API route).
272272
* 2. `defaultRows` (sync) — static seeds declared on the block config.
273273
* 3. Single empty row — the pre-existing behavior.
274-
* Existing store values are never touched.
274+
*
275+
* We only seed when `storeValue` is `null` / `undefined` — a stored
276+
* `[]` means the workflow author intentionally cleared every row and
277+
* must NOT be treated as "unseeded" (would re-seed defaults on every
278+
* remount and clobber the empty state they chose).
275279
*/
276280
useEffect(() => {
277281
if (isPreview || disabled) return
278-
if (Array.isArray(storeValue) && storeValue.length > 0) return
282+
if (storeValue !== undefined && storeValue !== null) return
279283
let cancelled = false
280284
const seedWith = (rows: Array<{ cells: Record<string, string> }> | undefined) => {
281285
if (cancelled) return

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,18 @@ const areSubBlockRowPropsEqual = (
106106
const nextValue = subBlockId ? nextProps.allSubBlockValues?.[subBlockId]?.value : undefined
107107
const valueEqual = prevValue === nextValue || isEqual(prevValue, nextValue)
108108

109+
// Some subblocks resolve their friendly-name label using SIBLING values
110+
// (e.g. managed-agent rows key off the selected `connection` id). If a
111+
// sibling changes but this row's own value doesn't, the memo would keep
112+
// showing a label tied to the previous sibling. Compare a small set of
113+
// known dependency keys explicitly so hydration stays fresh.
114+
const depFields = ['connection', 'oauthCredential', 'credential'] as const
115+
const depsEqual = depFields.every((field) => {
116+
const prev = prevProps.allSubBlockValues?.[field]?.value
117+
const next = nextProps.allSubBlockValues?.[field]?.value
118+
return prev === next || isEqual(prev, next)
119+
})
120+
109121
return (
110122
prevProps.title === nextProps.title &&
111123
prevProps.value === nextProps.value &&
@@ -116,6 +128,7 @@ const areSubBlockRowPropsEqual = (
116128
prevProps.blockId === nextProps.blockId &&
117129
prevProps.blockType === nextProps.blockType &&
118130
valueEqual &&
131+
depsEqual &&
119132
prevProps.displayAdvancedOptions === nextProps.displayAdvancedOptions &&
120133
prevProps.canonicalIndex === nextProps.canonicalIndex &&
121134
prevProps.canonicalModeOverrides === nextProps.canonicalModeOverrides

apps/sim/tools/managed_agent/normalizers.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,22 @@ describe('normalizeFiles', () => {
113113
])
114114
})
115115

116+
it("accepts the block's declared column names (`File ID` / `Mount path`)", () => {
117+
// The cloud block declares `columns: ['File ID', 'Mount path']`, so
118+
// the table subblock stores each row keyed by exactly those strings.
119+
// Regression test — the previous normalizer only knew about
120+
// `Key`/`Value` and silently dropped every row.
121+
expect(
122+
normalizeFiles([
123+
{ id: 'r1', cells: { 'File ID': 'file_1', 'Mount path': '/data/one' } },
124+
{ id: 'r2', cells: { 'File ID': 'file_2', 'Mount path': '' } },
125+
])
126+
).toEqual([
127+
{ fileId: 'file_1', mountPath: '/data/one' },
128+
{ fileId: 'file_2' },
129+
])
130+
})
131+
116132
it('drops rows with no file id and rows that are not objects', () => {
117133
expect(
118134
normalizeFiles([

apps/sim/tools/managed_agent/normalizers.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,15 @@ export function isTruthyAck(value: unknown): boolean {
4040
}
4141

4242
/**
43-
* Coerces the block's file table (a JSON array of `{fileId, mountPath?}`
44-
* or the raw table-subblock shape `Array<{Key: string, Value: string}>`)
45-
* into the tidy shape the session-client expects. Silently drops rows
46-
* missing a file id.
43+
* Coerces the block's file table into the tidy shape the session-client
44+
* expects. Silently drops rows missing a file id.
45+
*
46+
* The table subblock stores rows as `WorkflowTableRow[]` with `cells`
47+
* keyed by the column-header STRINGS the block declared. The cloud
48+
* Managed Agents block uses `columns: ['File ID', 'Mount path']`, so
49+
* cells arrive as `{ 'File ID': '...', 'Mount path': '...' }`. We also
50+
* accept the older `{Key, Value}` and the flat `{fileId, mountPath}`
51+
* shapes so hand-authored or legacy stored values still resolve.
4752
*/
4853
export function normalizeFiles(
4954
value: unknown
@@ -57,19 +62,20 @@ export function normalizeFiles(
5762
record.cells && typeof record.cells === 'object'
5863
? (record.cells as Record<string, unknown>)
5964
: record
65+
const readString = (key: string): string | undefined =>
66+
typeof cells[key] === 'string' ? (cells[key] as string) : undefined
6067
const fileId =
61-
typeof cells.fileId === 'string'
62-
? cells.fileId
63-
: typeof cells.Key === 'string'
64-
? cells.Key
65-
: ''
68+
readString('fileId') ??
69+
readString('File ID') ??
70+
readString('file_id') ??
71+
readString('Key') ??
72+
''
6673
if (!fileId.trim()) continue
6774
const mountPath =
68-
typeof cells.mountPath === 'string'
69-
? cells.mountPath
70-
: typeof cells.Value === 'string'
71-
? cells.Value
72-
: undefined
75+
readString('mountPath') ??
76+
readString('Mount path') ??
77+
readString('mount_path') ??
78+
readString('Value')
7379
out.push({
7480
fileId: fileId.trim(),
7581
...(mountPath && mountPath.trim() ? { mountPath: mountPath.trim() } : {}),

0 commit comments

Comments
 (0)