diff --git a/src/browser/components/AutomationModal/AutomationModal.tsx b/src/browser/components/AutomationModal/AutomationModal.tsx index b7c9df8f07..b13b31ba55 100644 --- a/src/browser/components/AutomationModal/AutomationModal.tsx +++ b/src/browser/components/AutomationModal/AutomationModal.tsx @@ -34,6 +34,12 @@ import { workflowScheduleIntervalMinutesToMs, } from "@/browser/utils/workflowScheduleIntervalMinutes"; +// Fallback messages shown when the save/remove RPC fails without a specific +// error string. Deduplicated so the wording stays consistent across the +// pre-check, RPC-result, and thrown-error paths below. +const SAVE_AUTOMATION_ERROR_MESSAGE = "Failed to save automation."; +const REMOVE_AUTOMATION_ERROR_MESSAGE = "Failed to remove automation."; + interface AutomationModalProps { open: boolean; projectPath: string; @@ -308,7 +314,7 @@ export function AutomationModal(props: AutomationModalProps) { schedule: workspaceSchedule, }); if (!result.success) { - setSaveError(result.error ?? "Failed to save automation."); + setSaveError(result.error ?? SAVE_AUTOMATION_ERROR_MESSAGE); return false; } } else { @@ -327,7 +333,7 @@ export function AutomationModal(props: AutomationModalProps) { }, }); if (!result.success) { - setSaveError(result.error ?? "Failed to save automation."); + setSaveError(result.error ?? SAVE_AUTOMATION_ERROR_MESSAGE); return false; } } @@ -335,7 +341,7 @@ export function AutomationModal(props: AutomationModalProps) { await refreshProjects(); return true; } catch (error) { - setSaveError(getErrorMessage(error) || "Failed to save automation."); + setSaveError(getErrorMessage(error) || SAVE_AUTOMATION_ERROR_MESSAGE); return false; } finally { setIsSaving(false); @@ -388,7 +394,7 @@ export function AutomationModal(props: AutomationModalProps) { scheduleId: props.projectWorkflowSchedule.id, }); if (!result.success) { - throw new Error(result.error ?? "Failed to remove automation."); + throw new Error(result.error ?? REMOVE_AUTOMATION_ERROR_MESSAGE); } } else if (isLegacyWorkspaceSchedule) { const result = await api.workspace.setWorkflowSchedule({ @@ -396,13 +402,13 @@ export function AutomationModal(props: AutomationModalProps) { schedule: null, }); if (!result.success) { - throw new Error(result.error ?? "Failed to remove automation."); + throw new Error(result.error ?? REMOVE_AUTOMATION_ERROR_MESSAGE); } } await refreshProjects(); props.onOpenChange(false); } catch (error) { - setSaveError(getErrorMessage(error) || "Failed to remove automation."); + setSaveError(getErrorMessage(error) || REMOVE_AUTOMATION_ERROR_MESSAGE); } finally { setIsSaving(false); } diff --git a/src/browser/features/Tools/WorkflowActionListToolCall.tsx b/src/browser/features/Tools/WorkflowActionListToolCall.tsx index dc3900739f..a164a31516 100644 --- a/src/browser/features/Tools/WorkflowActionListToolCall.tsx +++ b/src/browser/features/Tools/WorkflowActionListToolCall.tsx @@ -20,6 +20,7 @@ import { } from "./Shared/ToolPrimitives"; import { getStatusDisplay, isToolErrorResult, type ToolStatus } from "./Shared/toolUtils"; import { + BlockedBadge, WorkflowBadge, WorkflowJsonBlock, WorkflowKindBadge, @@ -126,7 +127,7 @@ function WorkflowActionListRow(props: { descriptor: WorkflowActionDescriptor }) {action.metadata.effect} ) : ( - blocked + )} blocked; +} + export function WorkflowSection(props: { title: string; children: React.ReactNode; @@ -151,7 +157,7 @@ function WorkflowDefinitionListRow(props: { descriptor: WorkflowDefinitionDescri
{descriptor.scope} - {!descriptor.executable && blocked} + {!descriptor.executable && }
@@ -195,7 +201,7 @@ export function WorkflowDefinitionCard(props: {
{descriptor.name} {descriptor.scope} - {!descriptor.executable && blocked} + {!descriptor.executable && }
{!props.compact && (
{descriptor.description}
diff --git a/src/browser/features/Tools/WorkflowRunToolCall.tsx b/src/browser/features/Tools/WorkflowRunToolCall.tsx index cff4a6c5d2..10d009f036 100644 --- a/src/browser/features/Tools/WorkflowRunToolCall.tsx +++ b/src/browser/features/Tools/WorkflowRunToolCall.tsx @@ -1097,10 +1097,6 @@ export const WorkflowRunToolCall: React.FC = ({ resetKey: runId, }); - const toggleWorkflowExpanded = () => { - toggleExpanded(); - }; - const [actionError, setActionError] = useState(null); const [promotedDefinition, setPromotedDefinition] = useState( null @@ -1461,7 +1457,7 @@ export const WorkflowRunToolCall: React.FC = ({ return ( - + diff --git a/src/browser/hooks/useModelsFromSettings.test.ts b/src/browser/hooks/useModelsFromSettings.test.ts index b3b88a25eb..b2310f94ae 100644 --- a/src/browser/hooks/useModelsFromSettings.test.ts +++ b/src/browser/hooks/useModelsFromSettings.test.ts @@ -87,20 +87,28 @@ const actualRoutingModule = { ...RoutingModule }; const actualAPIModule = { ...APIModule }; const actualPolicyContextModule = { ...PolicyContextModule }; +// Specifiers mocked by this suite, shared by install/restore so the two helpers can never +// drift: a path that gets mocked but not restored would silently re-introduce the +// cross-suite leakage these helpers exist to prevent. +const PROVIDERS_CONFIG_MODULE = "@/browser/hooks/useProvidersConfig"; +const ROUTING_MODULE = "@/browser/hooks/useRouting"; +const API_MODULE = "@/browser/contexts/API"; +const POLICY_CONTEXT_MODULE = "@/browser/contexts/PolicyContext"; + async function installUseModelsModuleMocks() { - await mock.module("@/browser/hooks/useProvidersConfig", () => ({ + await mock.module(PROVIDERS_CONFIG_MODULE, () => ({ ...actualProvidersConfigModule, useProvidersConfig: useProvidersConfigMock, })); - await mock.module("@/browser/hooks/useRouting", () => ({ + await mock.module(ROUTING_MODULE, () => ({ ...actualRoutingModule, useRouting: useRoutingMock, })); - await mock.module("@/browser/contexts/API", () => ({ + await mock.module(API_MODULE, () => ({ ...actualAPIModule, useAPI: () => ({ api: apiMock }), })); - await mock.module("@/browser/contexts/PolicyContext", () => ({ + await mock.module(POLICY_CONTEXT_MODULE, () => ({ ...actualPolicyContextModule, usePolicy: () => ({ status: { state: "disabled" as const }, @@ -112,10 +120,10 @@ async function installUseModelsModuleMocks() { async function restoreUseModelsModuleMocks() { // Bun's mock.module() has no disposer, and mock.restore() does not undo module mocks. // Restore the real modules so this file cannot poison later tests in the same process. - await mock.module("@/browser/hooks/useProvidersConfig", () => actualProvidersConfigModule); - await mock.module("@/browser/hooks/useRouting", () => actualRoutingModule); - await mock.module("@/browser/contexts/API", () => actualAPIModule); - await mock.module("@/browser/contexts/PolicyContext", () => actualPolicyContextModule); + await mock.module(PROVIDERS_CONFIG_MODULE, () => actualProvidersConfigModule); + await mock.module(ROUTING_MODULE, () => actualRoutingModule); + await mock.module(API_MODULE, () => actualAPIModule); + await mock.module(POLICY_CONTEXT_MODULE, () => actualPolicyContextModule); } let cleanupDom: (() => void) | null = null; diff --git a/src/node/services/analytics/etl.ts b/src/node/services/analytics/etl.ts index 41adc6815f..0b3277fe96 100644 --- a/src/node/services/analytics/etl.ts +++ b/src/node/services/analytics/etl.ts @@ -9,9 +9,12 @@ import { getErrorMessage } from "@/common/utils/errors"; import { createDisplayUsage } from "@/common/utils/tokens/displayUsage"; import { log } from "@/node/services/log"; import { toUtcDateString } from "@/node/services/analytics/dateUtils"; -import { CHAT_ARCHIVE_FILE_NAME } from "@/common/constants/paths"; +import { CHAT_FILE_NAME, CHAT_ARCHIVE_FILE_NAME } from "@/common/constants/paths"; -export const CHAT_FILE_NAME = "chat.jsonl"; +// Re-export the canonical chat history filename (defined in constants/paths.ts) +// so existing analytics consumers (workspaceDiscovery, tests) can keep importing +// it from this module without duplicating the "chat.jsonl" literal. +export { CHAT_FILE_NAME }; /** * Sealed pre-boundary history rotates from chat.jsonl into chat-archive.jsonl diff --git a/src/node/services/memoryConsolidationService.ts b/src/node/services/memoryConsolidationService.ts index a119ac2304..f8d2980839 100644 --- a/src/node/services/memoryConsolidationService.ts +++ b/src/node/services/memoryConsolidationService.ts @@ -177,13 +177,22 @@ function findNewestWorkspaceRecord( ); } +/** + * Effective ordering timestamp for a harvest record: when it completed, or when + * it started if still pending. findNewestHarvestRecord and pruneHarvestRecords + * must rank records the same way, so both derive recency from this single key. + */ +function harvestRecordTime(record: MemoryHarvestRecord): number { + return record.completedAt ?? record.startedAt; +} + function findNewestHarvestRecord( records: Record | undefined ): MemoryHarvestRecord | null { if (records === undefined) return null; return Object.values(records).reduce((latest, record) => { - const recordTime = record.completedAt ?? record.startedAt; - const latestTime = latest === null ? -1 : (latest.completedAt ?? latest.startedAt); + const recordTime = harvestRecordTime(record); + const latestTime = latest === null ? -1 : harvestRecordTime(latest); return recordTime > latestTime ? record : latest; }, null); } @@ -231,11 +240,9 @@ function parseHarvestRecords(value: unknown): Record): void { - const ranked = Object.entries(records).sort(([, left], [, right]) => { - const leftTime = left.completedAt ?? left.startedAt; - const rightTime = right.completedAt ?? right.startedAt; - return rightTime - leftTime; - }); + const ranked = Object.entries(records).sort( + ([, left], [, right]) => harvestRecordTime(right) - harvestRecordTime(left) + ); for (const [boundaryKey] of ranked.slice(HARVEST_RECORD_RETENTION)) { delete records[boundaryKey]; } diff --git a/src/node/services/workflows/WorkflowRunner.ts b/src/node/services/workflows/WorkflowRunner.ts index 59647ad75f..5f1c445690 100644 --- a/src/node/services/workflows/WorkflowRunner.ts +++ b/src/node/services/workflows/WorkflowRunner.ts @@ -17,6 +17,7 @@ import { validateJsonSchemaSubset } from "@/common/utils/jsonSchemaSubset"; import { removeCommonJsWorkflowMetadataDeclaration } from "./staticWorkflowMetadata"; import { WORKFLOW_RUNTIME_STDLIB_SOURCE } from "./workflowRuntimeSources.generated"; import type { IJSRuntime, IJSRuntimeFactory } from "@/node/services/ptc/runtime"; +import { isWorkflowRunTaskId } from "@/node/services/tools/taskId"; import { AsyncMutex } from "@/node/utils/concurrency/asyncMutex"; import { AsyncSemaphore } from "@/node/utils/concurrency/asyncSemaphore"; import type { ResolvedWorkflowAction, WorkflowActionRegistry } from "./WorkflowActionRegistry"; @@ -1001,7 +1002,7 @@ export class WorkflowRunner { const run = await this.runStore.getRun(runId); const driftedStep = run.steps.find( (step) => - step.stepId === spec.id && step.inputHash !== inputHash && step.taskId?.startsWith("wfr_") + step.stepId === spec.id && step.inputHash !== inputHash && isWorkflowRunTaskId(step.taskId) ); if (driftedStep != null) { throw new Error( diff --git a/src/node/services/workflows/workflowArgs.ts b/src/node/services/workflows/workflowArgs.ts index 4d342bafaf..ec4b79238d 100644 --- a/src/node/services/workflows/workflowArgs.ts +++ b/src/node/services/workflows/workflowArgs.ts @@ -40,6 +40,11 @@ interface TokenizeResult { const RAW_INPUT_FIELD = "input"; +// Shared by assertObjectSchema and propertySchemas so the two `properties` +// validators can't drift to differing messages for the same failure. +const ARGS_SCHEMA_PROPERTIES_ERROR_MESSAGE = + "Workflow metadata.argsSchema.properties must be an object"; + export function normalizeWorkflowArgsForSource( definitionSource: string, rawArgs: unknown, @@ -146,14 +151,14 @@ function assertObjectSchema(schema: Record): void { throw validationError('Workflow metadata.argsSchema must have type "object"'); } if (!isPlainObject(schema.properties)) { - throw validationError("Workflow metadata.argsSchema.properties must be an object"); + throw validationError(ARGS_SCHEMA_PROPERTIES_ERROR_MESSAGE); } } function propertySchemas(schema: Record): ArgsPropertySchema[] { const rawProperties = schema.properties; if (!isPlainObject(rawProperties)) { - throw validationError("Workflow metadata.argsSchema.properties must be an object"); + throw validationError(ARGS_SCHEMA_PROPERTIES_ERROR_MESSAGE); } return Object.entries(rawProperties).map(([name, rawProperty]) => { diff --git a/src/node/services/workflows/workspaceHostActions.ts b/src/node/services/workflows/workspaceHostActions.ts index 917dde224f..28af4d5683 100644 --- a/src/node/services/workflows/workspaceHostActions.ts +++ b/src/node/services/workflows/workspaceHostActions.ts @@ -118,6 +118,21 @@ const WorkspaceIdInputSchema = z.object({ workspaceId: z.string().min(1), }); +/** + * Metadata JSON Schema for the id-targeted host actions whose only input is a + * workspace id (getLatestAssistantMessage, archive, unarchive). Shared so a new + * id-only action can't drift from the rest — #3583's workspace.unarchive + * otherwise duplicated this block verbatim. Runtime parsing of these inputs + * uses the matching WorkspaceIdInputSchema above. + */ +const WORKSPACE_ID_ONLY_INPUT_SCHEMA = { + type: "object", + properties: { + workspaceId: { type: "string" }, + }, + required: ["workspaceId"], +} as const; + function listedWorkspace(metadata: { id: string; name: string; @@ -524,13 +539,7 @@ const WORKSPACE_HOST_ACTION_DEFINITIONS: Record