diff --git a/changelog.d/fixes/orchestration-followups.md b/changelog.d/fixes/orchestration-followups.md new file mode 100644 index 0000000000..6502c7f40b --- /dev/null +++ b/changelog.d/fixes/orchestration-followups.md @@ -0,0 +1,15 @@ +- **fix(dashboard):** five follow-ups from the Phase 2 Orchestration Canvas review. The History + drawer now shows the memory section for runs that already left the live TTL window: the + persisted `memory_hits` event is parsed into `metadata.memoryHits` with the same defensive + validation the drawer applies, and — because that event is observability rather than a state + transition — it no longer leaks into the timeline, where it had been inheriting the task's + state and rendering as a duplicate transition. The A2A memory recall runs against its own 1.5s + deadline instead of inheriting the memory backend's 30s one; overshooting degrades exactly like + any other recall failure (no hits, task proceeds), and the timer is cleared on both paths. + Repeating a Conductor run carries its `requirements.cli`/`requirements.model` forward, so the + new run is pinned to the same runner profile and model rather than drifting to whatever the + fleet picks. A successful repeat from the Agents tab now focuses the run it created, instead of + leaving the operator on the finished one. And the auth test for `POST /api/conductor/tasks` + moved into the shared `ROUTES` array rather than restating the pattern. + + Closes #12639 diff --git a/src/app/(dashboard)/dashboard/orchestration/OrchestrationPageClient.tsx b/src/app/(dashboard)/dashboard/orchestration/OrchestrationPageClient.tsx index dda844277e..e455e4c499 100644 --- a/src/app/(dashboard)/dashboard/orchestration/OrchestrationPageClient.tsx +++ b/src/app/(dashboard)/dashboard/orchestration/OrchestrationPageClient.tsx @@ -115,6 +115,13 @@ export default function OrchestrationPageClient() { [collapsed, setParams] ); const closeDrawer = useCallback(() => setParams({ node: null }), [setParams]); + const onActionDone = useCallback( + (newNodeId?: string) => { + refetch(); + if (newNodeId) setParams({ node: newNodeId }); + }, + [refetch, setParams] + ); const clearFilters = useCallback( () => setParams({ q: null, state: null, source: null, provider: null }), [setParams] @@ -176,8 +183,17 @@ export default function OrchestrationPageClient() { {tab === "history" && } + {/* A successful repeat hands back the CANVAS id of the task it created: refetch, then + focus it (`?node=`) so the operator lands on the new run instead of staring at the + finished one. No id (approve/cancel, or a creation response without one) keeps the + current selection. The History tab renders its own drawer and deliberately does NOT + navigate (HistoryTab.tsx) — its runs are not addressable in the live snapshot. */} {tab !== "history" && ( - + )} ); diff --git a/src/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer.tsx b/src/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer.tsx index 1480e4e3d5..3a50a1589c 100644 --- a/src/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer.tsx +++ b/src/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer.tsx @@ -4,7 +4,7 @@ import { useTranslations } from "next-intl"; import { StatusDot } from "@/shared/components/flow/StatusDot"; import { orchStateColor, type OrchNode, type OrchState } from "../model/orchestrationTypes"; import { useDrawerDetail } from "./useDrawerDetail"; -import type { DrawerError } from "./useDrawerDetail"; +import type { DrawerError, RepeatOutcome } from "./useDrawerDetail"; import type { CloudAgentTask } from "@/lib/cloudAgent/types"; import type { A2ATask } from "@/lib/a2a/taskManager"; @@ -349,15 +349,18 @@ function RepeatButton({ }: { canRepeat: boolean; busy: boolean; - repeat: () => Promise; - onActionDone: () => void; + repeat: () => Promise; + onActionDone: (newNodeId?: string) => void; onToast: (text: string) => void; t: Translate; }) { const { confirming, onClick } = useTwoClickConfirm(() => { void (async () => { - if (await repeat()) { - onActionDone(); + // The created task's CANVAS id is handed to `onActionDone` so the page can focus it — + // `undefined` when the creation response carried no usable id (plain refetch, no jump). + const { ok, newNodeId } = await repeat(); + if (ok) { + onActionDone(newNodeId ?? undefined); onToast(t("repeatDone")); } })(); @@ -395,8 +398,8 @@ function DrawerActions({ busy: boolean; approve: () => Promise; cancel: () => Promise; - repeat: () => Promise; - onActionDone: () => void; + repeat: () => Promise; + onActionDone: (newNodeId?: string) => void; onToast: (text: string) => void; t: Translate; }) { @@ -488,7 +491,7 @@ export function OrchestrationDrawer({ }: { node: OrchNode | null; onClose: () => void; - onActionDone: () => void; + onActionDone: (newNodeId?: string) => void; }) { const t = useTranslations("orchestration"); const { diff --git a/src/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail.ts b/src/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail.ts index 08262248ac..c941c1f360 100644 --- a/src/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail.ts +++ b/src/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail.ts @@ -143,7 +143,14 @@ function repeatReqForA2a( }; } -/** conductor repeat builder — `POST /api/conductor/tasks` (D1 task-creation route). */ +/** + * conductor repeat builder — `POST /api/conductor/tasks` (D1 task-creation route). + * `cli`/`model` come from the hub's `requirements` (`ConductorTaskDetail`, hubProxy.ts) and are + * carried over so the repeat lands on the SAME runner profile/model the original task was + * pinned to. Both are `z.string().optional()` in the route's Zod: a `null` would 400, so a + * missing requirement must OMIT the field (`undefined`) rather than send `null` — and the two + * are independent (one may be set while the other is not). + */ function repeatReqForConductor(detail: unknown): { url: string; init: RequestInit } | null { const d = detail as ConductorTaskDetail | null; if (!d?.repo || !d?.prompt) return null; @@ -154,6 +161,8 @@ function repeatReqForConductor(detail: unknown): { url: string; init: RequestIni prompt: d.prompt, baseRef: d.base_ref ?? undefined, mode: d.mode, + cli: d.cli ?? undefined, + model: d.model ?? undefined, }), }; } @@ -168,6 +177,36 @@ export function repeatReqFor( return null; } +/** `` when `id` is a non-empty string, `null` otherwise (never a bare prefix). */ +function prefixedNodeId(prefix: string, id: unknown): string | null { + return typeof id === "string" && id.length > 0 ? `${prefix}${id}` : null; +} + +/** + * CANVAS node id of the task a successful repeat just created, from the creation response + * body — `null` whenever the body does not carry a usable id (the caller then simply refetches + * without focusing anything). The canvas addresses nodes by PREFIXED id + * (`mergeSnapshot.ts`), so the raw upstream id is never returned on its own. Response + * envelopes, verified against the live routes: + * - conductor (`POST /api/conductor/tasks`): `{ task_id }`. + * - cloud-agent (`POST /api/v1/agents/tasks`): `{ data: { id } }`. + * - a2a (`POST /a2a`, JSON-RPC `message/send`): `{ result: { task: { id } } }`. + */ +export function newNodeIdFrom(node: OrchNode, body: unknown): string | null { + if (!body || typeof body !== "object") return null; + const b = body as Record; + if (node.id.startsWith("conductor:task:")) return prefixedNodeId("conductor:task:", b.task_id); + if (node.id.startsWith("cloud-agent:")) { + const data = b.data as { id?: unknown } | undefined; + return prefixedNodeId("cloud-agent:", data?.id); + } + if (node.id.startsWith("a2a:")) { + const result = b.result as { task?: { id?: unknown } } | undefined; + return prefixedNodeId("a2a:", result?.task?.id); + } + return null; +} + /** * Unwraps a task-detail GET response to the actual task payload. Each source's * route has its own envelope — verified against the live handlers, not assumed: @@ -258,43 +297,62 @@ function useFetchDetail( * that never happened, so the `/a2a` action also inspects the envelope. Only the numeric * `error.code` is surfaced (`RPC `) — never the upstream `error.message`. */ -async function jsonRpcErrorCode(res: { - json?: () => Promise; -}): Promise { +function jsonRpcErrorCode(body: unknown): number | undefined { + const b = body as { error?: { code?: unknown } } | undefined | null; + const code = b?.error?.code; + return typeof code === "number" ? code : b?.error ? -32603 : undefined; +} + +/** + * Reads an action response body ONCE, tolerating a non-JSON/empty body. A body that cannot be + * parsed is not evidence of failure — the status already stood — so it yields `null` and the + * action stays successful (it just has no new-task id to focus). + */ +async function readJsonBody(res: { json?: () => Promise }): Promise { try { - const body = (await res.json?.()) as { error?: { code?: unknown } } | undefined; - const code = body?.error?.code; - return typeof code === "number" ? code : body?.error ? -32603 : undefined; + return (await res.json?.()) ?? null; } catch { - // A non-JSON / already-consumed body is not evidence of failure — the status stands. - return undefined; + return null; } } +/** Outcome of an action POST: whether it succeeded, plus the parsed body on success. */ +interface ActionOutcome { + ok: boolean; + body: unknown; +} + async function performAction( req: { url: string; init: RequestInit } | null, setActionError: (text: string) => void, clearError: () => void -): Promise { - if (!req) return false; +): Promise { + if (!req) return { ok: false, body: null }; try { const res = await fetch(req.url, req.init); if (!res.ok) throw new Error(`HTTP ${res.status}`); + const body = await readJsonBody(res); if (req.url === "/a2a") { - const code = await jsonRpcErrorCode(res); + const code = jsonRpcErrorCode(body); if (code !== undefined) throw new Error(`RPC ${code}`); } // The banner is not sticky: a retry (or any later action) that works clears whatever // detail/action error was on screen, so the drawer never shows a failure the operator // already recovered from. clearError(); - return true; + return { ok: true, body }; } catch (err) { setActionError(toSafeErrorText(err)); - return false; + return { ok: false, body: null }; } } +/** Result of the drawer's repeat action: success plus the canvas id of the created task. */ +export interface RepeatOutcome { + ok: boolean; + newNodeId: string | null; +} + export function useDrawerDetail(node: OrchNode | null) { const [detail, setDetail] = useState(null); const [isLoading, setIsLoading] = useState(false); @@ -311,8 +369,10 @@ export function useDrawerDetail(node: OrchNode | null) { const { canApprove, canCancel } = deriveActionAvailability(route, node); const repeatReq = node ? repeatReqFor(node, detail) : null; - const runAction = async (req: { url: string; init: RequestInit } | null): Promise => { - if (busy) return false; + const runAction = async ( + req: { url: string; init: RequestInit } | null + ): Promise => { + if (busy) return { ok: false, body: null }; setBusy(true); try { return await performAction(req, setActionError, () => setErrorState(null)); @@ -320,6 +380,8 @@ export function useDrawerDetail(node: OrchNode | null) { setBusy(false); } }; + const runBooleanAction = async (req: { url: string; init: RequestInit } | null) => + (await runAction(req)).ok; return { detail, @@ -330,8 +392,13 @@ export function useDrawerDetail(node: OrchNode | null) { canApprove, canCancel, canRepeat: !!repeatReq && !busy, - approve: () => runAction(route?.approveReq ?? null), - cancel: () => runAction(route?.cancelReq ?? null), - repeat: () => runAction(repeatReq), + approve: () => runBooleanAction(route?.approveReq ?? null), + cancel: () => runBooleanAction(route?.cancelReq ?? null), + // Only the repeat reports a new node id: approve/cancel act on the task already open, so + // there is nothing new to focus (and their responses can echo the SAME task's id back). + repeat: async (): Promise => { + const { ok, body } = await runAction(repeatReq); + return { ok, newNodeId: ok && node ? newNodeIdFrom(node, body) : null }; + }, }; } diff --git a/src/app/api/a2a/tasks/[id]/route.ts b/src/app/api/a2a/tasks/[id]/route.ts index 34f408fa57..8733fa1ded 100644 --- a/src/app/api/a2a/tasks/[id]/route.ts +++ b/src/app/api/a2a/tasks/[id]/route.ts @@ -20,6 +20,40 @@ function safeJsonParse(json: string | null | undefined, fallback: T): T { } const STATE_EVENT_PREFIX = "state:"; +/** Event type written by `collectMemoryHits` (src/lib/a2a/taskExecution.ts). */ +const MEMORY_HITS_EVENT_TYPE = "memory_hits"; +const MEMORY_HIT_FIELDS = ["id", "key", "type", "snippet"] as const; + +interface MemoryHit { + id: string; + key: string; + type: string; + snippet: string; +} + +/** + * Parse a persisted `memory_hits` event's `data_json` into the hits the drawer renders. + * Mirrors `DrawerMemory`'s validation in + * `src/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer.tsx`: `metadata` is + * caller-supplied and unvalidated end to end, so what got persisted can be anything — + * a bare string (`"boom"`, whose `.length` is truthy), an object, or an array carrying entries + * with a non-string `key`/`type`/`snippet` (rendered as React children, so an object there + * would throw "Objects are not valid as a React child" and take the whole drawer down). + * Never throws: a malformed payload degrades to an empty list, so the route answers 200 + * with `metadata: {}` instead of 500. + */ +function parseMemoryHits(dataJson: string | null | undefined): MemoryHit[] { + const raw = safeJsonParse(dataJson, null); + if (!Array.isArray(raw)) return []; + return raw.filter( + (hit): hit is MemoryHit => + !!hit && + typeof hit === "object" && + MEMORY_HIT_FIELDS.every( + (field) => typeof (hit as Record)[field] === "string" + ) + ); +} /** * Reconstitute the in-memory `A2ATask` shape (src/lib/a2a/taskManager.ts) from a persisted @@ -29,22 +63,32 @@ const STATE_EVENT_PREFIX = "state:"; * state each event represents is recovered by stripping that prefix. */ function reconstituteHistoricalTask(row: A2ATaskHistoryRow) { - const input = safeJsonParse<{ skill: string; messages: Array<{ role: string; content: string }> }>( - row.input_json, - { skill: row.skill_id ?? "", messages: [] } - ); + const input = safeJsonParse<{ + skill: string; + messages: Array<{ role: string; content: string }>; + }>(row.input_json, { skill: row.skill_id ?? "", messages: [] }); const artifacts = safeJsonParse(row.output_json, []); - const events = listA2ATaskEvents(row.id).map((event) => { - const data = safeJsonParse<{ message?: string } | null>(event.data_json, null); - const state = event.event_type.startsWith(STATE_EVENT_PREFIX) - ? event.event_type.slice(STATE_EVENT_PREFIX.length) - : row.state; - return { - timestamp: event.created_at, - state, - ...(data?.message !== undefined ? { message: data.message } : {}), - }; - }); + const eventRows = listA2ATaskEvents(row.id); + + // `memory_hits` is observability, not a state transition — it is hydrated into `metadata` + // (Fase 3, Task C1) and kept out of the timeline the drawer renders. + const memoryHits = eventRows + .filter((event) => event.event_type === MEMORY_HITS_EVENT_TYPE) + .flatMap((event) => parseMemoryHits(event.data_json)); + + const events = eventRows + .filter((event) => event.event_type !== MEMORY_HITS_EVENT_TYPE) + .map((event) => { + const data = safeJsonParse<{ message?: string } | null>(event.data_json, null); + const state = event.event_type.startsWith(STATE_EVENT_PREFIX) + ? event.event_type.slice(STATE_EVENT_PREFIX.length) + : row.state; + return { + timestamp: event.created_at, + state, + ...(data?.message !== undefined ? { message: data.message } : {}), + }; + }); return { id: row.id, @@ -53,7 +97,7 @@ function reconstituteHistoricalTask(row: A2ATaskHistoryRow) { input, artifacts, events, - metadata: {}, + metadata: memoryHits.length > 0 ? { memoryHits } : {}, createdAt: row.created_at, updatedAt: row.updated_at, expiresAt: row.updated_at, diff --git a/src/lib/a2a/taskExecution.ts b/src/lib/a2a/taskExecution.ts index 68c8ff0937..ff154f7efa 100644 --- a/src/lib/a2a/taskExecution.ts +++ b/src/lib/a2a/taskExecution.ts @@ -1,6 +1,9 @@ import type { A2ATask, TaskArtifact } from "./taskManager"; import { appendA2ATaskEvent } from "@/lib/db/a2aTasks"; import { memoryManager } from "@/lib/memory/manager"; +import { logger } from "@omniroute/open-sse/utils/logger"; + +const log = logger("A2A_TASKS"); type TaskManagerLike = { updateTask: ( @@ -41,8 +44,22 @@ export interface MemoryHitsDeps { limit?: number; }) => Promise>; appendEvent?: (taskId: string, eventType: string, dataJson?: string) => void; + /** Recall deadline override — tests inject a few ms instead of waiting {@link MEMORY_RECALL_TIMEOUT_MS}. */ + timeoutMs?: number; } +/** + * Task C2 (Orchestration Canvas Fase 3, PR-C): deadline for the observability-only memory + * recall. The recall runs BEFORE the skill handler, so an unbounded one delays the task + * itself — the HTTP memory backend (`genericBackend`) alone defaults to a 30s timeout. + * Overshooting the deadline degrades exactly like any other recall failure: empty hits, + * task proceeds. + */ +export const MEMORY_RECALL_TIMEOUT_MS = 1500; + +/** Internal marker so the catch below can tell a deadline apart from a backend error. */ +class MemoryRecallTimeoutError extends Error {} + /** * Collect the memories consulted for a task's last user message, as pure observability. * @@ -82,21 +99,37 @@ export async function collectMemoryHits( } if (!query || query.trim() === "") return []; + const timeoutMs = deps?.timeoutMs ?? MEMORY_RECALL_TIMEOUT_MS; + let timer: ReturnType | undefined; try { const search = deps?.search ?? (async (cfg: { query: string; apiKeyId: string; limit?: number }) => memoryManager.getPrimaryBackend().search(cfg)); const apiKeyId = task.owner ?? "mcp"; - const results = await search({ query, apiKeyId, limit: 5 }); + const deadline = new Promise((_resolve, reject) => { + timer = setTimeout( + () => reject(new MemoryRecallTimeoutError("memory recall deadline exceeded")), + timeoutMs + ); + }); + const results = await Promise.race([search({ query, apiKeyId, limit: 5 }), deadline]); return results.map((m) => ({ id: m.id, key: m.key, type: m.type, snippet: m.content.slice(0, 200), })); - } catch { + } catch (err) { + if (err instanceof MemoryRecallTimeoutError) { + log.warn( + `Memory recall for task ${task.id} exceeded ${timeoutMs}ms — continuing without hits` + ); + } return []; + } finally { + // Cleared on BOTH paths: a surviving timer holds the event loop open. + if (timer) clearTimeout(timer); } } diff --git a/src/lib/conductor/hubProxy.ts b/src/lib/conductor/hubProxy.ts index 8992aa9257..b0679d14c1 100644 --- a/src/lib/conductor/hubProxy.ts +++ b/src/lib/conductor/hubProxy.ts @@ -45,6 +45,10 @@ export interface ConductorTaskDetail extends FleetTask { tests: unknown; council: unknown; created_at: string | null; + /** Runner profile the task was pinned to (hub `requirements.cli`), `null` when unconstrained. */ + cli: string | null; + /** Model the task was pinned to (hub `requirements.model`), `null` when unconstrained. */ + model: string | null; } // ============ Untrusted hub shapes (parse only what we read) ============ @@ -74,6 +78,13 @@ const hubTaskSchema = z.object({ tests: z.unknown().optional(), }) .nullish(), + // The hub echoes back the `requirements` object `createConductorTask` sends on creation. + // `.catch(null)` keeps an unexpected shape from failing the WHOLE task parse — a single odd + // requirement must not blank the fleet snapshot (the list parses with this same schema). + requirements: z + .object({ cli: z.string().nullish(), model: z.string().nullish() }) + .nullish() + .catch(null), council: z.unknown().optional(), created_at: z.string().optional(), updated_at: z.string().optional(), @@ -159,13 +170,16 @@ export async function getFleetSnapshot(opts: HubProxyOptions = {}): Promise ({ - id: r.id, - name: r.capabilities.name ?? "?", - clis: (r.capabilities.clis ?? []).map((c) => c.profile), - online: r.online !== false, - draining: r.draining === true, - })); + const runners = z + .array(hubRunnerSchema) + .parse(rawRunners) + .map((r) => ({ + id: r.id, + name: r.capabilities.name ?? "?", + clis: (r.capabilities.clis ?? []).map((c) => c.profile), + online: r.online !== false, + draining: r.draining === true, + })); const tasks = z.array(hubTaskSchema).parse(rawTasks).map(toFleetTask); emitFleetTransitions(tasks); return { offline: false, runners, tasks }; @@ -190,6 +204,8 @@ export async function getConductorTaskDetail( tests: t.manifest?.tests ?? null, council: t.council ?? null, created_at: t.created_at ?? null, + cli: t.requirements?.cli ?? null, + model: t.requirements?.model ?? null, }; } catch { return null; diff --git a/task-c1-report.md b/task-c1-report.md new file mode 100644 index 0000000000..61e93e3157 --- /dev/null +++ b/task-c1-report.md @@ -0,0 +1,68 @@ +# Task C1 — History hidrata `memoryHits` (PR-C, Refs #12639) + +**Commit:** `da8014442debd2eca7a326472ab704aa86aefba1` +**Branch:** `feat/orch-fase3-c` · 2 arquivos, +164/-16 + +## O que mudou + +### `src/app/api/a2a/tasks/[id]/route.ts` + +- Novas constantes `MEMORY_HITS_EVENT_TYPE = "memory_hits"` e `MEMORY_HIT_FIELDS = ["id","key","type","snippet"]`, interface `MemoryHit`. +- Nova função `parseMemoryHits(dataJson)`: `safeJsonParse` (try/catch já existente) → `Array.isArray` → filtro por item exigindo os 4 campos como `string`. Nunca lança; payload ruim vira lista vazia. +- `reconstituteHistoricalTask`: `listA2ATaskEvents(row.id)` agora é lido uma vez em `eventRows`; os eventos `memory_hits` são extraídos para `memoryHits` e **filtrados fora** de `events`; `metadata` passou de `{}` fixo para `memoryHits.length > 0 ? { memoryHits } : {}`. + +### `tests/unit/a2a-history-route.test.ts` (+104 linhas, 365 no total — teto 1200 ok) + +7 testes novos: + +1. hidrata `metadata.memoryHits` com 2 hits e o evento de memória NÃO aparece em `events` (timeline fica `["submitted","completed"]`). +2. descarta entradas malformadas (`sem snippet`, `key` objeto, `null`, string, número) e mantém só a válida. + 3–7. tabela de payloads ruins (`{not-json`, string JSON pura, objeto JSON, array 100% inválido, array vazio) ⇒ HTTP **200**, `metadata` `{}`, e o evento de memória continua fora da timeline. + +## Gates (saída real) + +``` +$ node --import tsx/esm --test tests/unit/a2a-history-route.test.ts # ANTES do fix (C1.2) +✔ 10 pré-existentes ✖ 7 novos (falha típica: metadata {} esperado ≠ { memoryHits: [...] } / timeline extra 'completed') + +$ node --import tsx/esm --test tests/unit/a2a-history-route.test.ts # DEPOIS (C1.4) +ℹ tests 17 +ℹ pass 17 +ℹ fail 0 + +$ node --import tsx/esm --test tests/unit/db-a2a-tasks.test.ts +ℹ tests 7 +ℹ pass 7 +ℹ fail 0 + +$ npm run typecheck:core +npm notice run tsc --pretty false -p tsconfig.typecheck-core.json +(sem erros, exit 0) + +$ npx eslint 'src/app/api/a2a/tasks/[id]/route.ts' tests/unit/a2a-history-route.test.ts +eslint exit=0 + +$ git commit (husky ativo, sem --no-verify) +lint-staged: prettier + eslint --fix ✔ +[docs-sync] PASS +[t11:any-budget] PASS +[tracked-artifacts] OK +``` + +## Divergências entre plano e código real + +Nenhuma divergência material. Confirmado no código: + +- `event_type` exato = `"memory_hits"` (`src/lib/a2a/taskExecution.ts:143`, `appendA2ATaskEvent(task.id, "memory_hits", JSON.stringify(hits))`). +- Shape de `listA2ATaskEvents` = `{ event_type, data_json, created_at }` (`src/lib/db/a2aTasks.ts`, `A2ATaskHistoryEventRow`) — sem `id` exposto, ordenação `ORDER BY id ASC`. +- `STATE_EVENT_PREFIX = "state:"` continua governando só os eventos de transição; o `memory_hits` caía no `else` e herdava `row.state`, o que o fazia aparecer como uma transição duplicada na timeline — daí o filtro ser obrigatório, não cosmético. + +Observações (não são divergência): + +- A validação implementada é espelho exata da que já existe no drawer (`DrawerMemory` / `MEMORY_HIT_FIELDS` em `OrchestrationDrawer.tsx:275`), incluindo o motivo dos 4 campos: `type`/`key`/`snippet` são renderizados como React children. +- Zod não foi usado: esta rota não valida body (é GET sem query schema) e a leitura é de JSON persistido, coberto pelo `safeJsonParse` já existente no arquivo. Hard Rule #12 intocada — o catch da rota continua usando `sanitizeErrorMessage`. +- Hard Rule #5 respeitada: nenhum SQL na rota; tudo por `src/lib/db/a2aTasks.ts`. + +## Trabalho paralelo + +`src/lib/a2a/taskExecution.ts` e `tests/unit/a2a-memory-hits.test.ts` (Task C2) aparecem como `M` no `git status` — **não foram tocados nem adicionados** por esta task. O commit contém exclusivamente os 2 arquivos da C1. diff --git a/task-c2-report.md b/task-c2-report.md new file mode 100644 index 0000000000..7a13e81b41 --- /dev/null +++ b/task-c2-report.md @@ -0,0 +1,104 @@ +# Task C2 — timeout no recall de memória (PR-C, Fase 3, Refs #12639) + +Commit: `3ef0aef15be50bf458d3f54078e4c0107533f4b2` — `fix(a2a): bound the memory recall with its own deadline` +Branch: `feat/orch-fase3-c`. Arquivos commitados (só estes dois): + +- `src/lib/a2a/taskExecution.ts` +- `tests/unit/a2a-memory-hits.test.ts` + +## O que mudou + +`src/lib/a2a/taskExecution.ts`: + +- `export const MEMORY_RECALL_TIMEOUT_MS = 1500;` +- `MemoryHitsDeps` ganhou `timeoutMs?: number` (default `MEMORY_RECALL_TIMEOUT_MS`). +- `class MemoryRecallTimeoutError extends Error` (marcador interno, não exportado) só para o + `catch` distinguir deadline de erro de backend. +- `collectMemoryHits` roda `Promise.race([search(...), deadline])`; estouro ⇒ `log.warn` + + `[]`. O contrato best-effort é idêntico ao de hoje: nada propaga, a task segue. +- `finally { if (timer) clearTimeout(timer); }` — limpo nos DOIS caminhos. +- `logger("A2A_TASKS")` importado de `@omniroute/open-sse/utils/logger` (mesmo canal já usado + por `src/lib/a2a/taskManager.ts`). O log não contém `err.message`/`err.stack` — só o id da + task e o deadline (Hard Rule #12; nada disso vai para resposta nenhuma). + +`tests/unit/a2a-memory-hits.test.ts` (+5 testes, node:test, sem SQLite/deps reais): + +1. `MEMORY_RECALL_TIMEOUT_MS` é 1500. +2. `search` que nunca resolve ⇒ `[]` (com asserção de que voltou em <1s, provando que o + deadline injetado é o que valeu). +3. `search` que resolve dentro do prazo ⇒ hits normais. +4. Nenhum timer fica pendurado nos dois caminhos. +5. `executeA2ATaskWithState` completa a task normalmente quando o recall estoura. + +## Por que o teste é rápido + +Deadline injetável via `MemoryHitsDeps.timeoutMs` (o arquivo de teste já é 100% construído +sobre esse DI seam — `search`/`appendEvent` fakes, zero fake timers, zero mock de módulo), então +o caminho de timeout usa `timeoutMs: 5`. Suíte inteira em ~0,9 s de relógio. + +## Decisão de projeto que mudei no meio (importante) + +A primeira implementação tinha `timer.unref?.()` além do `clearTimeout`. Mutation-check +(remover o `clearTimeout` e rodar) mostrou que **o teste do timer pendurado passava mesmo +assim**: `process.getActiveResourcesInfo()` não reporta timer `unref`ado, ou seja o `unref` +mascarava exatamente o defeito que o teste existe para pegar. Tirei o `unref` — a garantia +real é o `clearTimeout` no `finally` — e refiz o mutation-check: + +``` +--- MUTATED (clearTimeout removed) --- +✖ collectMemoryHits leaves no pending timer behind on either path (20.667659ms) +ℹ pass 15 +ℹ fail 1 +--- RESTORED --- +✔ collectMemoryHits leaves no pending timer behind on either path (9.073282ms) +ℹ pass 16 +ℹ fail 0 +``` + +## Gates (saída real) + +C2.2 — vermelho antes da implementação: + +``` +SyntaxError: The requested module '../../src/lib/a2a/taskExecution.ts' does not provide an +export named 'MEMORY_RECALL_TIMEOUT_MS' +ℹ pass 0 +ℹ fail 1 +``` + +C2.4 — `node --import tsx/esm --test tests/unit/a2a-memory-hits.test.ts`: + +``` +ℹ tests 16 +ℹ pass 16 +ℹ fail 0 +``` + +Todas as suítes a2a (`node --import tsx/esm --test $(ls tests/unit/*a2a*.test.ts)`): + +``` +ℹ tests 106 +ℹ suites 2 +ℹ pass 106 +ℹ fail 0 +ℹ duration_ms 12146.220038 +``` + +`npm run typecheck:core` — sem saída de erro (só as linhas `npm notice`), exit 0. +`npx eslint src/lib/a2a/taskExecution.ts tests/unit/a2a-memory-hits.test.ts` — zero achados. +`npx prettier --write` nos dois arquivos (o de teste foi reformatado; re-rodei a suíte depois: +16/16). Hooks do husky rodaram no commit (`any-budget` PASS, `tracked-artifacts` OK) — sem +`--no-verify`. + +## Divergências plano × código + +- **Nenhuma no contrato.** `MemoryHit`, `MemoryHitsDeps`, a assinatura de `collectMemoryHits` e + o kill-switch `OMNIROUTE_A2A_MEMORY_HITS === "0"` estão exatamente como o plano copiou. +- Detalhe não previsto: o plano dizia "estouro ⇒ `[]` + log" sem dizer qual logger. Usei o + mesmo `logger("A2A_TASKS")` de `taskManager.ts` (o `taskExecution.ts` não tinha logger algum). +- O `unref` sugerido pela minha leitura inicial de "sem handle pendurado segurando o processo do + runner" foi descartado por mascarar o teste (ver acima); o requisito continua atendido pelo + `clearTimeout` no `finally`, agora com asserção que tem dentes. +- Trabalho paralelo (C1): `git status` mostrava apenas `task-c1-report.md` untracked; não toquei + nele nem em `src/app/api/a2a/tasks/[id]/route.ts` / `tests/unit/a2a-history-route.test.ts`. + `git add` foi feito nomeando só os meus dois arquivos. diff --git a/task-c3-report.md b/task-c3-report.md new file mode 100644 index 0000000000..80be663229 --- /dev/null +++ b/task-c3-report.md @@ -0,0 +1,159 @@ +# Task C3 — `requirements` no repeat do Conductor + foco na task nova + teste de auth + +Branch `feat/orch-fase3-c` · commit **`a7d430fe9ee3ae1fd1d86ff0083318fc1e96005b`** +(`fix(dashboard): carry conductor requirements and focus the repeated task`, corpo com `Refs #12639`). + +## O que mudou, por item + +### 1. `src/lib/conductor/hubProxy.ts` — `requirements` no schema de detalhe + +- `hubTaskSchema` ganhou + `requirements: z.object({ cli: z.string().nullish(), model: z.string().nullish() }).nullish().catch(null)`. + O `.catch(null)` é deliberado: o MESMO schema parseia a **lista** em `getFleetSnapshot` + (`z.array(hubTaskSchema).parse(rawTasks)`) — sem ele, um `requirements` de formato inesperado + numa única task faria o `parse` inteiro lançar e o snapshot cair para `{offline: true}`. +- `ConductorTaskDetail` ganhou `cli: string | null` e `model: string | null` (com JSDoc); + `getConductorTaskDetail` preenche com `t.requirements?.cli ?? null` / `?? null`. +- Origem do shape: é exatamente o objeto que `createConductorTask` (mesmo arquivo, linhas ~238-241) + envia para o hub (`body.requirements = { cli?, model? }`) — não foi inventado. + +### 2. `drawer/useDrawerDetail.ts` + +- `repeatReqForConductor` agora envia `cli: d.cli ?? undefined` / `model: d.model ?? undefined`. + **`undefined`, nunca `null`** — o Zod real da rota (`src/app/api/conductor/tasks/route.ts`) tem + `cli: z.string().optional()` / `model: z.string().optional()`, e `null` daria 400. `JSON.stringify` + descarta as chaves `undefined`, então o corpo sai idêntico ao de hoje quando não há requirement. + Os dois campos são independentes (um pode existir sem o outro) — há teste para esse caso. +- `jsonRpcErrorCode` deixou de ler `res.json()` e passou a receber o **corpo já parseado**; novo + `readJsonBody(res)` lê o corpo UMA vez (um body só pode ser consumido uma vez, e agora o id da + task nova sai do mesmo corpo). Body não-JSON/vazio ⇒ `null`, sem virar falha (mantém o contrato + anterior: "corpo ilegível não é evidência de falha, o status já valeu"). +- `performAction` passou a devolver `{ ok, body }` em vez de `boolean`. +- Novo `export function newNodeIdFrom(node, body): string | null` — extrai o id da task criada e + devolve **já no formato do canvas** (com prefixo), porque `setParams({ node })` casa contra + `snapshot.nodes.find(n => n.id === nodeId)` e os ids em `mergeSnapshot.ts` são prefixados: + - conductor: `body.task_id` → `conductor:task:` + - cloud-agent: `body.data.id` → `cloud-agent:` + - a2a: `body.result.task.id` → `a2a:` (envelope real de `/a2a` `message/send` v0.3) + Leitura defensiva: id ausente/vazio/não-string ⇒ `null` (nunca um prefixo pelado), corpo não-objeto + ⇒ `null`, nunca lança. +- No hook: `approve`/`cancel` continuam `Promise` (via `runBooleanAction`) — de propósito: + eles agem sobre a task já aberta e a resposta do cloud-agent pode ecoar o id da MESMA task, o que + faria o painel "navegar" para onde já está. Só `repeat` devolve + `RepeatOutcome { ok, newNodeId }` (interface exportada). + +### 3. `drawer/OrchestrationDrawer.tsx` + +- `RepeatButton` consome `RepeatOutcome` e chama `onActionDone(newNodeId ?? undefined)`. +- `onActionDone` virou `(newNodeId?: string) => void` no `RepeatButton`, `DrawerActions` e no + componente exportado. `DrawerActions.run()` (approve/cancel) segue chamando `onActionDone()` sem + argumento. + +### 4. `OrchestrationPageClient.tsx` + +- Novo `onActionDone` memoizado: `refetch()` e, se veio id, `setParams({ node: newNodeId })`. + A aba History renderiza o próprio drawer (`HistoryTab.tsx`) e **não** foi tocada — seu + `onActionDone={() => …}` ignora o argumento extra, então o comportamento lá continua idêntico + (re-amostra `nowMs`, não navega), como o plano pede. + +### 5. Testes + +- `tests/unit/conductor-routes-auth.test.ts`: `src/app/api/conductor/tasks/route.ts` entrou no array + `ROUTES`; o regex do proxy ganhou a alternativa `createConductorTask\(`. +- `tests/unit/conductor-create-route.test.ts`: removido o teste de fonte duplicado + (`route: requireManagementAuth antes de criar a task no hub`), substituído por um comentário + apontando para o array. `fs`/`path` continuam usados pelo resto do arquivo. +- `tests/unit/ui/orchestrationDrawerRepeat.test.tsx` (786 → 971 linhas, cap 1200 — não precisou + dividir): 3 describes novos, 6 testes: + - `repeatReqFor` conductor com `cli`/`model`; com ambos `null` (corpo idêntico ao atual e sem as + strings `cli`/`model`); com só `cli` setado. + - `newNodeIdFrom` nas três fontes (prefixado), corpos malformados ⇒ `null`, fonte sem contrato ⇒ `null`. + - drawer: repeat de conductor faz POST com `cli`/`model` e chama `onActionDone` exatamente uma vez + com `"conductor:task:t_new"`, com o toast `repeatDone`. + +## TDD — evidência + +**C3.2 (vermelho)** `npx vitest run tests/unit/ui/orchestrationDrawerRepeat.test.tsx`: + +``` + Test Files 1 failed (1) + Tests 6 failed | 29 passed (35) +``` + +(falhas: `newNodeIdFrom is not a function` ×3, corpo sem `cli`/`model` ×3 — a última: +`expected { …(4) } to match object { cli: 'claude', model: 'sonnet' }`) + +`node --import tsx/esm --test tests/unit/conductor-routes-auth.test.ts tests/unit/conductor-create-route.test.ts` já passou nesse ponto +(`ℹ pass 11 / fail 0`) — **divergência menor com o plano**: a mudança do array `ROUTES` é ampliação +de cobertura + de-duplicação, não a prova de um defeito, então ela não tinha como "falhar antes". +A rota já chamava `requireManagementAuth` corretamente. + +**C3.4 (verde)**: + +``` +$ npx vitest run tests/unit/ui/orchestrationDrawerRepeat.test.tsx + Test Files 1 passed (1) + Tests 35 passed (35) + +$ node --import tsx/esm --test tests/unit/conductor-routes-auth.test.ts tests/unit/conductor-create-route.test.ts tests/unit/client-bundle-no-server-only-10692.test.ts +✔ src/app/api/conductor/tasks/route.ts: requireManagementAuth antes do proxy ao hub (4.175999ms) +ℹ tests 12 +ℹ pass 12 +ℹ fail 0 + +$ npx vitest run tests/unit/ui/ + Test Files 210 passed (210) + Tests 1200 passed (1200) + Duration 179.85s + +$ node --import tsx/esm --test tests/unit/conductor-hub-proxy.test.ts tests/unit/conductor-fleet-mirror.test.ts tests/unit/conductor-fleet-route.test.ts tests/unit/conductor-delegate.test.ts +ℹ tests 17 +ℹ pass 17 +ℹ fail 0 +``` + +## Gates + +``` +$ npm run typecheck:core +EXIT=0 (sem saída de erro) + +$ npm run check:dashboard-typecheck +[dashboard-typecheck] OK — 206 pre-existing error(s), all within frozen baseline. +EXIT=0 + +$ npx eslint +EXIT=0 (zero erros, zero warnings) + +$ npm run check:file-size +[file-size] OK — 136 arquivos congelados, cap 1200 para novos (4588 arquivos verificados) +[test-file-size] OK — 40 test files congelados, testCap 1200 para novos (5495 test files verificados) +``` + +Hooks do husky rodaram no commit (lint-staged + docs-sync + any-budget + tracked-artifacts, todos PASS); +nada de `--no-verify`. + +## Divergências entre plano e código real + +1. **`newNodeIdFrom` prefixado (não previsto explicitamente no plano).** O plano diz "extrair o id da + task nova (`task_id` no Conductor, `data.id` no Cloud Agent) e chamar `onActionDone` com ele". O id + CRU não serve para `setParams({ node })`: o canvas indexa por id prefixado + (`conductor:task:`, `cloud-agent:`, `a2a:` — `mergeSnapshot.ts` / `routeFor`). Segui o + código: `newNodeIdFrom` já devolve o id de canvas. O plano não citava o a2a; incluí porque o + `repeat` do drawer também atende a2a e o envelope (`result.task.id`) é verificável em + `src/app/a2a/route.ts:250`. +2. **`performAction` "devolve o corpo parseado" — só o `repeat` propaga o id.** `approve`/`cancel` + ficaram `Promise` de propósito (ver item 2 acima): o POST de approve/cancel do cloud-agent + pode devolver o id da MESMA task, e navegar para ela seria ruído. Cumprimento do espírito do plano + ("o drawer extrai o id da task nova") sem o efeito colateral. +3. **`.catch(null)` no `requirements`.** O plano não pedia; adicionei porque `hubTaskSchema` também + parseia a LISTA de tasks, e um campo novo estrito ali seria um vetor de "snapshot vira offline por + causa de um campo estranho de uma task". +4. **A mudança em `conductor-routes-auth.test.ts` não é um teste que falha antes** (explicado acima). +5. `ConductorTaskDetail` estende `FleetTask`; `cli`/`model` foram adicionados só no detalhe (não em + `FleetTask`/`toFleetTask`), porque o repeat lê o DETALHE carregado, não o nó. + +## Fora de escopo (não feito, é da Task C4) + +`changelog.d/fixes/orchestration-followups.md`, o comentário na issue #12639 e a bateria completa +(lint total, test:vitest, check:cycles, i18n) continuam pendentes na C4. diff --git a/tests/unit/a2a-history-route.test.ts b/tests/unit/a2a-history-route.test.ts index c44995075f..a4748bdf63 100644 --- a/tests/unit/a2a-history-route.test.ts +++ b/tests/unit/a2a-history-route.test.ts @@ -259,3 +259,107 @@ test("GET /api/a2a/tasks/[id] still 404s when the task is absent from both memor }); assert.equal(res.status, 404); }); + +/** + * Task C1 (Orchestration Canvas Fase 3, PR-C): the history fallback hydrates + * `metadata.memoryHits` from the persisted `memory_hits` event that `executeA2ATaskWithState` + * writes (src/lib/a2a/taskExecution.ts), so the drawer's "Memory used" section survives a task + * leaving the in-memory TTL window. `data_json` is persisted JSON — every read is defensive: + * a malformed payload degrades to `metadata: {}` and NEVER a 500. + */ +test("GET /api/a2a/tasks/[id] hydrates metadata.memoryHits from the persisted memory_hits event", async () => { + seedRow({ id: "history-memory" }); + a2aTasksDb.appendA2ATaskEvent("history-memory", "state:submitted"); + a2aTasksDb.appendA2ATaskEvent( + "history-memory", + "memory_hits", + JSON.stringify([ + { id: "m1", key: "user.name", type: "factual", snippet: "Diego" }, + { id: "m2", key: "user.tz", type: "factual", snippet: "UTC-3" }, + ]) + ); + a2aTasksDb.appendA2ATaskEvent("history-memory", "state:completed"); + + const res = await detailRoute.GET( + new Request("http://localhost/api/a2a/tasks/history-memory", { + headers: AUTH_HEADERS, + }) as never, + { params: Promise.resolve({ id: "history-memory" }) } + ); + assert.equal(res.status, 200); + const body = (await res.json()) as { + task: { + metadata: { memoryHits?: unknown }; + events: Array<{ state: string }>; + }; + }; + + assert.deepEqual(body.task.metadata.memoryHits, [ + { id: "m1", key: "user.name", type: "factual", snippet: "Diego" }, + { id: "m2", key: "user.tz", type: "factual", snippet: "UTC-3" }, + ]); + // The memory event is not a state transition — it must never reach the timeline. + assert.equal(body.task.events.length, 2); + assert.deepEqual( + body.task.events.map((e) => e.state), + ["submitted", "completed"] + ); +}); + +test("GET /api/a2a/tasks/[id] drops malformed memoryHits entries and keeps the valid ones", async () => { + seedRow({ id: "history-memory-partial" }); + a2aTasksDb.appendA2ATaskEvent( + "history-memory-partial", + "memory_hits", + JSON.stringify([ + { id: "ok", key: "k", type: "t", snippet: "s" }, + { id: "no-snippet", key: "k", type: "t" }, + { id: "object-key", key: { a: 1 }, type: "t", snippet: "s" }, + null, + "boom", + 42, + ]) + ); + + const res = await detailRoute.GET( + new Request("http://localhost/api/a2a/tasks/history-memory-partial", { + headers: AUTH_HEADERS, + }) as never, + { params: Promise.resolve({ id: "history-memory-partial" }) } + ); + assert.equal(res.status, 200); + const body = (await res.json()) as { task: { metadata: { memoryHits?: unknown } } }; + assert.deepEqual(body.task.metadata.memoryHits, [ + { id: "ok", key: "k", type: "t", snippet: "s" }, + ]); +}); + +for (const [label, dataJson] of [ + ["unparseable JSON", "{not-json"], + ["a bare JSON string", JSON.stringify("boom")], + ["a JSON object instead of an array", JSON.stringify({ id: "m1" })], + ["an array whose every entry is malformed", JSON.stringify([{ id: "m1" }, null, 7])], + ["an empty array", JSON.stringify([])], +] as const) { + test(`GET /api/a2a/tasks/[id] answers 200 with metadata {} when memory_hits carries ${label}`, async () => { + const id = `history-memory-${label.replace(/\W+/g, "-")}`; + seedRow({ id }); + a2aTasksDb.appendA2ATaskEvent(id, "state:submitted"); + a2aTasksDb.appendA2ATaskEvent(id, "memory_hits", dataJson); + + const res = await detailRoute.GET( + new Request(`http://localhost/api/a2a/tasks/${id}`, { headers: AUTH_HEADERS }) as never, + { params: Promise.resolve({ id }) } + ); + assert.equal(res.status, 200); + const body = (await res.json()) as { + task: { metadata: Record; events: Array<{ state: string }> }; + }; + assert.deepEqual(body.task.metadata, {}); + // Even a malformed memory event stays out of the timeline. + assert.deepEqual( + body.task.events.map((e) => e.state), + ["submitted"] + ); + }); +} diff --git a/tests/unit/a2a-memory-hits.test.ts b/tests/unit/a2a-memory-hits.test.ts index 9d6a42486f..3e69b76446 100644 --- a/tests/unit/a2a-memory-hits.test.ts +++ b/tests/unit/a2a-memory-hits.test.ts @@ -13,6 +13,7 @@ import assert from "node:assert/strict"; import { collectMemoryHits, executeA2ATaskWithState, + MEMORY_RECALL_TIMEOUT_MS, type MemoryHit, type MemoryHitsDeps, } from "../../src/lib/a2a/taskExecution.ts"; @@ -329,3 +330,87 @@ test("executeA2ATaskWithState never leaks memoryHits into task.input.metadata or tm.destroy(); } }); + +/** + * Task C2 (Orchestration Canvas Fase 3, PR-C): the recall is best-effort, so it must also be + * BOUNDED. Without a deadline a slow memory backend (the HTTP `genericBackend` defaults to a + * 30s timeout) delays the start of every A2A task. The deadline is injectable through + * `MemoryHitsDeps.timeoutMs` so these tests cost milliseconds, not 1.5s of wall clock. + */ +test("MEMORY_RECALL_TIMEOUT_MS is the 1.5s default deadline", () => { + assert.equal(MEMORY_RECALL_TIMEOUT_MS, 1500); +}); + +test("collectMemoryHits returns [] when search never resolves (deadline hit)", async () => { + const deps: MemoryHitsDeps = { + search: () => new Promise(() => {}), // never settles + timeoutMs: 5, + }; + + const task = makeTask(); + const started = Date.now(); + const hits = await collectMemoryHits(task, deps); + + assert.deepEqual(hits, []); + assert.ok(Date.now() - started < 1000, "gave up on the injected deadline, not the default"); +}); + +test("collectMemoryHits returns the hits when search resolves inside the deadline", async () => { + const deps: MemoryHitsDeps = { + search: async () => { + await new Promise((resolve) => setTimeout(resolve, 1)); + return [{ id: "m1", key: "k1", type: "factual", content: "hello" }]; + }, + timeoutMs: 1000, + }; + + const hits = await collectMemoryHits(makeTask(), deps); + + assert.deepEqual(hits, [{ id: "m1", key: "k1", type: "factual", snippet: "hello" }]); +}); + +test("collectMemoryHits leaves no pending timer behind on either path", async () => { + const countTimers = () => process.getActiveResourcesInfo().filter((r) => r === "Timeout").length; + + const before = countTimers(); + + // Success path with a long deadline: the timer must be cleared, not left ticking. + await collectMemoryHits(makeTask(), { + search: async () => [{ id: "m1", key: "k1", type: "factual", content: "hello" }], + timeoutMs: 60_000, + }); + assert.equal(countTimers(), before, "success path cleared its deadline timer"); + + // Timeout path: the timer has fired, so nothing may stay registered either. + await collectMemoryHits(makeTask(), { + search: () => new Promise(() => {}), + timeoutMs: 5, + }); + assert.equal(countTimers(), before, "timeout path left no timer registered"); +}); + +test("executeA2ATaskWithState completes the task normally when memory recall times out", async () => { + const deps: MemoryHitsDeps = { + search: () => new Promise(() => {}), + timeoutMs: 5, + }; + + let completedState: string | undefined; + const tm = { + updateTask: (_taskId: string, state: string) => { + completedState = state; + }, + }; + + const task = makeTask(); + const result = await executeA2ATaskWithState( + tm, + task, + async () => ({ artifacts: [{ type: "text", content: "ok" }], metadata: {} }), + deps + ); + + assert.equal(completedState, "completed"); + assert.deepEqual(result.artifacts, [{ type: "text", content: "ok" }]); + assert.equal("memoryHits" in task.metadata, false); +}); diff --git a/tests/unit/conductor-create-route.test.ts b/tests/unit/conductor-create-route.test.ts index d16d47b1d3..815107fb8b 100644 --- a/tests/unit/conductor-create-route.test.ts +++ b/tests/unit/conductor-create-route.test.ts @@ -57,21 +57,8 @@ test.after(async () => { } }); -test("route: requireManagementAuth antes de criar a task no hub", () => { - const src = fs.readFileSync( - path.join(process.cwd(), "src/app/api/conductor/tasks/route.ts"), - "utf8" - ); - const authAt = src.indexOf("requireManagementAuth("); - assert.ok(authAt > 0, "handler chama requireManagementAuth"); - assert.match(src, /if \(authError\) return authError;/, "curto-circuito no erro de auth"); - const proxyAt = src.indexOf("createConductorTask("); - assert.ok(proxyAt > authAt, "proxy ao hub só depois do gate de auth"); - assert.ok( - !src.includes("CONDUCTOR_HUB_TOKEN"), - "token nunca manuseado na rota (vive no hubProxy)" - ); -}); +// A prova de fonte "requireManagementAuth antes do proxy ao hub" desta rota vive no array +// `ROUTES` de `conductor-routes-auth.test.ts` (uma única implementação para as quatro rotas). test("POST /api/conductor/tasks: body válido + hub ok → 201 {task_id}", async () => { process.env.CONDUCTOR_HUB_URL = await fakeHub({ diff --git a/tests/unit/conductor-routes-auth.test.ts b/tests/unit/conductor-routes-auth.test.ts index 8c35f1f5a6..9dff394bb6 100644 --- a/tests/unit/conductor-routes-auth.test.ts +++ b/tests/unit/conductor-routes-auth.test.ts @@ -8,6 +8,9 @@ const ROUTES = [ "src/app/api/conductor/fleet/route.ts", "src/app/api/conductor/tasks/[id]/route.ts", "src/app/api/conductor/tasks/[id]/cancel/route.ts", + // Rota de criação (repeat do drawer): coberta aqui pelo mesmo padrão, em vez de + // duplicar o teste de fonte dentro de `conductor-create-route.test.ts`. + "src/app/api/conductor/tasks/route.ts", ]; for (const route of ROUTES) { @@ -16,8 +19,13 @@ for (const route of ROUTES) { const authAt = src.indexOf("requireManagementAuth("); assert.ok(authAt > 0, "handler chama requireManagementAuth"); assert.match(src, /if \(authError\) return authError;/, "curto-circuito no erro de auth"); - const proxyAt = src.search(/getFleetSnapshot\(|getConductorTaskDetail\(|cancelConductorTask\(/); + const proxyAt = src.search( + /getFleetSnapshot\(|getConductorTaskDetail\(|cancelConductorTask\(|createConductorTask\(/ + ); assert.ok(proxyAt > authAt, "proxy ao hub só depois do gate de auth"); - assert.ok(!src.includes("CONDUCTOR_HUB_TOKEN"), "token nunca manuseado na rota (vive no hubProxy)"); + assert.ok( + !src.includes("CONDUCTOR_HUB_TOKEN"), + "token nunca manuseado na rota (vive no hubProxy)" + ); }); } diff --git a/tests/unit/ui/orchestrationDrawerRepeat.test.tsx b/tests/unit/ui/orchestrationDrawerRepeat.test.tsx index 761b0d922f..dcbd0d0a5b 100644 --- a/tests/unit/ui/orchestrationDrawerRepeat.test.tsx +++ b/tests/unit/ui/orchestrationDrawerRepeat.test.tsx @@ -9,7 +9,10 @@ vi.mock("next-intl", () => ({ })); import { OrchestrationDrawer } from "@/app/(dashboard)/dashboard/orchestration/drawer/OrchestrationDrawer"; -import { repeatReqFor } from "@/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail"; +import { + newNodeIdFrom, + repeatReqFor, +} from "@/app/(dashboard)/dashboard/orchestration/drawer/useDrawerDetail"; function render(el: React.ReactElement) { const c = document.createElement("div"); @@ -784,3 +787,185 @@ describe("OrchestrationDrawer repeat action (two-click confirm)", () => { cleanup(); }); }); + +describe("repeatReqFor conductor requirements (cli/model)", () => { + const node = { + id: "conductor:task:1", + kind: "work", + source: "conductor", + state: "succeeded", + label: "x", + }; + + it("carries cli/model when the hub detail exposes them (a repeat must land on the same runner profile/model)", () => { + const detail = { + repo: "https://github.com/x/y", + prompt: "fix the bug", + base_ref: "main", + mode: "auto", + cli: "claude", + model: "sonnet", + }; + const req = repeatReqFor(node as never, detail); + expect(JSON.parse(String(req?.init.body))).toEqual({ + repoUrl: "https://github.com/x/y", + prompt: "fix the bug", + baseRef: "main", + mode: "auto", + cli: "claude", + model: "sonnet", + }); + }); + + it("omits cli/model entirely when the detail has them null (the route's Zod treats both as optional strings — `null` would 400)", () => { + const detail = { + repo: "https://github.com/x/y", + prompt: "fix the bug", + base_ref: "main", + mode: "auto", + cli: null, + model: null, + }; + const body = String(repeatReqFor(node as never, detail)?.init.body); + expect(JSON.parse(body)).toEqual({ + repoUrl: "https://github.com/x/y", + prompt: "fix the bug", + baseRef: "main", + mode: "auto", + }); + expect(body).not.toContain("cli"); + expect(body).not.toContain("model"); + }); + + it("carries only the field that is present (cli set, model null — they are independent)", () => { + const detail = { + repo: "https://github.com/x/y", + prompt: "fix the bug", + base_ref: null, + mode: "solo", + cli: "codex", + model: null, + }; + expect(JSON.parse(String(repeatReqFor(node as never, detail)?.init.body))).toEqual({ + repoUrl: "https://github.com/x/y", + prompt: "fix the bug", + mode: "solo", + cli: "codex", + }); + }); +}); + +describe("newNodeIdFrom", () => { + it("maps each source's creation response to the CANVAS node id (prefixed), never the raw id", () => { + const cond = { id: "conductor:task:1", source: "conductor" }; + expect(newNodeIdFrom(cond as never, { task_id: "t2" })).toBe("conductor:task:t2"); + const ca = { id: "cloud-agent:1", source: "cloud-agent" }; + expect(newNodeIdFrom(ca as never, { data: { id: "c2" } })).toBe("cloud-agent:c2"); + const a2a = { id: "a2a:1", source: "a2a" }; + expect(newNodeIdFrom(a2a as never, { result: { task: { id: "n2" } } })).toBe("a2a:n2"); + }); + + it("returns null for a body that does not carry a usable id (never throws, never yields a bare prefix)", () => { + const cond = { id: "conductor:task:1", source: "conductor" }; + expect(newNodeIdFrom(cond as never, null)).toBeNull(); + expect(newNodeIdFrom(cond as never, {})).toBeNull(); + expect(newNodeIdFrom(cond as never, { task_id: "" })).toBeNull(); + expect(newNodeIdFrom(cond as never, { task_id: 7 })).toBeNull(); + expect(newNodeIdFrom(cond as never, "nope")).toBeNull(); + const ca = { id: "cloud-agent:1", source: "cloud-agent" }; + expect(newNodeIdFrom(ca as never, { data: {} })).toBeNull(); + const a2a = { id: "a2a:1", source: "a2a" }; + expect(newNodeIdFrom(a2a as never, { result: {} })).toBeNull(); + }); + + it("returns null for a node whose source has no repeat contract", () => { + const overflow = { id: "overflow:1", kind: "overflow" }; + expect(newNodeIdFrom(overflow as never, { task_id: "t2" })).toBeNull(); + }); +}); + +describe("OrchestrationDrawer repeat focuses the newly created task", () => { + const CONDUCTOR_DETAIL = { + id: "1", + status: "succeeded", + mode: "solo", + repo: "https://github.com/x/y", + runner: null, + summary: null, + branch: null, + error: null, + updated_at: null, + prompt: "fix the bug", + base_ref: "main", + tests: null, + council: null, + created_at: null, + cli: "claude", + model: "sonnet", + }; + + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + function findRepeatButton(c: HTMLElement): HTMLButtonElement { + return Array.from(c.querySelectorAll("button")).find( + (b) => b.textContent?.includes("actionRepeat") || b.textContent?.includes("repeatConfirm") + ) as HTMLButtonElement; + } + + async function repeatOnce(onActionDone: (id?: string) => void) { + const fetchMock = vi.fn((_url: string, init?: RequestInit) => { + if (init?.method === "POST") { + return Promise.resolve({ + ok: true, + status: 201, + json: () => Promise.resolve({ task_id: "t_new" }), + }); + } + return Promise.resolve({ ok: true, json: () => Promise.resolve(CONDUCTOR_DETAIL) }); + }); + vi.stubGlobal("fetch", fetchMock); + const node = { + id: "conductor:task:1", + kind: "work", + source: "conductor", + state: "running", + label: "x", + }; + const { c, cleanup } = render( + {}} onActionDone={onActionDone} /> + ); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + findRepeatButton(c).click(); + }); + await act(async () => { + findRepeatButton(c).click(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + return { c, cleanup, fetchMock }; + } + + it("hands the canvas id of the created task to onActionDone, so the page can focus it", async () => { + const seen: Array = []; + const { c, cleanup, fetchMock } = await repeatOnce((id) => seen.push(id)); + const post = fetchMock.mock.calls.find(([, init]) => (init as RequestInit)?.method === "POST"); + expect(post![0]).toBe("/api/conductor/tasks"); + expect(JSON.parse(String((post![1] as RequestInit).body))).toMatchObject({ + cli: "claude", + model: "sonnet", + }); + expect(seen).toEqual(["conductor:task:t_new"]); + expect(c.textContent).toContain("repeatDone"); + cleanup(); + }); +});