Skip to content

Commit d3ce970

Browse files
authored
improvement(cleanup): tighten the agent-harness changes from the latest audit (#9650)
- CLI parse-error guidance branches on commander's error codes, reads hidden siblings through Help.visibleCommands, and names a misplaced positional only when unambiguous (no unknown option, one missing required flag, one extra operand); the hint uses the option's long flag, not '-w,' - body-variant shape errors use isRecordLike, compute the shape once, and show a generic example - workflow lint builds sections eagerly, derives hasWorkflowLintIssues from them, and phrases Run-button advice from the issue's fields - shared quotedBlockLabel; formatQuotedNameList takes a quote formatter - grep marks skipped non-text files instead of threading a callback - drop dead run-from-block loop branch and unreachable run-hook guard - shared empty lint report test factory; tighter assertions
1 parent 6ff4f32 commit d3ce970

26 files changed

Lines changed: 360 additions & 344 deletions

File tree

‎apps/sim/app/api/v2/workflows/[workflowId]/deploy/route.test.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
v2RouteMocks,
77
} from '@sim/testing'
88
import { createPersonalApiKeyPrincipal } from '@sim/testing/factories/principal.factory'
9+
import { createEmptyWorkflowLintReport } from '@sim/testing/factories/workflow-lint.factory'
910
import { createRouteContext } from '@sim/testing/helpers/http'
1011
import { auditMock } from '@sim/testing/mocks/audit.mock'
1112
import { createMockRequest } from '@sim/testing/mocks/request.mock'
@@ -43,13 +44,7 @@ const sideEffectWarning =
4344
'Deployment activation completed, and post-activation notifications are queued.'
4445

4546
const unquotedRowJson = {
46-
sources: [],
47-
sinks: [],
48-
orphanBlocks: [],
49-
emptyOutgoingPorts: [],
50-
invalidBranchPorts: [],
51-
invalidConnectionTargets: [],
52-
fieldIssues: [],
47+
...createEmptyWorkflowLintReport(),
5348
unresolvedReferences: [
5449
{
5550
blockId: 'insert',
@@ -60,9 +55,6 @@ const unquotedRowJson = {
6055
reason: 'unquoted-json-string: quote it',
6156
},
6257
],
63-
tableFieldIssues: [],
64-
manualRunIssue: null,
65-
notes: [],
6658
}
6759

6860
describe('POST /api/v2/workflows/[workflowId]/deploy', () => {

‎apps/sim/app/api/v2/workflows/[workflowId]/operations/route.test.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
v2RateLimiterModuleMock,
66
v2RouteMocks,
77
} from '@sim/testing'
8+
import { createEmptyWorkflowLintReport } from '@sim/testing/factories/workflow-lint.factory'
89
import { NextRequest } from 'next/server'
910
import { beforeEach, describe, expect, it, vi } from 'vitest'
1011

@@ -41,20 +42,7 @@ const DROPPED_INPUT = {
4142
error: 'Invalid credential ID',
4243
}
4344

44-
/** An empty report, with every field the contract publishes. */
45-
const LINT = {
46-
sources: [],
47-
sinks: [],
48-
orphanBlocks: [],
49-
emptyOutgoingPorts: [],
50-
invalidBranchPorts: [],
51-
invalidConnectionTargets: [],
52-
fieldIssues: [],
53-
unresolvedReferences: [],
54-
tableFieldIssues: [],
55-
manualRunIssue: null,
56-
notes: [],
57-
}
45+
const LINT = createEmptyWorkflowLintReport()
5846

5947
const auth = {
6048
principal: { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'personal-key-1' },

‎apps/sim/app/api/v2/workflows/[workflowId]/state/route.test.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
v2RateLimiterModuleMock,
66
v2RouteMocks,
77
} from '@sim/testing'
8+
import { createEmptyWorkflowLintReport } from '@sim/testing/factories/workflow-lint.factory'
89
import { NextRequest } from 'next/server'
910
import { beforeEach, describe, expect, it, vi } from 'vitest'
1011

@@ -63,19 +64,7 @@ function putRequest(body: unknown) {
6364
})
6465
}
6566

66-
const EMPTY_LINT = {
67-
sources: [],
68-
sinks: [],
69-
orphanBlocks: [],
70-
emptyOutgoingPorts: [],
71-
invalidBranchPorts: [],
72-
invalidConnectionTargets: [],
73-
fieldIssues: [],
74-
unresolvedReferences: [],
75-
tableFieldIssues: [],
76-
manualRunIssue: null,
77-
notes: [],
78-
}
67+
const EMPTY_LINT = createEmptyWorkflowLintReport()
7968

8069
describe('/api/v2/workflows/[workflowId]/state', () => {
8170
beforeEach(() => {

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ export function useWorkflowExecution() {
10761076
selectedOutputs = chatStore.getState().getSelectedWorkflowOutput(activeWorkflowId)
10771077
}
10781078

1079-
let startBlockId: string | undefined
1079+
let startBlockId: string
10801080
let finalWorkflowInput = workflowInput
10811081

10821082
if (isExecutingFromChat) {
@@ -1128,18 +1128,6 @@ export function useWorkflowExecution() {
11281128
}
11291129
}
11301130

1131-
if (!startBlockId) {
1132-
const error = new WorkflowValidationError(
1133-
'No valid trigger block found to start execution',
1134-
'validation',
1135-
'validation',
1136-
'Workflow Validation'
1137-
)
1138-
logger.error('No startBlockId found after trigger search')
1139-
if (activeWorkflowId) finishOwnedExecution(activeWorkflowId, persistenceExecution)
1140-
throw error
1141-
}
1142-
11431131
logger.info('Final execution setup:', {
11441132
startBlockId,
11451133
isExecutingFromChat,
@@ -1316,7 +1304,7 @@ export function useWorkflowExecution() {
13161304
}
13171305

13181306
// Add trigger block to executed blocks so downstream blocks can use run-from-block
1319-
if (data.success && startBlockId) {
1307+
if (data.success) {
13201308
executedBlockIds.add(startBlockId)
13211309
}
13221310

‎apps/sim/executor/utils/run-from-block.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ describe('validateRunFromBlock', () => {
200200
subflowType: 'loop',
201201
}),
202202
])
203+
dag.loopConfigs.set('loop-1', { id: 'loop-1', nodes: ['A'], iterations: 2, loopType: 'for' })
203204
const executedBlocks = new Set(['A'])
204205

205206
const result = validateRunFromBlock('A', dag, executedBlocks)

‎apps/sim/executor/utils/run-from-block.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ export function validateRunFromBlock(
230230
}
231231

232232
if (node) {
233-
if (node.metadata.isLoopNode) return insideSubflow(node.metadata.subflowId, 'loop')
233+
// A branch-clone id (`block₍0₎`) is in no subflow config, so findOutermostContainer misses it.
234234
if (node.metadata.isParallelBranch) return insideSubflow(node.metadata.subflowId, 'parallel')
235235

236236
if (node.metadata.isSentinel) {

‎apps/sim/lib/api/contracts/v2/workflow-graph.test.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createEmptyWorkflowLintReport } from '@sim/testing/factories/workflow-lint.factory'
12
import { describe, expect, it } from 'vitest'
23
import {
34
v2ApplyWorkflowOperationsDataSchema,
@@ -92,20 +93,7 @@ describe('v2WorkflowGraphSchema', () => {
9293
})
9394
})
9495

95-
/** An empty report carrying every field the lint schema requires. */
96-
const EMPTY_LINT = {
97-
sources: [],
98-
sinks: [],
99-
orphanBlocks: [],
100-
emptyOutgoingPorts: [],
101-
invalidBranchPorts: [],
102-
invalidConnectionTargets: [],
103-
fieldIssues: [],
104-
unresolvedReferences: [],
105-
tableFieldIssues: [],
106-
manualRunIssue: null,
107-
notes: [],
108-
}
96+
const EMPTY_LINT = createEmptyWorkflowLintReport()
10997

11098
/** A report exercising every finding kind the lint schema publishes. */
11199
const FULL_LINT = {

‎apps/sim/lib/mothership/agent-cli/engines/lint-scope.test.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
createSessionPrincipal,
44
createWorkspaceApiKeyPrincipal,
55
} from '@sim/testing/factories/principal.factory'
6+
import { createEmptyWorkflowLintReport } from '@sim/testing/factories/workflow-lint.factory'
67
import {
78
secretsUseCasesMock,
89
secretsUseCasesMockFns,
@@ -82,19 +83,7 @@ describe('Mothership workflow lint scope', () => {
8283
})
8384
mocks.permission.mockResolvedValue('read')
8485
mocks.snapshot.mockResolvedValue({ workflowRecord: { id: 'wf-1' }, normalizedData: graph })
85-
mocks.report.mockResolvedValue({
86-
sources: [],
87-
sinks: [],
88-
orphanBlocks: [],
89-
emptyOutgoingPorts: [],
90-
invalidBranchPorts: [],
91-
invalidConnectionTargets: [],
92-
fieldIssues: [],
93-
unresolvedReferences: [],
94-
tableFieldIssues: [],
95-
manualRunIssue: null,
96-
notes: [],
97-
})
86+
mocks.report.mockResolvedValue(createEmptyWorkflowLintReport())
9887
mocks.secrets.mockResolvedValue({ secrets: [{ envKey: 'WORKFLOW_KEY' }], nextCursorKeys: null })
9988
})
10089

‎apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ const PLATFORM_SCOPES: ReadonlySet<Scope> = new Set<Scope>(['blocks', 'tools'])
6161

6262
interface GrepRuntime extends AgentCliRuntime {
6363
reportIncomplete(message: string): void
64-
reportNonText(): void
6564
}
6665

6766
/**
@@ -121,6 +120,11 @@ export interface GrepMaterialized {
121120
label: string
122121
id: string
123122
text: string | null
123+
/**
124+
* Skipped unread because it has no text to extract: a note, not an incomplete search.
125+
* Still returned, so `--in` naming it is not an unknown selector.
126+
*/
127+
nonText?: true
124128
}
125129

126130
interface Page {
@@ -389,13 +393,11 @@ async function fetchAll(
389393
if (scope !== 'files') {
390394
return mapConcurrent(entries, FETCH_CONCURRENCY, (item) => FETCHERS[scope](runtime, item))
391395
}
392-
// Skipped files keep their identity, so `--in` naming one is not an unknown selector.
393396
const skipped: GrepMaterialized[] = []
394397
const readable: IndexEntry[] = []
395398
for (const item of entries) {
396399
if (lacksText(item.raw)) {
397-
runtime.reportNonText()
398-
skipped.push({ scope, id: item.id, label: item.label, text: null })
400+
skipped.push({ scope, id: item.id, label: item.label, text: null, nonText: true })
399401
} else readable.push(item)
400402
}
401403
if (readable.length > MAX_GREP_FILES) {
@@ -589,15 +591,10 @@ export interface GrepSource {
589591
registry?: AgentCliRuntime['resolvedSecretTraceRegistry']
590592
/** The only scopes this corpus holds, and the refusal for a search that names another. */
591593
scopeLimit?: { scopes: readonly Scope[]; refusal: string }
592-
/**
593-
* `reportNonText` counts a file skipped unread because it has no text to extract: a note,
594-
* not an incomplete search. The skipped file is still returned with null text.
595-
*/
596594
materialize(
597595
scopes: Scope[],
598596
nameFilter: string | undefined,
599-
reportIncomplete: (message: string) => void,
600-
reportNonText: () => void
597+
reportIncomplete: (message: string) => void
601598
): Promise<GrepMaterialized[]>
602599
}
603600

@@ -652,10 +649,9 @@ export async function executeGrep(positionals: string[], flags: AgentCliFlags, s
652649
issueCount++
653650
if (issues.length < 10) issues.push(clip(message))
654651
}
655-
let nonTextCount = 0
656-
const reportNonText = () => {
657-
nonTextCount++
658-
}
652+
const candidates = await source.materialize(searched, nameFilter, reportIncomplete)
653+
source.signal?.throwIfAborted()
654+
const nonTextCount = candidates.filter((candidate) => candidate.nonText).length
659655
const finish = (stdout: string) => {
660656
const skipped = nonTextCount
661657
? `Skipped ${nonTextCount} file${nonTextCount === 1 ? '' : 's'} with no searchable text (images, media, archives, and other binary formats).`
@@ -669,8 +665,6 @@ export async function executeGrep(positionals: string[], flags: AgentCliFlags, s
669665
stderr: [incomplete, skipped].filter(Boolean).join('\n'),
670666
}
671667
}
672-
const candidates = await source.materialize(searched, nameFilter, reportIncomplete, reportNonText)
673-
source.signal?.throwIfAborted()
674668
/**
675669
* A resource nothing in the searched worlds answers to is a wrong selector, not a
676670
* search with no hits — a silent "No matches" would hide the misspelling.
@@ -758,8 +752,8 @@ export const universalGrepCommand: AgentCliEngine = {
758752
executeGrep(positionals, flags, {
759753
signal: runtime.signal,
760754
registry: runtime.resolvedSecretTraceRegistry,
761-
async materialize(scopes, nameFilter, reportIncomplete, reportNonText) {
762-
const bounded = gatedRuntime({ ...runtime, reportIncomplete, reportNonText })
755+
async materialize(scopes, nameFilter, reportIncomplete) {
756+
const bounded = gatedRuntime({ ...runtime, reportIncomplete })
763757
return nameFilter
764758
? materializeWithin(bounded, scopes, nameFilter)
765759
: (await settleAll(scopes.map((scope) => materializeScope(bounded, scope)))).flat()

‎apps/sim/lib/mothership/agent-cli/project-file-grep.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ export function grepProjectFiles(
3535
scopes: ['files'],
3636
refusal: 'Project grep supports only --scope files. Search workspace resources separately.',
3737
},
38-
async materialize(_scopes, nameFilter, reportIncomplete, reportNonText) {
39-
// Kept so `--in` naming an unread non-text file is not an unknown selector.
38+
async materialize(_scopes, nameFilter, reportIncomplete) {
4039
const skipped: GrepMaterialized[] = []
4140
const selectedFiles: {
4241
item: Awaited<ReturnType<typeof listProjectFiles.execute>>['files'][number]
@@ -72,8 +71,7 @@ export function grepProjectFiles(
7271
type: observedMediaType(item.type, item.name),
7372
})
7473
) {
75-
reportNonText()
76-
skipped.push({ scope: 'files', id: item.id, label, text: null })
74+
skipped.push({ scope: 'files', id: item.id, label, text: null, nonText: true })
7775
continue
7876
}
7977
if (selectedFiles.length >= MAX_GREP_FILES) {

0 commit comments

Comments
 (0)