From 687b3d615fff7679571de0930c5171eb0424ff45 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Sun, 2 Aug 2026 23:21:10 -0400 Subject: [PATCH 01/16] refactor(space): relocate coding artifact writers/readers out of daemon core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a WorkflowArtifactProfile domain seam and a CodingArtifactProfile implementation in the coding-workflow layer, then move every coding-specific writer/reader out of daemon core behind it. Daemon core (node-agent-tools, space-runtime, space-runtime-service, task-agent-manager, evolution-episode-service) now references no domain kinds (`pr`, `review`) or coding identifiers (`review-posted-gate`, `pr_url`) — only the profile does. Relocated: - The 5 duplicated `resolvePrUrlForRun` copies → profile.resolvePrimaryLinkUrl (gate data → hook state → artifacts, preferring link kind:'pr'). - The review-posted-gate auto-save (decision kind:'review' round-N) → profile.onGateDataCommitted, fired as a generic gate-write hook. - The 3 kindless-decision result-summary readers → profile.summarizeRunOutcome. Migrate every coding-workflow agent prompt (built-in-workflows, post-approval-merge-template) off the legacy `type`/`append` API onto explicit shapes: PR → link kind:'pr', planner/QA outcome → decision, audit notes (merge_conflict/merge_blocked/cleanup_warning) → note with kind, merge audit → link kind:'merge'. Drop the legacy type/append compat shim from SaveArtifactSchema and the list_artifacts legacy-type filter mapping now that no caller emits them. Update live-query timeline tone classification to read `kind` (new) as well as `_legacyType` (backfilled) for blocker/warning notes. Task #809 (#796 PR C). --- packages/daemon/src/lib/rpc-handlers/index.ts | 13 + .../lib/rpc-handlers/live-query-handlers.ts | 12 +- .../lib/space/evolution-episode-service.ts | 22 +- .../src/lib/space/runtime/artifact-profile.ts | 56 ++++ .../space/runtime/space-runtime-service.ts | 77 +---- .../src/lib/space/runtime/space-runtime.ts | 158 ++------- .../lib/space/runtime/task-agent-manager.ts | 103 +----- .../space/tools/node-agent-tool-schemas.ts | 125 +++---- .../src/lib/space/tools/node-agent-tools.ts | 317 +++--------------- .../lib/space/workflows/built-in-workflows.ts | 28 +- .../workflows/coding-artifact-profile.ts | 205 +++++++++++ .../workflows/post-approval-merge-template.ts | 14 +- 12 files changed, 467 insertions(+), 663 deletions(-) create mode 100644 packages/daemon/src/lib/space/runtime/artifact-profile.ts create mode 100644 packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts diff --git a/packages/daemon/src/lib/rpc-handlers/index.ts b/packages/daemon/src/lib/rpc-handlers/index.ts index 11c17a33a9..903be24a54 100644 --- a/packages/daemon/src/lib/rpc-handlers/index.ts +++ b/packages/daemon/src/lib/rpc-handlers/index.ts @@ -48,6 +48,7 @@ import { NodeExecutionRepository } from '../../storage/repositories/node-executi import { TaskAgentManager } from '../space/runtime/task-agent-manager'; import { ReplyRoutingRegistry } from '../space/runtime/reply-routing-registry'; import { SpaceWorktreeManager } from '../space/managers/space-worktree-manager'; +import { CodingArtifactProfile } from '../space/workflows/coding-artifact-profile'; import { setupSpaceWorkflowHandlers, checkBuiltInWorkflowDriftOnStartup, @@ -615,12 +616,22 @@ export function setupRPCHandlers(deps: RPCHandlerDependencies): RPCHandlerSetupR // Reply Routing Registry — shared between space-agent-tools (register) // and task-agent-tools / node-agent-tools (lookup). const replyRoutingRegistry = new ReplyRoutingRegistry(); + // Domain profile that owns coding-specific artifact semantics (which `link` + // is the PR, which `decision` is the terminal outcome, the review-posted-gate + // history). Injected into the runtime services so daemon core never names + // domain kinds (`pr` / `review`). + const artifactProfile = new CodingArtifactProfile({ + db: deps.db.getDatabase(), + artifactRepo, + gateDataRepo, + }); const evolutionEpisodeService = new EvolutionEpisodeService({ evolutionRepo: deps.db.evolution, spaceRepo, taskRepo: spaceTaskRepo, workflowRunRepo: spaceWorkflowRunRepo, artifactRepo, + artifactProfile, goalService: spaceGoalService, db: deps.db.getDatabase(), taskCreatedEventHub: { @@ -708,6 +719,7 @@ export function setupRPCHandlers(deps: RPCHandlerDependencies): RPCHandlerSetupR goalService: spaceGoalService, evolutionScopeService, evolutionEpisodeService, + artifactProfile, }); // When a space is resumed/started, re-seed skipped schedules and re-run restart @@ -888,6 +900,7 @@ export function setupRPCHandlers(deps: RPCHandlerDependencies): RPCHandlerSetupR goalService: spaceGoalService, evolutionScopeService, externalEventStore: deps.externalEventStore, + artifactProfile, }); deps.commandBus.register('agent.message.inject', async (command) => { diff --git a/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts b/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts index 63edc0d7d2..a19a39888e 100644 --- a/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts +++ b/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts @@ -1217,11 +1217,17 @@ artifact_rows AS ( ELSE 'artifact' END AS category, CASE - -- Legacy notes that record blockers (mapped from unknown legacy types, - -- original meaning kept under _legacyType) warrant a warning tone. + -- Notes that record blockers/warnings warrant a warning tone. Modern + -- audit notes carry the meaning in kind (merge_blocked / merge_conflict + -- / cleanup_warning); rows backfilled from the legacy freeform type + -- system carry it under _legacyType. Match either so old and new data + -- share the tone. WHEN wra.artifact_type = 'note' AND json_valid(wra.data) - AND json_extract(wra.data, '$._legacyType') IN ('merge_blocked', 'cleanup_warning') + AND ( + json_extract(wra.data, '$.kind') IN ('merge_blocked', 'merge_conflict', 'cleanup_warning') + OR json_extract(wra.data, '$._legacyType') IN ('merge_blocked', 'cleanup_warning') + ) THEN 'warning' WHEN wra.artifact_type = 'note' THEN 'progress' WHEN wra.artifact_type = 'link' THEN 'success' diff --git a/packages/daemon/src/lib/space/evolution-episode-service.ts b/packages/daemon/src/lib/space/evolution-episode-service.ts index efecc232d9..b44bb50cdf 100644 --- a/packages/daemon/src/lib/space/evolution-episode-service.ts +++ b/packages/daemon/src/lib/space/evolution-episode-service.ts @@ -35,6 +35,7 @@ import type { WorkflowRunArtifactRepository, } from '../../storage/repositories/workflow-run-artifact-repository'; import type { SpaceGoalService } from './goals/goal-service'; +import type { WorkflowArtifactProfile } from './runtime/artifact-profile'; import { isRunningUnderBun, resolveSDKCliPath } from '../agent/sdk-cli-resolver'; import { Logger } from '../logger'; import { getProviderService, mergeProviderEnvVars } from '../provider-service'; @@ -119,6 +120,12 @@ export interface EvolutionEpisodeServiceDeps { taskRepo: SpaceTaskRepository; workflowRunRepo: SpaceWorkflowRunRepository; artifactRepo: WorkflowRunArtifactRepository; + /** + * Domain artifact profile. Used by the result-artifact gap detector to read + * a run's terminal outcome (coding: the kindless `decision` summary) without + * this service naming domain kinds. + */ + artifactProfile?: WorkflowArtifactProfile; goalService?: Pick; taskIdFactory?: () => string; db?: BunDatabase; @@ -482,18 +489,9 @@ export class EvolutionEpisodeService { let hasResultArtifact = runHasResultArtifact.get(runId); if (hasResultArtifact === undefined) { - // The terminal "result" is a kind-less `decision` carrying a summary - // (legacy result→decision has no kind; review/gate decisions carry a - // kind and are not terminal). - const decisions = this.deps.artifactRepo.listByRun(runId, { - artifactType: 'decision', - }); - hasResultArtifact = decisions.some( - (artifact) => - !artifact.data.kind && - typeof artifact.data.summary === 'string' && - artifact.data.summary.trim().length > 0 - ); + // Delegated to the domain artifact profile (coding: the kindless + // terminal `decision` summary). + hasResultArtifact = this.deps.artifactProfile?.summarizeRunOutcome(runId) != null; runHasResultArtifact.set(runId, hasResultArtifact); } if (!hasResultArtifact) return; diff --git a/packages/daemon/src/lib/space/runtime/artifact-profile.ts b/packages/daemon/src/lib/space/runtime/artifact-profile.ts new file mode 100644 index 0000000000..c938f1d1c1 --- /dev/null +++ b/packages/daemon/src/lib/space/runtime/artifact-profile.ts @@ -0,0 +1,56 @@ +/** + * Workflow Artifact Profile — the domain seam between generic workflow infra + * (daemon core) and a domain layer. + * + * Infra knows the closed SHAPE vocabulary (`link`, `commit_set`, `check`, + * `metric`, `decision`, `note`) but never a domain KIND (`pr`, `review`, …). + * Anything that depends on what a particular kind means — which `link` is the + * run's primary URL, which `decision` is the terminal outcome, what to persist + * when a particular gate fires — lives behind this interface. The coding- + * workflow layer supplies the implementation; infra only calls the methods. + * + * All methods are best-effort: implementations log and return a safe default + * ('' / null / no-op) on error rather than throwing, so a profile failure can + * never break infra control flow. + */ + +/** + * Event passed to {@link WorkflowArtifactProfile.onGateDataCommitted} after a + * gated `send_message` commits a gate-data write (before message delivery). + */ +export interface GateDataCommittedEvent { + runId: string; + nodeId: string; + gateId: string; + /** The committed gate data (after the field merge). */ + gateData: Record; + /** The `data` payload from the originating `send_message` call. */ + messageData?: Record; +} + +export interface WorkflowArtifactProfile { + /** + * Resolve the canonical "primary link" URL for a run — the single URL infra + * treats as THE link: gate-script `prUrl`, merge-template `{{pr_url}}`, and PR + * event subscriptions all read this. The coding profile knows it is the PR + * (`link kind:'pr'`); generic infra does not. Returns '' when none. + */ + resolvePrimaryLinkUrl(runId: string): string; + + /** + * Build a short terminal outcome summary from a run's artifacts, or null when + * the run recorded no outcome. Used by task completion (mark_complete / run + * completion / Forge gap detection). The coding profile reads the kindless + * terminal `decision`. + */ + summarizeRunOutcome(runId: string): string | null; + + /** + * Hook fired after a gated `send_message` commits gate data, before delivery. + * Lets a domain layer persist side-artifacts keyed to that gate. The coding + * profile records a `decision kind:'review'` (round-N) each time the + * review-posted-gate receives a `review_url`. Fire-and-forget from infra's + * perspective: errors are logged by the caller, never propagated. + */ + onGateDataCommitted?(event: GateDataCommittedEvent): Promise | void; +} diff --git a/packages/daemon/src/lib/space/runtime/space-runtime-service.ts b/packages/daemon/src/lib/space/runtime/space-runtime-service.ts index c3a1eae042..2ea44f9453 100644 --- a/packages/daemon/src/lib/space/runtime/space-runtime-service.ts +++ b/packages/daemon/src/lib/space/runtime/space-runtime-service.ts @@ -40,7 +40,7 @@ import type { SpaceWorkflowRepository } from '../../../storage/repositories/spac import type { SpaceAgentInboxRepository } from '../../../storage/repositories/space-agent-inbox-repository'; import { NodeExecutionRepository } from '../../../storage/repositories/node-execution-repository'; import { GateDataRepository } from '../../../storage/repositories/gate-data-repository'; -import { WorkflowHookStateRepository } from '../../../storage/repositories/workflow-hook-state-repository'; +import type { WorkflowArtifactProfile } from './artifact-profile'; import type { ChannelCycleRepository } from '../../../storage/repositories/channel-cycle-repository'; import type { WorkflowRunArtifactRepository } from '../../../storage/repositories/workflow-run-artifact-repository'; import type { PendingAgentMessageRepository } from '../../../storage/repositories/pending-agent-message-repository'; @@ -154,6 +154,13 @@ export interface SpaceRuntimeServiceConfig { * can resolve artifact data for script env injection. */ artifactRepo?: WorkflowRunArtifactRepository; + /** + * Domain artifact profile. Passed through to SpaceRuntime and used by this + * service's temporary ChannelRouters to resolve the run's primary link URL + * for feature scripts. Owns coding-specific semantics so neither this service + * nor SpaceRuntime names domain kinds. + */ + artifactProfile?: WorkflowArtifactProfile; /** * Optional LLM-backed workflow selector override. Passed through to * SpaceRuntime verbatim. Defaults to `selectWorkflowWithLlmDefault` which @@ -2758,72 +2765,8 @@ export class SpaceRuntimeService { * can inject PR_URL into feature scripts. */ private resolvePrUrlForRun(runId: string): string { - // Only an explicit legacy PR field (pr_url/prUrl) qualifies as a PR URL — - // never a generic data.url (which could be an issue or preview link). The - // sole exception is a `link` artifact tagged kind:'pr' (handled below). - const legacyPrUrl = (data: Record | undefined): string => - (typeof data?.prUrl === 'string' && data.prUrl) || - (typeof data?.pr_url === 'string' && data.pr_url) || - ''; - - try { - const gateDataRepo = this.config.gateDataRepo ?? new GateDataRepository(this.config.db); - const gateRecords = gateDataRepo.listByRun(runId).sort((a, b) => b.updatedAt - a.updatedAt); - for (const record of gateRecords) { - const candidate = legacyPrUrl(record.data); - if (candidate) return candidate; - } - } catch (err) { - log.warn( - `SpaceRuntimeService.resolvePrUrlForRun: failed to read gate data for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - // Scan workflow hook state next. `pr_ready` hooks persist `pr_url` in - // localState after a successful send_message handoff (see - // workflow-hook-engine.ts); without this scan the resolver cannot find - // PR URLs for review-approval-gate when that gate's schema does not - // declare `pr_url` (the typical Review→QA handoff case). - try { - const hookStateRepo = new WorkflowHookStateRepository(this.config.db); - const hookStates = hookStateRepo - .listByRun(runId) - .sort((a, b) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)); - for (const snapshot of hookStates) { - const candidate = legacyPrUrl(snapshot.localState); - if (candidate) return candidate; - } - } catch (err) { - log.warn( - `SpaceRuntimeService.resolvePrUrlForRun: failed to read hook state for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - if (this.config.artifactRepo) { - try { - // Return the most recently updated eligible PR candidate — a link - // kind:'pr' (data.url) or a legacy pr_url/prUrl row. - const artifacts = this.config.artifactRepo.listByRun(runId); - let best: { url: string; updatedAt: number } | null = null; - for (const a of artifacts) { - const url = - a.artifactType === 'link' && a.data.kind === 'pr' - ? typeof a.data.url === 'string' - ? a.data.url - : '' - : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; - } - if (best) return best.url; - } catch (err) { - log.warn( - `SpaceRuntimeService.resolvePrUrlForRun: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - } - - return ''; + // Delegated to the domain artifact profile (coding: resolves the PR URL). + return this.config.artifactProfile?.resolvePrimaryLinkUrl(runId) ?? ''; } /** diff --git a/packages/daemon/src/lib/space/runtime/space-runtime.ts b/packages/daemon/src/lib/space/runtime/space-runtime.ts index 89020e9d52..c954b923d6 100644 --- a/packages/daemon/src/lib/space/runtime/space-runtime.ts +++ b/packages/daemon/src/lib/space/runtime/space-runtime.ts @@ -63,7 +63,7 @@ import { import { ChannelCycleRepository } from '../../../storage/repositories/channel-cycle-repository'; import { normalizeMeaningfulTaskResult } from '../task-result-utils'; import { GateDataRepository } from '../../../storage/repositories/gate-data-repository'; -import { WorkflowHookStateRepository } from '../../../storage/repositories/workflow-hook-state-repository'; +import type { WorkflowArtifactProfile } from './artifact-profile'; import type { NodeExecutionRepository } from '../../../storage/repositories/node-execution-repository'; import type { PendingAgentMessageRepository } from '../../../storage/repositories/pending-agent-message-repository'; import { SDKMessageRepository } from '../../../storage/repositories/sdk-message-repository'; @@ -226,6 +226,13 @@ export interface SpaceRuntimeConfig { * interpolation context for post-approval sessions. */ artifactRepo?: WorkflowRunArtifactRepository; + /** + * Domain artifact profile. Owns coding-specific semantics (which `link` is + * the run's PR, which `decision` is the terminal outcome) so this class never + * names domain kinds. When omitted, primary-link resolution returns '' and + * outcome summaries return undefined. + */ + artifactProfile?: WorkflowArtifactProfile; /** * Optional SDK message repository used to emit synthetic SDK messages into * a task's agent session. Defaults to a repo constructed from `db` if not @@ -4197,48 +4204,22 @@ export class SpaceRuntime { // 2. Resolve the post-approval route context (PR URL + template tokens). // - // `{{pr_url}}` in the merge template is sourced from the most recent - // `workflow_run_artifacts` row whose `data` carries `prUrl` / `pr_url`. - // The end-node reviewer persists the URL via - // `save_artifact({ type: 'result', data: { prUrl } })` immediately - // before calling `approve_task()`, so by the time we reach this branch - // the artifact row exists. We deliberately do NOT read from - // `SpaceTask`: migration 84 dropped `pr_url`/`pr_number` columns from - // `space_tasks` and moved PR metadata to the artifact store. + // `{{pr_url}}` in the merge template is sourced from the run's primary link + // URL — resolved by the domain artifact profile (coding: the PR URL across + // gate data, hook state, and artifacts). The end-node reviewer persists the + // URL (a `link kind:'pr'`) immediately before calling `approve_task()`, so + // by the time we reach this branch the artifact row exists. We deliberately + // do NOT read from `SpaceTask`: migration 84 dropped `pr_url`/`pr_number` + // columns from `space_tasks` and moved PR metadata to the artifact store. // // Callers may still override by passing `pr_url` in `contextExtras` // (RPC paths forward operator-supplied values) — their value wins // because the spread order below places `contextExtras` after the // artifact-resolved default. let resolvedPrUrl: string | undefined; - if (this.config.artifactRepo && approvedTask.workflowRunId) { - try { - const artifacts = this.config.artifactRepo.listByRun(approvedTask.workflowRunId); - // Return the most recently updated eligible PR candidate — a link - // kind:'pr' (data.url) or a legacy pr_url/prUrl row — so a newer legacy - // PR is never shadowed by an older shape link. A generic data.url never - // qualifies (it could be an issue or preview link). - const legacyPrUrl = (data: Record | undefined): string => - (typeof data?.prUrl === 'string' && data.prUrl) || - (typeof data?.pr_url === 'string' && data.pr_url) || - ''; - let best: { url: string; updatedAt: number } | null = null; - for (const a of artifacts) { - const url = - a.artifactType === 'link' && a.data.kind === 'pr' - ? typeof a.data.url === 'string' - ? a.data.url - : '' - : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; - } - if (best) resolvedPrUrl = best.url; - } catch (err) { - log.warn( - `dispatchPostApproval: artifact lookup failed for run ${approvedTask.workflowRunId}: ${err instanceof Error ? err.message : String(err)}` - ); - } + if (approvedTask.workflowRunId) { + resolvedPrUrl = + this.config.artifactProfile?.resolvePrimaryLinkUrl(approvedTask.workflowRunId) || undefined; } // The template interpolator (see `post-approval-template.ts`) resolves // tokens by raw identifier match — `{{autonomy_level}}` looks up the @@ -9376,71 +9357,10 @@ export class SpaceRuntime { */ private resolvePrUrlForRun(runId: string): string { - // Only an explicit legacy PR field (pr_url/prUrl) qualifies as a PR URL — - // never a generic data.url (which could be an issue or preview link). The - // sole exception is a `link` artifact tagged kind:'pr' (handled below). - const legacyPrUrl = (data: Record | undefined): string => - (typeof data?.prUrl === 'string' && data.prUrl) || - (typeof data?.pr_url === 'string' && data.pr_url) || - ''; - - try { - const gateDataRepo = this.config.gateDataRepo ?? new GateDataRepository(this.config.db); - const gateRecords = gateDataRepo.listByRun(runId).sort((a, b) => b.updatedAt - a.updatedAt); - for (const record of gateRecords) { - const candidate = legacyPrUrl(record.data); - if (candidate) return candidate; - } - } catch (err) { - log.warn( - `SpaceRuntime.resolvePrUrlForRun: failed to read gate data for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - // Scan workflow hook state next so `pr_ready` hook state (which persists - // `pr_url` after a successful send_message) is picked up even when the - // gate schema does not declare `pr_url` (e.g. Review→QA approval gate). - try { - const hookStateRepo = new WorkflowHookStateRepository(this.config.db); - const hookStates = hookStateRepo - .listByRun(runId) - .sort((a, b) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)); - for (const snapshot of hookStates) { - const candidate = legacyPrUrl(snapshot.localState); - if (candidate) return candidate; - } - } catch (err) { - log.warn( - `SpaceRuntime.resolvePrUrlForRun: failed to read hook state for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - if (this.config.artifactRepo) { - try { - // Return the most recently updated eligible PR candidate — a link - // kind:'pr' (data.url) or a legacy pr_url/prUrl row — so a newer legacy - // PR is never shadowed by an older shape link. - const artifacts = this.config.artifactRepo.listByRun(runId); - let best: { url: string; updatedAt: number } | null = null; - for (const a of artifacts) { - const url = - a.artifactType === 'link' && a.data.kind === 'pr' - ? typeof a.data.url === 'string' - ? a.data.url - : '' - : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; - } - if (best) return best.url; - } catch (err) { - log.warn( - `SpaceRuntime.resolvePrUrlForRun: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - } - - return ''; + // Delegated to the domain artifact profile (coding: resolves the PR URL). + // Generic infra does not know which `link` is the PR, so without a profile + // there is no primary link to resolve. + return this.config.artifactProfile?.resolvePrimaryLinkUrl(runId) ?? ''; } /** @@ -9556,37 +9476,9 @@ export class SpaceRuntime { } private resolvePrimaryResultArtifactSummary(runId: string): string | undefined { - if (!this.config.artifactRepo) return undefined; - - try { - // The terminal "result" is a kind-less `decision` (the bare terminal - // form — legacy `result`→decision carries no kind; review rounds and gate - // approvals carry a kind and are not terminal). Rolling-status `note`s - // are excluded too. - const decisions = this.config.artifactRepo.listByRun(runId, { artifactType: 'decision' }); - const summaryOf = (item: { data: Record }): string => { - const s = item.data.summary; - return typeof s === 'string' ? s : ''; - }; - const isTerminal = (item: { data: Record }): boolean => - !item.data.kind && summaryOf(item).trim().length > 0; - const artifact = decisions - .map((item, index) => ({ item, index })) - .filter(({ item }) => isTerminal(item)) - .toSorted( - (a, b) => - b.item.updatedAt - a.item.updatedAt || - b.item.createdAt - a.item.createdAt || - b.index - a.index - )[0]?.item; - const summary = artifact ? summaryOf(artifact) : ''; - return summary.length > 0 ? summary : undefined; - } catch (err) { - log.warn( - `SpaceRuntime.resolvePrimaryResultArtifactSummary: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - return undefined; - } + // Delegated to the domain artifact profile (coding: the kindless terminal + // `decision` summary). Returns undefined when no profile is wired. + return this.config.artifactProfile?.summarizeRunOutcome(runId) ?? undefined; } private buildTaskOutcomeUpdates( diff --git a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts index 7b15c15310..aafaf362b6 100644 --- a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts +++ b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts @@ -103,6 +103,7 @@ import { ChannelResolver } from './channel-resolver'; import { ChannelRouter } from './channel-router'; import { AgentMessageRouter } from './agent-message-router'; import type { ReplyRoutingRegistry } from './reply-routing-registry'; +import type { WorkflowArtifactProfile } from './artifact-profile'; import type { AgentMemoryRepository } from '../../../storage/repositories/agent-memory-repository'; import type { EvolutionScopeService } from '../evolution-scope-service'; import { createAgentMemoryMcpServer } from '../tools/agent-memory-tools'; @@ -292,6 +293,13 @@ export interface TaskAgentManagerConfig { dbPath?: string; /** Workflow run artifact repository — for write_artifact / list_artifacts node agent tools */ artifactRepo?: WorkflowRunArtifactRepository; + /** + * Domain artifact profile. Owns coding-specific semantics (primary-link + * resolution, terminal outcome summary, gate-keyed side-artifact history) so + * this manager and the node-agent tools it spawns never name domain kinds. + * Threaded through to the node-agent tool handlers. + */ + artifactProfile?: WorkflowArtifactProfile; /** * Persistent queue of Task Agent → peer agent messages waiting for the target * session to activate. When provided, `createSubSession` flushes all pending @@ -4518,28 +4526,10 @@ export class TaskAgentManager { internalEventBus: this.config.internalEventBus, goalService: this.config.goalService, resolveResultArtifactSummary: (task) => { - if (!task.workflowRunId || !this.config.artifactRepo) return null; - // The terminal "result" is a kind-less `decision` (legacy result→ - // decision carries no kind; review/gate decisions carry a kind and are - // not terminal). (Rolling-status `note`s are excluded too.) - const decisions = this.config.artifactRepo.listByRun(task.workflowRunId, { - artifactType: 'decision', - }); - const summaryOf = (item: { data: Record }): string => { - const s = item.data.summary; - return typeof s === 'string' ? s : ''; - }; - const artifact = decisions - .map((item, index) => ({ item, index })) - .filter(({ item }) => !item.data.kind && summaryOf(item).trim().length > 0) - .toSorted( - (a, b) => - b.item.updatedAt - a.item.updatedAt || - b.item.createdAt - a.item.createdAt || - b.index - a.index - )[0]?.item; - const summary = artifact ? summaryOf(artifact) : ''; - return summary.length > 0 ? summary : null; + // Delegated to the domain artifact profile (coding: the kindless + // terminal `decision` summary). + if (!task.workflowRunId) return null; + return this.config.artifactProfile?.summarizeRunOutcome(task.workflowRunId) ?? null; }, }); @@ -4886,6 +4876,7 @@ export class TaskAgentManager { onSubscribeExternalEvent, onUnsubscribeExternalEvent, artifactRepo: this.config.artifactRepo, + artifactProfile: this.config.artifactProfile, taskRepo: this.config.taskRepo, auditLogRepo: this.auditLogRepo, externalEventStore: this.config.externalEventStore, @@ -5129,70 +5120,8 @@ export class TaskAgentManager { } private resolvePrUrlForRun(runId: string): string { - // Only an explicit legacy PR field (pr_url/prUrl) qualifies as a PR URL — - // never a generic data.url, which could be an issue or preview link. The - // sole exception is a `link` artifact tagged kind:'pr' (handled below). - const legacyPrUrl = (data: Record | undefined): string => - (typeof data?.prUrl === 'string' && data.prUrl) || - (typeof data?.pr_url === 'string' && data.pr_url) || - ''; - - try { - const hookStateRepo = new WorkflowHookStateRepository(this.config.db.getDatabase()); - const run = this.config.workflowRunRepo.getRun(runId); - const workflow = run ? this.config.spaceWorkflowManager.getWorkflow(run.workflowId) : null; - for (const hook of workflow?.hooks ?? []) { - if (hook.validator.kind !== 'built_in' || hook.validator.id !== 'pr_ready') continue; - const candidate = legacyPrUrl(hookStateRepo.get(runId, hook.id)?.localState); - if (candidate) return candidate; - } - } catch (err) { - log.warn( - `TaskAgentManager.resolvePrUrlForRun: failed to read hook state for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - try { - const records = this.config.gateDataRepo?.listByRun(runId); - if (records) { - const sorted = records.sort((a, b) => b.updatedAt - a.updatedAt); - for (const record of sorted) { - const candidate = legacyPrUrl(record.data); - if (candidate) return candidate; - } - } - } catch (err) { - log.warn( - `TaskAgentManager.resolvePrUrlForRun: failed to read gate data for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - - if (this.config.artifactRepo) { - try { - // Gather every eligible PR-URL candidate — a `link` kind:'pr' (data.url) - // or a legacy row carrying pr_url/prUrl — and return the most recently - // updated, so a newer legacy PR row is never shadowed by an older shape - // link. A generic data.url on a non-pr artifact never qualifies. - const all = this.config.artifactRepo.listByRun(runId); - let best: { url: string; updatedAt: number } | null = null; - for (const a of all) { - const url = - a.artifactType === 'link' && a.data.kind === 'pr' - ? typeof a.data.url === 'string' - ? a.data.url - : '' - : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; - } - if (best) return best.url; - } catch (err) { - log.warn( - `TaskAgentManager.resolvePrUrlForRun: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` - ); - } - } - - return ''; + // Delegated to the domain artifact profile (coding: resolves the PR URL + // across gate data, hook state, and artifacts by recency). + return this.config.artifactProfile?.resolvePrimaryLinkUrl(runId) ?? ''; } } diff --git a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts index c9e0d04360..65c8cbcb43 100644 --- a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts +++ b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts @@ -169,82 +169,57 @@ export type GetExternalEventInput = z.infer; * Review verdict: save_artifact({ shape: 'decision', kind:'review', data: { recommendation: 'approve', summary } }) * Multi-round history: save_artifact({ shape: 'decision', kind:'review', key: 'round-0', data: {...} }) * Rolling status: save_artifact({ shape: 'note', data: { text: 'writing tests' } }) - * - * Legacy `type` is accepted as a deprecated alias and mapped to a shape - * (progress→note, result→decision, review→decision, pr→link) so in-flight - * agents keep working; unknown legacy types are rejected. */ -export const SaveArtifactSchema = z - .object({ - /** - * STRUCTURE — closed vocabulary. One of the values in ARTIFACT_SHAPES. - * Validated against the set; unknown values are rejected. Either `shape` - * or the legacy `type` alias must be provided. - */ - shape: z - .enum(ARTIFACT_SHAPES) - .describe( - "Structured shape from the closed set: 'link' | 'commit_set' | 'check' | 'metric' | 'decision' | 'note'. Either `shape` or legacy `type` is required." - ) - .optional(), - /** - * SEMANTIC hint (freeform, domain-extensible). Supplies the icon/label in - * the UI and folds into the identity key for `link`/`decision` so one kind - * never overwrites another. Examples: 'pr', 'issue', 'preview', 'ci', 'review'. - */ - kind: z - .string() - .min(1) - .describe( - "Semantic hint (freeform): 'pr', 'issue', 'preview', 'ci', 'review', etc. Used for the UI label/icon and folds into the identity key." - ) - .optional(), - /** - * Identity override. Defaults are derived from the shape (note→'current', - * link→kind, check/metric→name, decision→key|kind|'current'). Pass an - * explicit key only for multi-round history (e.g. decision 'round-0'). - */ - key: z - .string() - .describe( - "Identity key override. Derived from the shape by default. Pass an explicit value only for multi-round history (e.g. decision key: 'round-0')." - ) - .optional(), - /** ≤1 sentence human note. Stored under data.summary (note/decision). */ - summary: z - .string() - .describe('Short human note (≤1 sentence). Stored as data.summary for note/decision shapes.') - .optional(), - /** - * Shape-specific structured payload. Required fields depend on the shape - * (e.g. link needs `url`; check needs `name`+`status`; decision needs - * `recommendation`). Validated by save_artifact. - */ - data: z - .record(z.string(), z.unknown()) - .describe( - 'Shape-specific structured payload. Required fields vary by shape (link.url, check.name+status, decision.recommendation, etc.).' - ) - .optional(), - // ── Legacy aliases (deprecated; removed in a follow-up) ────────────────── - /** DEPRECATED — use `shape`. Legacy freeform type, mapped to a shape. */ - type: z - .string() - .describe( - 'DEPRECATED — use `shape`. Legacy freeform type; mapped to a shape (progress→note, result/review→decision, pr→link). Unknown values rejected.' - ) - .optional(), - /** DEPRECATED — use shape identity. Legacy append-only flag. */ - append: z - .boolean() - .describe( - 'DEPRECATED — use shape identity. Legacy append-only flag (inserts a new row). Ignored on the shape path.' - ) - .optional(), - }) - .refine((v) => v.shape !== undefined || v.type !== undefined, { - message: 'Either `shape` or legacy `type` is required.', - }); +export const SaveArtifactSchema = z.object({ + /** + * STRUCTURE — closed vocabulary. One of the values in ARTIFACT_SHAPES. + * Validated against the set; unknown values are rejected. Required. + */ + shape: z + .enum(ARTIFACT_SHAPES) + .describe( + "Structured shape from the closed set: 'link' | 'commit_set' | 'check' | 'metric' | 'decision' | 'note'. Required." + ), + /** + * SEMANTIC hint (freeform, domain-extensible). Supplies the icon/label in + * the UI and folds into the identity key for `link`/`decision` so one kind + * never overwrites another. Examples: 'pr', 'issue', 'preview', 'ci', 'review'. + */ + kind: z + .string() + .min(1) + .describe( + "Semantic hint (freeform): 'pr', 'issue', 'preview', 'ci', 'review', etc. Used for the UI label/icon and folds into the identity key." + ) + .optional(), + /** + * Identity override. Defaults are derived from the shape (note→'current', + * link→kind, check/metric→name, decision→key|kind|'current'). Pass an + * explicit key only for multi-round history (e.g. decision 'round-0'). + */ + key: z + .string() + .describe( + "Identity key override. Derived from the shape by default. Pass an explicit value only for multi-round history (e.g. decision key: 'round-0')." + ) + .optional(), + /** ≤1 sentence human note. Stored under data.summary (note/decision). */ + summary: z + .string() + .describe('Short human note (≤1 sentence). Stored as data.summary for note/decision shapes.') + .optional(), + /** + * Shape-specific structured payload. Required fields depend on the shape + * (e.g. link needs `url`; check needs `name`+`status`; decision needs + * `recommendation`). Validated by save_artifact. + */ + data: z + .record(z.string(), z.unknown()) + .describe( + 'Shape-specific structured payload. Required fields vary by shape (link.url, check.name+status, decision.recommendation, etc.).' + ) + .optional(), +}); export type SaveArtifactInput = z.infer; diff --git a/packages/daemon/src/lib/space/tools/node-agent-tools.ts b/packages/daemon/src/lib/space/tools/node-agent-tools.ts index df16615749..0b068f61d8 100644 --- a/packages/daemon/src/lib/space/tools/node-agent-tools.ts +++ b/packages/daemon/src/lib/space/tools/node-agent-tools.ts @@ -60,12 +60,9 @@ import { ARTIFACT_SHAPES, computeGateDefaults, deriveArtifactKey, - isArtifactShape, normalizeLinkData, - resolveLegacyShape, resolveNodeAgents, validateArtifactShape, - type ArtifactShape, } from '@hyperneo/shared'; import { jsonResult } from './tool-result'; import type { ToolResult } from './tool-result'; @@ -121,70 +118,10 @@ import { parseAddress } from '../../../../../messaging/src/address'; import { translateLegacyNodeTargets } from '../messaging-adapter'; import { getEffectiveGate, hasInjectedGateFeature } from '../runtime/gate-features'; import { buildPrEventTopicPattern, parsePrUrl } from '../runtime/parse-pr-url'; +import type { WorkflowArtifactProfile } from '../runtime/artifact-profile'; import type { WorkflowHookEngine } from '../runtime/workflow-hook-engine'; import { wrapHandlerWithHooks } from '../runtime/workflow-hook-engine'; -/** - * Resolves the most recent PR URL for a workflow run by scanning gate - * data records and artifacts, sorted by recency. A PR is a `link` shape tagged - * kind:'pr' whose `data.url` carries the URL; legacy rows / gate data may still - * carry the snake/camel `pr_url`/`prUrl` fields. The generic fallback NEVER - * accepts an arbitrary `data.url` (which could be an issue or preview link) — - * only link kind:'pr' or an explicit legacy PR field qualifies. - */ -function resolvePrUrlForRun( - gateDataRepo: GateDataRepository, - artifactRepo: WorkflowRunArtifactRepository | undefined, - runId: string -): string { - const legacyPrUrl = (data: Record | undefined): string => - (typeof data?.prUrl === 'string' && data.prUrl) || - (typeof data?.pr_url === 'string' && data.pr_url) || - ''; - - try { - const records = gateDataRepo.listByRun(runId); - if (records) { - const sorted = records.sort((a, b) => b.updatedAt - a.updatedAt); - for (const record of sorted) { - const candidate = legacyPrUrl(record.data); - if (candidate) return candidate; - } - } - } catch { - // ignore - } - - if (artifactRepo) { - try { - const artifacts = artifactRepo.listByRun(runId); - if (artifacts) { - // Gather every eligible PR-URL candidate — a `link` kind:'pr' (read via - // data.url) or a legacy row carrying pr_url/prUrl — and return the most - // recently updated, so a newer legacy PR row is never shadowed by an - // older shape link (and vice versa). A generic data.url on a non-pr - // artifact never qualifies. - let best: { url: string; updatedAt: number } | null = null; - for (const a of artifacts) { - const url = - a.artifactType === 'link' && a.data.kind === 'pr' - ? typeof a.data.url === 'string' - ? a.data.url - : '' - : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; - } - if (best) return best.url; - } - } catch { - // ignore - } - } - - return ''; -} - /** * Decode the JSON payload from a ToolResult created by jsonResult(). * Returns the parsed object or null if parsing fails. @@ -217,13 +154,14 @@ export async function evaluateTerminalGateFeatures( scriptExecutor?: GateScriptExecutorFn, scriptContext?: GateScriptExecutorContext, currentNodeId?: string, - artifactRepo?: WorkflowRunArtifactRepository + artifactProfile?: WorkflowArtifactProfile ): Promise { if (!workflow || !scriptExecutor || !scriptContext) return null; - // Resolve freshest PR URL for this run so terminal checks evaluate against - // the correct PR even when it was written after the MCP server was created. - const freshPrUrl = resolvePrUrlForRun(gateDataRepo, artifactRepo, workflowRunId); + // Resolve freshest primary link URL for this run so terminal checks evaluate + // against the correct PR even when it was written after the MCP server was + // created. Delegated to the domain profile (coding: the PR URL). + const freshPrUrl = artifactProfile?.resolvePrimaryLinkUrl(workflowRunId) ?? ''; // Scope terminal checks to gates on channels connected to the current node. // Outgoing channels (from current node) are always included. Incoming channels @@ -477,6 +415,12 @@ export interface NodeAgentToolsConfig { * Optional — when absent, artifact tools are not registered. */ artifactRepo?: WorkflowRunArtifactRepository; + /** + * Domain artifact profile. Owns coding-specific semantics (primary-link + * resolution, terminal outcome summary, gate-keyed side-artifact history) so + * these handlers never name domain kinds. Threaded from TaskAgentManager. + */ + artifactProfile?: WorkflowArtifactProfile; /** * Task repository for list_tasks and get_task tools. * Optional — when absent, task read tools are not registered. @@ -700,8 +644,8 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { * Returns completionState per peer: execution status, latest progress summary, and completedAt. * Returns nodeCompletionState: all executions on this workflow node with their completion state. * - * Progress summary is sourced from the latest 'progress' type artifact for the node - * (written via save_artifact({ type: 'progress', ... })). Falls back to ne.result for + * Progress summary is sourced from the latest `note` artifact for the node + * (written via save_artifact({ shape: 'note', ... })). Falls back to ne.result for * historical rows that predate the artifact migration. */ async list_peers(_args: ListPeersInput): Promise { @@ -1080,11 +1024,8 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { ) : gateDataRepo.merge(workflowRunId, gateId, partialToMerge); const updatedRecord = gateDataRepo.get(workflowRunId, gateId); - const freshPrUrl = resolvePrUrlForRun( - gateDataRepo, - config.artifactRepo, - workflowRunId - ); + const freshPrUrl = + config.artifactProfile?.resolvePrimaryLinkUrl(workflowRunId) ?? ''; const evalResult = await evaluateGate( getEffectiveGate(gateDef, workflow, gatedChannel.from), updated.data, @@ -1103,62 +1044,24 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { ); gateWriteResult = { gateId, gateOpen: evalResult.open }; - // Multi-round review history: every time the reviewer writes a - // `review_url` to this gate, persist one `decision` (kind:review) - // artifact per cycle (round-0, round-1 …) keyed so each round is a - // distinct upsert. Persist this before any rate-limited early return - // so the review record is not lost when the gate script is blocked. - if ( - config.artifactRepo && - gateId === 'review-posted-gate' && - typeof authorizedData.review_url === 'string' && - authorizedData.review_url.length > 0 - ) { + // Domain profile hook: let the coding layer persist any + // gate-keyed side-artifacts (e.g. a multi-round review decision + // when a review gate fires). Fired before any rate-limited early + // return so the record is not lost when the gate script is + // blocked. Infra knows neither the gate id nor the kind — only + // the profile does. + if (config.artifactProfile?.onGateDataCommitted) { try { - const decisions = config.artifactRepo.listByRun(workflowRunId, { - artifactType: 'decision', - }); - // Next round = one past the highest existing review-round - // number, derived from the trailing digits of each review - // decision's key (handles sparse keys and both legacy - // 'cycle-N' and namespaced 'review:round-N' forms). This - // never overwrites an existing review round. - let maxCycle = -1; - for (const a of decisions) { - if (a.data.kind !== 'review') continue; - const m = /(\d+)$/.exec(a.artifactKey); - if (m) maxCycle = Math.max(maxCycle, Number.parseInt(m[1], 10)); - } - const cycle = maxCycle + 1; - const artifactData: Record = { - recommendation: 'reviewed', - kind: 'review', - review_url: authorizedData.review_url, - cycle, - submittedAt: new Date().toISOString(), - }; - const rawCommentUrls = (data as Record).comment_urls; - if ( - Array.isArray(rawCommentUrls) && - rawCommentUrls.every((u) => typeof u === 'string') - ) { - artifactData.comment_urls = rawCommentUrls; - } - config.artifactRepo.upsert({ - id: crypto.randomUUID(), + await config.artifactProfile.onGateDataCommitted({ runId: workflowRunId, nodeId: workflowNodeId, - artifactType: 'decision', - artifactKey: deriveArtifactKey( - 'decision', - { kind: 'review' }, - `round-${cycle}` - ), - data: artifactData, + gateId, + gateData: updated.data, + messageData: data, }); } catch (err) { log.warn( - `Failed to append review artifact for run "${workflowRunId}":`, + `onGateDataCommitted failed for gate "${gateId}" in run "${workflowRunId}":`, err instanceof Error ? err.message : String(err) ); } @@ -1508,7 +1411,7 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { // Evaluate current gate status. Uses scriptExecutor when available for // async script-based gates; otherwise falls back to field-only evaluation. - const freshPrUrl = resolvePrUrlForRun(gateDataRepo, config.artifactRepo, workflowRunId); + const freshPrUrl = config.artifactProfile?.resolvePrimaryLinkUrl(workflowRunId) ?? ''; const sourceName = resolveCurrentGateSource(gateId); const evalResult = await evaluateGate( getEffectiveGate(gateDef, workflow ?? undefined, sourceName), @@ -1550,10 +1453,6 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { * per kind, check/metric→name, decision→key|kind|'current'), so repeated * status updates overwrite in place instead of accumulating per round. * - * The legacy `type` param is accepted as a deprecated alias and mapped to a - * shape (progress→note, result/review→decision, pr→link) so in-flight agents - * keep working; unknown legacy types are rejected. - * * Requires `artifactRepo` to be provided in the config. */ async save_artifact(args: SaveArtifactInput): Promise { @@ -1562,60 +1461,8 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { return jsonResult({ success: false, error: 'Artifact repository not available.' }); } - const { shape: shapeArg, type, kind, key: keyArg, append, summary, data } = args; - - // Resolve the shape. The new `shape` param wins. The legacy `type` alias is - // then mapped (data-aware, since `result` was overloaded): - // - a shape NAME passed as `type` → treated as a new shape call (validated); - // - a known legacy type (progress/result/review/pr) → mapped to a shape, - // bypassing strict validation since it predates the contracts; - // - any other freeform type (e.g. merge_conflict_loop, merge_blocked, - // cleanup_warning from active post-approval prompts) → accepted as a - // `note` tagged with the original type, so the write still records state. - let shape: ArtifactShape | undefined; - let legacyAppend = false; - let isLegacy = false; - let isUnknownLegacy = false; - // Legacy `pr`→link and `review`→decision carry an implicit kind so PR - // readers and round counters find them without an explicit kind arg. - let legacyKind: string | undefined; - if (shapeArg !== undefined) { - shape = shapeArg; - } else if (type !== undefined) { - legacyAppend = append === true; - if (isArtifactShape(type)) { - // A shape name passed via the legacy alias is NOT a legacy semantic - // type — validate it like a normal shape call (no bypass). - shape = type; - } else { - const provisional: Record = {}; - if (summary !== undefined) provisional.summary = summary; - if (data !== undefined) Object.assign(provisional, data); - const mapped = resolveLegacyShape(type, provisional); - if (!mapped) { - // Unknown freeform type: accept as a note so active prompts that - // predate the shape vocabulary (post-approval audit/blocker writes) - // keep recording state rather than erroring. - isLegacy = true; - isUnknownLegacy = true; - shape = 'note'; - } else { - isLegacy = true; - shape = mapped; - if (type === 'pr') { - legacyKind = 'pr'; - } else if (type === 'review') { - legacyKind = 'review'; - } else if (shape === 'link') { - // Legacy `result`-with-URL routed to a link: infer the kind from - // the URL field so PR readers and the link identity key pick it up. - const d = (data as Record | undefined) ?? {}; - if (typeof d.pr_url === 'string' || typeof d.prUrl === 'string') legacyKind = 'pr'; - else if (typeof d.review_url === 'string') legacyKind = 'review'; - } - } - } - } + const { shape, kind, key: keyArg, summary, data } = args; + if (!shape) { return jsonResult({ success: false, @@ -1623,17 +1470,13 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { }); } - // Merge summary + data into a single payload, then fold in the kind hint - // (explicit kind wins over the legacy implicit kind). Unknown legacy types - // keep their original type under _legacyType for traceability. + // Merge summary + data into a single payload, then fold in the kind hint. const artifactData: Record = {}; if (summary !== undefined) artifactData.summary = summary; if (data !== undefined) Object.assign(artifactData, data); - const effectiveKind = kind ?? legacyKind; - if (effectiveKind !== undefined) artifactData.kind = effectiveKind; - if (isUnknownLegacy && type !== undefined) artifactData._legacyType = type; - // Legacy link rows may carry pr_url/review_url instead of url — normalise - // so link readers (which key off data.url) find the URL. + if (kind !== undefined) artifactData.kind = kind; + // Link rows may carry a URL-bearing field under a domain key; normalise + // onto data.url so link readers (which key off data.url) find it. const normalized = shape === 'link' ? normalizeLinkData(artifactData) : artifactData; if (Object.keys(normalized).length === 0) { @@ -1643,25 +1486,14 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { }); } - // Validate the payload against the per-shape contract (new shape calls - // only — legacy callers bypass, since they predate the contracts). - if (!isLegacy) { - const validation = validateArtifactShape(shape, normalized); - if (!validation.ok) { - return jsonResult({ success: false, error: validation.error }); - } + // Validate the payload against the per-shape contract. + const validation = validateArtifactShape(shape, normalized); + if (!validation.ok) { + return jsonResult({ success: false, error: validation.error }); } try { - // Identity: legacy append mode forces a unique key (new row); an unknown - // legacy type uses the original type as its key so distinct unknown types - // don't collapse into the single rolling 'current' note; otherwise the - // key is derived from the shape so like shapes upsert in place. - const artifactKey = legacyAppend - ? `${Date.now()}-${Math.random().toString(36).slice(2)}` - : isUnknownLegacy - ? (type as string) - : deriveArtifactKey(shape, normalized, keyArg); + const artifactKey = deriveArtifactKey(shape, normalized, keyArg); const record = artifactRepo.upsert({ id: crypto.randomUUID(), @@ -1676,7 +1508,6 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { shape, kind: kind ?? undefined, key: artifactKey, - legacyType: type ?? undefined, summary: summary ?? undefined, dataKeys: data ? Object.keys(data) : undefined, }); @@ -1704,58 +1535,13 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { return jsonResult({ success: false, error: 'Artifact repository not available.' }); } try { - // Map a legacy freeform `type` filter to its shape set so in-flight - // agents that still ask for { type: 'result' } see their migrated rows - // (result→decision|link, progress→note, review→decision, pr→link). - const filterShapes = (t: string | undefined): string[] | undefined => { - if (!t) return undefined; - if (isArtifactShape(t)) return [t]; - switch (t) { - case 'result': - return ['decision', 'link']; - case 'progress': - return ['note']; - case 'review': - return ['decision']; - case 'pr': - return ['link']; - default: - return [t]; - } - }; - const shapes = filterShapes(args.type); - // Legacy `pr`→link and `review`→decision map to a shape that also holds - // unrelated kinds (issue/preview/doc links, gate decisions), so the shape - // filter alone over-returns. Post-filter on the legacy semantic kind; - // `result` (overloaded: decision|link) and `progress` stay unfiltered, as - // do direct shape-name queries (type:'link') and unknown types. - const kindFilter = args.type === 'pr' || args.type === 'review' ? args.type : undefined; - let artifacts: ReturnType; - if (!shapes || shapes.length <= 1) { - artifacts = artifactRepo.listByRun(workflowRunId, { - nodeId: args.nodeId, - artifactType: shapes?.[0], - }); - } else { - const seen = new Set(); - const merged: Array<(typeof artifacts)[number]> = []; - for (const s of shapes) { - for (const a of artifactRepo.listByRun(workflowRunId, { - nodeId: args.nodeId, - artifactType: s, - })) { - if (!seen.has(a.id)) { - seen.add(a.id); - merged.push(a); - } - } - } - // listByRun orders ASC by created_at; re-sort the merged set to match. - artifacts = merged.sort((a, b) => a.createdAt - b.createdAt); - } - if (kindFilter) { - artifacts = artifacts.filter((a) => a.data.kind === kindFilter); - } + // `type` filters by the canonical SHAPE vocabulary (link / commit_set / + // check / metric / decision / note); artifacts are always stored as a + // shape, so a single filter value is enough. + const artifacts = artifactRepo.listByRun(workflowRunId, { + nodeId: args.nodeId, + artifactType: args.type, + }); return jsonResult({ success: true, artifacts: artifacts.map((a) => ({ @@ -1878,7 +1664,7 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { }); } const prUrl = - args.prUrl || resolvePrUrlForRun(gateDataRepo, config.artifactRepo, workflowRunId); + args.prUrl || config.artifactProfile?.resolvePrimaryLinkUrl(workflowRunId) || ''; const parsed = prUrl ? parsePrUrl(prUrl) : null; if (!parsed) { return jsonResult({ @@ -1958,7 +1744,7 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { scriptExecutor, scriptContext, workflowNodeId, - config.artifactRepo + config.artifactProfile ); if (gateBlock) return gateBlock; @@ -2190,7 +1976,7 @@ export function createNodeAgentMcpServer(config: NodeAgentToolsConfig) { config.scriptExecutor, config.scriptContext, config.workflowNodeId, - config.artifactRepo + config.artifactProfile ); if (gateBlock) return gateBlock; return config.onSubmitForApproval!(args); @@ -2342,8 +2128,7 @@ export function createNodeAgentMcpServer(config: NodeAgentToolsConfig) { '`note` is a single rolling-status upsert; `link` is one per kind; `check`/`metric` keyed by name; ' + '`decision` is single-terminal or multi-round via `key`. Save structured facts (PR/preview/doc → ' + 'link, CI/tests → check, review verdict → decision, current status → note), NOT a re-narration of ' + - 'the thread. The legacy `type` param is accepted as a compatibility alias (progress→note, ' + - 'result→decision|link, review→decision, pr→link) but `shape` is preferred.', + 'the thread. Keep prose in chat; only structured facts belong here.', SaveArtifactSchema.shape, (args) => handlers.save_artifact(args) ), diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index 0d5cf1280f..1d4eb40e66 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -337,9 +337,9 @@ const PD_TASK_DISPATCHER_PROMPT = ' ```\n\n' + '4. Collect the returned task IDs. Build a stack map: ' + '{ prefix, items: [{ title, task_id, branch, base_branch, position }] }.\n' + - '5. Call `save_artifact({ type: "result", append: true, summary: "Created N tasks from plan: ", ' + - 'created_task_ids: [], stack_prefix: "", ' + - 'stack_branches: ["plan//", "plan//", ...] })` to record the dispatch audit entry.\n' + + '5. Call `save_artifact({ shape: "decision", summary: "Created N tasks from plan: ", ' + + 'data: { created_task_ids: [], stack_prefix: "", ' + + 'stack_branches: ["plan//", "plan//", ...] } })` to record the dispatch outcome.\n' + '6. Call `approve_task()` as your final action. If autonomy blocks self-close, call ' + '`submit_for_approval({ reason: "..." })` instead.\n\n' + 'CRITICAL: Do NOT create branches, make commits, push to git, or open PRs yourself — ' + @@ -603,12 +603,13 @@ export const CODING_WORKFLOW: SpaceWorkflow = { 'before releasing your message. If you skip `gh pr review`, the hook will block ' + 'and the coder will never hear from you.\n\n' + reviewerFeedbackProcedure('Coding') + - 'Use save_artifact every cycle. Nest pr_url inside artifact data for post-approval dispatch.\n\n' + + 'Use save_artifact every cycle to record the PR as a `link` so post-approval dispatch ' + + 'can resolve it.\n\n' + 'Review checklist: inspect PR diff and related worktree context, run tests if uncertain, ' + 'post visible GitHub review before sending feedback. If changes needed, include pr_url, ' + 'review_url, and comment_urls when messaging Coding. If approved, ' + REVIEW_THREAD_APPROVAL_CHECK_GUIDANCE + - ' Call save_artifact({ type: "result", data: { pr_url: "" } }) then approve_task() or submit_for_approval. ' + + ' Call save_artifact({ shape: "link", kind: "pr", data: { url: "" } }) then approve_task() or submit_for_approval. ' + 'Do NOT attempt to merge the PR yourself. Do not set auto-merge.' + REVIEWER_POST_APPROVAL_BLOCKER_PARAGRAPH, }, @@ -793,12 +794,13 @@ export const RESEARCH_WORKFLOW: SpaceWorkflow = { 'You are the Reviewer in a Research→Reviewer iterative workflow. You review the ' + 'research findings for completeness, accuracy, and quality.\n\n' + reviewerFeedbackProcedure('Research') + - 'Use save_artifact every cycle. Nest pr_url inside artifact data for post-approval dispatch.\n\n' + + 'Use save_artifact every cycle to record the PR as a `link` so post-approval dispatch ' + + 'can resolve it.\n\n' + 'Review checklist: read all research docs in the PR, verify completeness, evidence, ' + 'accuracy, and clarity. If more research is needed, message Research with specific ' + 'areas to investigate and stop. If satisfied, post approval review, ' + REVIEW_THREAD_APPROVAL_CHECK_GUIDANCE + - ' Call save_artifact({ type: "result", data: { pr_url: "" } }) then approve_task() or submit_for_approval. ' + + ' Call save_artifact({ shape: "link", kind: "pr", data: { url: "" } }) then approve_task() or submit_for_approval. ' + 'Do NOT attempt to merge the PR yourself. Do not set auto-merge.' + REVIEWER_POST_APPROVAL_BLOCKER_PARAGRAPH, }, @@ -919,7 +921,7 @@ export const REVIEW_ONLY_WORKFLOW: SpaceWorkflow = { 'You are the sole Reviewer in a single-node Review-Only workflow. Review an existing ' + 'PR or codebase directly. Follow the Reviewer System Contract and terminal-action tool ' + 'contract: post a visible GitHub review (`gh pr review`) before terminal actions; ' + - 'call save_artifact({ type: "result", data: { pr_url: "" } }) to save a result artifact, then approve_task() or submit_for_approval only on APPROVE, otherwise stop. ' + + 'call save_artifact({ shape: "link", kind: "pr", data: { url: "" } }) to record the PR, then approve_task() or submit_for_approval only on APPROVE, otherwise stop. ' + 'Do NOT attempt to merge the PR yourself. Never set a PR to auto-merge.', }, }, @@ -951,7 +953,7 @@ export const REVIEW_ONLY_WORKFLOW: SpaceWorkflow = { * Plan Review → Planning (revision requests, maxCycles: 5) * * Task Dispatcher (end node) creates follow-up tasks via `create_standalone_task` - * and calls `save_artifact({ type: 'result', append: true, created_task_ids })` + * and calls `save_artifact({ shape: 'decision', data: { created_task_ids } })` * before `approve_task()` closes the run. */ export const PLAN_AND_DECOMPOSE_WORKFLOW: SpaceWorkflow = { @@ -1083,7 +1085,7 @@ export const PLAN_AND_DECOMPOSE_WORKFLOW: SpaceWorkflow = { '\n\n' + 'Expected inputs: An approved plan PR (all 4 reviewers sent approved votes).\n' + 'Expected outputs: One standalone task per actionable work item in the plan, ' + - 'then save_artifact({ type: "result", append: true, created_task_ids: [...] }).\n\n' + + 'then save_artifact({ shape: "decision", data: { created_task_ids: [...] } }).\n\n' + 'Tool contract:\n' + "- `create_standalone_task` is available from the space's MCP server and " + 'creates a task owned by the same space as this workflow.', @@ -1254,13 +1256,13 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = { '5. If `ui_changed` is true, start HyperNeo with `make dev PORT= DB_PATH=/tmp/hyperneo-qa-.db` and exercise the changed flow in a browser (golden path, relevant edge cases, nearby regressions)\n' + '6. Validate CI and mergeability\n' + '7. If fail: send detailed failures and repro steps to Coding, then call ' + - '`save_artifact({ type: "result", append: true, summary: "QA failed: ..." })` to record the audit entry. Do ' + + '`save_artifact({ shape: "note", kind: "qa", summary: "QA failed: ..." })` to record the audit entry (a note, never a terminal decision). Do ' + 'NOT call `approve_task` or `submit_for_approval` — both are TERMINAL and ' + 'carry the same approval semantic. Leave the workflow open for the next ' + 'Coding cycle.\n' + '8. If all green:\n' + - ' a. Call `save_artifact({ type: "result", append: true, summary, data: { pr_url: "", test_output: "", ui_changed: , dev_server_started: , browser_validation: "" } })` ' + - 'to record the audit entry. The `pr_url` inside `data` is what ' + + ' a. Call `save_artifact({ shape: "decision", summary, data: { pr_url: "", test_output: "", ui_changed: , dev_server_started: , browser_validation: "" } })` ' + + 'to record the terminal outcome. The `pr_url` inside `data` is what ' + '`dispatchPostApproval` reads when interpolating `{{pr_url}}` into the ' + 'merge template — top-level keys outside `data` are silently stripped by ' + 'the tool schema, so nest it correctly.\n' + diff --git a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts new file mode 100644 index 0000000000..67e8bdbb7a --- /dev/null +++ b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts @@ -0,0 +1,205 @@ +/** + * Coding-Workflow Artifact Profile + * + * The domain implementation of {@link WorkflowArtifactProfile} for coding + * workflows. This is the ONLY place in the daemon that names coding-specific + * kinds (`pr`, `review`) and coding-specific identifiers (`review-posted-gate`, + * the `pr_url` / `prUrl` / `review_url` gate-data fields). Generic infra depends + * on the interface; it never imports this module. + * + * It consolidates the three behaviors that previously lived (duplicated and + * kind-hardcoded) inside daemon core: + * - `resolvePrimaryLinkUrl` — the PR URL (a `link kind:'pr'`, or a legacy + * `pr_url`/`prUrl` field), resolved across gate data, hook state, and + * artifacts by recency. + * - `summarizeRunOutcome` — the kindless terminal `decision` summary. + * - `onGateDataCommitted` — append one `decision kind:'review'` (round-N) + * each time the review-posted-gate receives a `review_url`. + */ + +import type { Database as BunDatabase } from 'bun:sqlite'; +import { deriveArtifactKey } from '@hyperneo/shared'; +import { Logger } from '../../logger'; +import type { GateDataCommittedEvent, WorkflowArtifactProfile } from '../runtime/artifact-profile'; +import { GateDataRepository } from '../../../storage/repositories/gate-data-repository'; +import { WorkflowHookStateRepository } from '../../../storage/repositories/workflow-hook-state-repository'; +import type { WorkflowRunArtifactRepository } from '../../../storage/repositories/workflow-run-artifact-repository'; + +const log = new Logger('coding-artifact-profile'); + +/** Gate id that records a multi-round review decision per cycle. */ +const REVIEW_POSTED_GATE = 'review-posted-gate'; + +export interface CodingArtifactProfileConfig { + db: BunDatabase; + artifactRepo?: WorkflowRunArtifactRepository; + /** Optional shared gate-data repo; created from `db` when omitted. */ + gateDataRepo?: GateDataRepository; +} + +/** + * Extract a legacy PR URL (`prUrl` / `pr_url`) from a data object. Returns '' + * when neither field holds a string. A generic `url` field never qualifies — + * it could be an issue or preview link. + */ +function legacyPrUrl(data: Record | undefined): string { + return ( + (typeof data?.prUrl === 'string' && data.prUrl) || + (typeof data?.pr_url === 'string' && data.pr_url) || + '' + ); +} + +export class CodingArtifactProfile implements WorkflowArtifactProfile { + private readonly db: BunDatabase; + private readonly artifactRepo?: WorkflowRunArtifactRepository; + private readonly sharedGateDataRepo?: GateDataRepository; + + constructor(config: CodingArtifactProfileConfig) { + this.db = config.db; + this.artifactRepo = config.artifactRepo; + this.sharedGateDataRepo = config.gateDataRepo; + } + + resolvePrimaryLinkUrl(runId: string): string { + // 1. Gate data — most recently updated record carrying a PR URL. + try { + const gateDataRepo = this.sharedGateDataRepo ?? new GateDataRepository(this.db); + const gateRecords = gateDataRepo.listByRun(runId).sort((a, b) => b.updatedAt - a.updatedAt); + for (const record of gateRecords) { + const candidate = legacyPrUrl(record.data); + if (candidate) return candidate; + } + } catch (err) { + log.warn( + `resolvePrimaryLinkUrl: failed to read gate data for run ${runId}: ${err instanceof Error ? err.message : String(err)}` + ); + } + + // 2. Workflow hook state — `pr_ready` hooks persist `pr_url` after a + // successful send_message even when the gate schema does not declare it. + try { + const hookStateRepo = new WorkflowHookStateRepository(this.db); + const hookStates = hookStateRepo + .listByRun(runId) + .sort((a, b) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)); + for (const snapshot of hookStates) { + const candidate = legacyPrUrl(snapshot.localState); + if (candidate) return candidate; + } + } catch (err) { + log.warn( + `resolvePrimaryLinkUrl: failed to read hook state for run ${runId}: ${err instanceof Error ? err.message : String(err)}` + ); + } + + // 3. Artifacts — the most recently updated eligible candidate. A `link` + // kind:'pr' (read via data.url) qualifies, as does a legacy row carrying + // pr_url/prUrl, so a newer legacy PR is never shadowed by an older shape + // link (or vice versa). A generic data.url on a non-pr artifact never + // qualifies. + if (this.artifactRepo) { + try { + const artifacts = this.artifactRepo.listByRun(runId); + let best: { url: string; updatedAt: number } | null = null; + for (const a of artifacts) { + const url = + a.artifactType === 'link' && a.data.kind === 'pr' + ? typeof a.data.url === 'string' + ? a.data.url + : '' + : legacyPrUrl(a.data); + if (!url) continue; + if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; + } + if (best) return best.url; + } catch (err) { + log.warn( + `resolvePrimaryLinkUrl: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` + ); + } + } + + return ''; + } + + summarizeRunOutcome(runId: string): string | null { + if (!this.artifactRepo) return null; + try { + // The terminal outcome is a kindless `decision` carrying a summary. + // Review/gate decisions carry a kind and are not terminal; rolling-status + // `note`s are excluded too. + const decisions = this.artifactRepo.listByRun(runId, { artifactType: 'decision' }); + const summaryOf = (item: { data: Record }): string => { + const s = item.data.summary; + return typeof s === 'string' ? s : ''; + }; + const isTerminal = (item: { data: Record }): boolean => + !item.data.kind && summaryOf(item).trim().length > 0; + const artifact = decisions + .map((item, index) => ({ item, index })) + .filter(({ item }) => isTerminal(item)) + .toSorted( + (a, b) => + b.item.updatedAt - a.item.updatedAt || + b.item.createdAt - a.item.createdAt || + b.index - a.index + )[0]?.item; + const summary = artifact ? summaryOf(artifact) : ''; + return summary.length > 0 ? summary : null; + } catch (err) { + log.warn( + `summarizeRunOutcome: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` + ); + return null; + } + } + + async onGateDataCommitted(event: GateDataCommittedEvent): Promise { + if (!this.artifactRepo) return; + const { runId, nodeId, gateId, gateData, messageData } = event; + // Multi-round review history: every time the reviewer writes a `review_url` + // to the review-posted-gate, persist one `decision kind:'review'` per cycle + // (round-0, round-1 …) keyed so each round is a distinct upsert. + if (gateId !== REVIEW_POSTED_GATE) return; + const reviewUrl = gateData.review_url; + if (typeof reviewUrl !== 'string' || reviewUrl.length === 0) return; + + try { + const decisions = this.artifactRepo.listByRun(runId, { artifactType: 'decision' }); + // Next round = one past the highest existing review-round number, derived + // from the trailing digits of each review decision's key (handles sparse + // keys and both legacy 'cycle-N' and namespaced 'review:round-N' forms). + let maxCycle = -1; + for (const a of decisions) { + if (a.data.kind !== 'review') continue; + const m = /(\d+)$/.exec(a.artifactKey); + if (m) maxCycle = Math.max(maxCycle, Number.parseInt(m[1], 10)); + } + const cycle = maxCycle + 1; + const artifactData: Record = { + recommendation: 'reviewed', + kind: 'review', + review_url: reviewUrl, + cycle, + submittedAt: new Date().toISOString(), + }; + const rawCommentUrls = messageData?.comment_urls; + if (Array.isArray(rawCommentUrls) && rawCommentUrls.every((u) => typeof u === 'string')) { + artifactData.comment_urls = rawCommentUrls; + } + this.artifactRepo.upsert({ + id: crypto.randomUUID(), + runId, + nodeId, + artifactType: 'decision', + artifactKey: deriveArtifactKey('decision', { kind: 'review' }, `round-${cycle}`), + data: artifactData, + }); + } catch (err) { + log.warn( + `onGateDataCommitted: failed to append review artifact for run ${runId}: ${err instanceof Error ? err.message : String(err)}` + ); + } + } +} diff --git a/packages/daemon/src/lib/space/workflows/post-approval-merge-template.ts b/packages/daemon/src/lib/space/workflows/post-approval-merge-template.ts index 6a28cd3ea2..d206f43302 100644 --- a/packages/daemon/src/lib/space/workflows/post-approval-merge-template.ts +++ b/packages/daemon/src/lib/space/workflows/post-approval-merge-template.ts @@ -153,7 +153,7 @@ export const PR_MERGE_POST_APPROVAL_INSTRUCTIONS: string = [ ' because the cap is reached, or the approval authority reports a blocker', ' neither of you can resolve, escalate to space-agent (NOT merely because', ' merges kept failing) — record a NON-result artifact and notify:', - ' save_artifact({ type: "merge_blocked", append: true,', + ' save_artifact({ shape: "note", kind: "merge_blocked",', ' summary: "Merge blocked on PR {{pr_url}} ( attempts, )",', ' data: { pr_url: "{{pr_url}}", blockers: ["..."], attempts: ,', ' exit_reason: "" } })', @@ -173,7 +173,7 @@ export const PR_MERGE_POST_APPROVAL_INSTRUCTIONS: string = [ ' git push origin --delete "$HEAD_REF"', ' Branch cleanup is BEST-EFFORT: if deletion fails for any reason (protected', ' branch, missing delete permission, already gone), record a NON-result warning', - ' artifact (e.g. type:"cleanup_warning") and continue — a "result" artifact would', + ' artifact (e.g. shape:"note", kind:"cleanup_warning", key:"branch-delete") and continue — a kindless `decision` would', ' be picked up as the task result on completion. The PR is already merged, so do', ' NOT let a cleanup failure block the completion step.', '4. Sync so both this isolated worktree AND the Space checkout track the freshly-merged', @@ -195,18 +195,18 @@ export const PR_MERGE_POST_APPROVAL_INSTRUCTIONS: string = [ ' continues):', ' if [ "$(git -C "$SPACE_WS" rev-parse --abbrev-ref HEAD)" != "$BASE" ]; then', ' # checkout on a different branch — do NOT move it; warn and skip.', - ' record a NON-result cleanup_warning artifact (space not on $BASE) and continue.', + ' record a NON-result `note` cleanup_warning artifact (key "space-checkout-base"; space not on $BASE) and continue.', ' fi', ' git -C "$SPACE_WS" fetch origin "$BASE"', ' git -C "$SPACE_WS" pull --ff-only origin "$BASE"', ' if [ "$(git -C "$SPACE_WS" rev-parse HEAD)" != "$(git -C "$SPACE_WS" rev-parse "origin/$BASE")" ]; then', ' # pull said "Already up to date" but local $BASE is AHEAD of origin/$BASE —', ' # stray commits remain at HEAD; do NOT claim the checkout is synchronized.', - ' record a NON-result cleanup_warning artifact (space $BASE ahead of origin/$BASE) and continue.', + ' record a NON-result `note` cleanup_warning artifact (key "space-checkout-ahead"; space $BASE ahead of origin/$BASE) and continue.', ' fi', ' If the pull itself fails (divergence, permissions), do NOT force it — record a', - ' NON-result cleanup_warning artifact and continue (the PR is already merged).', + ' NON-result `note` cleanup_warning artifact (key "space-checkout-pull") and continue (the PR is already merged).', '5. Save an audit artifact:', - ' save_artifact({ type: "result", append: true,', - ' data: { merged_pr_url, merged_at, approval_source: "{{approval_source}}" } })', + ' save_artifact({ shape: "link", kind: "merge",', + ' data: { url: , merged_at, approval_source: "{{approval_source}}" } })', ].join('\n'); From 492cac63be5688b734ba2211857bb5c0d75fee62 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Sun, 2 Aug 2026 23:22:37 -0400 Subject: [PATCH 02/16] test(space): wire coding artifact profile + migrate shape-API tests Update tests for the relocated artifact logic: - node-agent-tools: drop the legacy type-shim tests (shim removed); add a "shape required" guard test; wire CodingArtifactProfile into the test ctx so review-posted-gate history and PR-URL resolution assert against the profile. - evolution-episode-service: wire the profile so result-artifact gap detection reads outcomes through it. - space-runtime / -service / -rehydration / event-driven-gate-eval / external-event-delivery-e2e / post-approval-routing: inject the profile where the tests assert primary-link (PR URL) resolution or outcome summaries. - built-in-workflows + end-node-handoff: update prompt-content assertions for the migrated shape API (link kind:'pr', note kind:'merge_*'). Task #809 (#796 PR C). --- .../5-space/agent/node-agent-tools.test.ts | 110 ++++-------------- .../5-space/evolution-episode-service.test.ts | 38 ++++++ .../external-event-delivery-e2e.test.ts | 2 + .../post-approval-routing-integration.test.ts | 2 + .../runtime/space-runtime-completion.test.ts | 2 + ...ntime-event-driven-gate-evaluation.test.ts | 4 + .../space-runtime-external-events.test.ts | 11 ++ .../runtime/space-runtime-rehydration.test.ts | 4 + .../workflow/built-in-workflows.test.ts | 2 +- .../5-space/workflow/end-node-handoff.test.ts | 4 +- 10 files changed, 87 insertions(+), 92 deletions(-) diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index f50f063bb1..273aebcf2f 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -48,6 +48,8 @@ import type { DaemonInternalEventMap, InternalEventBus, } from '../../../../src/lib/internal-event-bus.ts'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile.ts'; +import type { WorkflowArtifactProfile } from '../../../../src/lib/space/runtime/artifact-profile.ts'; // --------------------------------------------------------------------------- // DB helpers @@ -187,6 +189,8 @@ interface TestCtx { spaceTaskRepo: SpaceTaskRepository; nodeExecutionRepo: NodeExecutionRepository; artifactRepo: WorkflowRunArtifactRepository; + /** Coding artifact profile (PR resolution + review history) wired into the handlers. */ + artifactProfile: WorkflowArtifactProfile; /** Workflow run ID for peer task seeding. */ workflowRunId: string; /** Workflow node ID for peer task seeding. */ @@ -211,6 +215,7 @@ function makeCtx(): TestCtx { const taskManager = new SpaceTaskManager(db, spaceId); const nodeExecutionRepo = new NodeExecutionRepository(db); const artifactRepo = new WorkflowRunArtifactRepository(db); + const artifactProfile = new CodingArtifactProfile({ db, artifactRepo }); // Session IDs for peers const taskAgentSessionId = 'session-task-agent'; @@ -259,6 +264,7 @@ function makeCtx(): TestCtx { spaceTaskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, workflowRunId, nodeId, coderSessionId, @@ -301,6 +307,7 @@ function makeConfig(ctx: TestCtx, overrides: NodeConfigOverrides = {}): NodeAgen workflow: null, gateDataRepo: new GateDataRepository(ctx.db), artifactRepo: ctx.artifactRepo, + artifactProfile: ctx.artifactProfile, ...configOverrides, }; } @@ -1062,96 +1069,20 @@ describe('node-agent-tools: save_artifact', () => { expect(data.error).toContain('summary'); }); - // ── Legacy compatibility shim ────────────────────────────────────────────── + // ── Shape required (legacy `type` alias removed) ────────────────────────── - test('legacy { type: "progress" } maps to a note shape', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - const result = await handlers.save_artifact({ type: 'progress', summary: 'halfway done' }); - const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('note'); - - const notes = ctx.artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'note' }); - expect(notes).toHaveLength(1); - expect(notes[0].data.summary).toBe('halfway done'); - }); - - test('legacy { type: "pr" } maps to a link kind:pr and normalizes data.url', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - const result = await handlers.save_artifact({ - type: 'pr', - data: { pr_url: 'https://github.com/acme/app/pull/7' }, - }); - const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('link'); - - const links = ctx.artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'link' }); - expect(links).toHaveLength(1); - expect(links[0].data.url).toBe('https://github.com/acme/app/pull/7'); - expect(links[0].data.kind).toBe('pr'); - }); - - test('legacy { type: "result", data: { pr_url } } (no summary) routes to link (data-aware)', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - const result = await handlers.save_artifact({ - type: 'result', - data: { pr_url: 'https://github.com/acme/app/pull/9' }, - }); - const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('link'); - expect(data.artifact.key).toBe('pr'); - }); - - test('legacy { type: "result", summary } routes to decision (data-aware)', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - const result = await handlers.save_artifact({ type: 'result', summary: 'shipped' }); - const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('decision'); - }); - - test('legacy { type: "result", summary + pr_url } keeps the summary as a decision', async () => { + test('rejects when shape is missing — the legacy `type` alias is no longer accepted', async () => { const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); + // `type` is silently stripped by the schema; with no `shape`, the handler + // rejects. This documents the cutover off the legacy freeform-type shim. const result = await handlers.save_artifact({ - type: 'result', - summary: 'QA passed', - data: { pr_url: 'https://github.com/acme/app/pull/9' }, + // @ts-expect-error — legacy `type` is no longer part of the schema + type: 'progress', + summary: 'halfway done', }); const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('decision'); - // pr_url preserved on the decision so PR readers still find it via the - // legacy-field fallback. - const decisions = ctx.artifactRepo.listByRun(ctx.workflowRunId, { - artifactType: 'decision', - }); - expect(decisions[0]?.data.summary).toBe('QA passed'); - expect(decisions[0]?.data.pr_url).toBe('https://github.com/acme/app/pull/9'); - }); - - test('legacy unknown freeform type is accepted as a note (keeps working)', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - const result = await handlers.save_artifact({ type: 'merge_blocked', summary: 'conflict' }); - const data = JSON.parse(result.content[0].text); - expect(data.success).toBe(true); - expect(data.artifact.shape).toBe('note'); - const notes = ctx.artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'note' }); - expect(notes).toHaveLength(1); - expect(notes[0]?.data._legacyType).toBe('merge_blocked'); - // Distinct key per unknown type so different blockers don't collapse. - expect(notes[0]?.artifactKey).toBe('merge_blocked'); - }); - - test('a shape NAME passed as legacy type is validated (no bypass)', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - // type:'link' without data.url must be rejected even though it uses the - // legacy alias (a shape name is not a legacy semantic type). - const result = await handlers.save_artifact({ type: 'link', data: { title: 'no url' } }); - const data = JSON.parse(result.content[0].text); expect(data.success).toBe(false); - expect(data.error).toContain('url'); + expect(data.error).toContain('shape'); }); }); @@ -3436,7 +3367,7 @@ describe('node-agent-tools: async gate evaluation', () => { gateId: gate.id, }, 'node-coder', - ctx.artifactRepo + ctx.artifactProfile ); const data = JSON.parse(result!.content[0].text); @@ -3490,7 +3421,7 @@ describe('node-agent-tools: async gate evaluation', () => { gateId: gate.id, }, 'node-coder', - ctx.artifactRepo + ctx.artifactProfile ); const data = JSON.parse(result!.content[0].text); @@ -5514,15 +5445,16 @@ describe('node-agent-tools: pr_url payloads do not auto-subscribe', () => { test('save_artifact with pr_url succeeds without requiring a subscription callback', async () => { const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); const result = await handlers.save_artifact({ - type: 'result', - data: { pr_url: 'https://github.com/acme/widgets/pull/123' }, + shape: 'link', + kind: 'pr', + data: { url: 'https://github.com/acme/widgets/pull/123' }, }); const data = JSON.parse(result.content[0].text); expect(data.success).toBe(true); const artifacts = ctx.artifactRepo.listByRun(ctx.workflowRunId); expect(artifacts).toHaveLength(1); - expect(artifacts[0]!.data.pr_url).toBe('https://github.com/acme/widgets/pull/123'); + expect(artifacts[0]!.data.url).toBe('https://github.com/acme/widgets/pull/123'); }); test('send_message with pr_url succeeds without requiring a subscription callback', async () => { diff --git a/packages/daemon/tests/unit/5-space/evolution-episode-service.test.ts b/packages/daemon/tests/unit/5-space/evolution-episode-service.test.ts index ecf59e8e51..d2c7af91e2 100644 --- a/packages/daemon/tests/unit/5-space/evolution-episode-service.test.ts +++ b/packages/daemon/tests/unit/5-space/evolution-episode-service.test.ts @@ -18,6 +18,8 @@ import { SpaceWorkflowRunRepository } from '../../../src/storage/repositories/sp import { WorkflowRunArtifactRepository } from '../../../src/storage/repositories/workflow-run-artifact-repository'; import { SpaceWorkflowRepository } from '../../../src/storage/repositories/space-workflow-repository'; import { SpaceGoalService } from '../../../src/lib/space/goals/goal-service'; +import { CodingArtifactProfile } from '../../../src/lib/space/workflows/coding-artifact-profile'; +import type { WorkflowArtifactProfile } from '../../../src/lib/space/runtime/artifact-profile'; import { createSpaceTables } from '../helpers/space-test-db'; describe('EvolutionEpisodeService', () => { @@ -26,6 +28,7 @@ describe('EvolutionEpisodeService', () => { let taskRepo: SpaceTaskRepository; let workflowRunRepo: SpaceWorkflowRunRepository; let artifactRepo: WorkflowRunArtifactRepository; + let artifactProfile: WorkflowArtifactProfile; let workflowRepo: SpaceWorkflowRepository; let goalRepo: SpaceGoalRepository; let spaceRepo: SpaceRepository; @@ -43,6 +46,7 @@ describe('EvolutionEpisodeService', () => { new GateOpenStateRepository(db as never) ); artifactRepo = new WorkflowRunArtifactRepository(db as never); + artifactProfile = new CodingArtifactProfile({ db: db as never, artifactRepo }); workflowRepo = new SpaceWorkflowRepository(db as never); spaceId = spaceRepo.createSpace({ workspacePath: '/workspace/episode-service-test', @@ -255,6 +259,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => { judgeCalled = true; return { title: 'Should not run', outcomeSummary: 'Nope', findings: [] }; @@ -346,6 +351,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const listedWithoutContext = scopeService.listEvidence(scope.id); expect(listedWithoutContext.preflightContext).toBeUndefined(); @@ -401,6 +407,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const input = service.buildEpisodeInput({ @@ -559,6 +566,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const before = service.buildEpisodeInput({ scopeId: scope.id, @@ -665,6 +673,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const input = service.buildEpisodeInput({ @@ -735,6 +744,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const input = service.buildEpisodeInput({ @@ -838,6 +848,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const prompt = buildEpisodeJudgePrompt( @@ -866,6 +877,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const prompt = buildEpisodeJudgePrompt( @@ -954,6 +966,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const input = service.buildEpisodeInput({ scopeId: scope.id, evidenceIds: [evidence.id] }); @@ -1088,6 +1101,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, db, taskCreatedEventHub: { publish: async (event, data) => { @@ -1162,6 +1176,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, goalService: createGoalService(), }); @@ -1207,6 +1222,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); expect(() => service.createTaskFromProposal(dismissed.id)).toThrow( @@ -1244,6 +1260,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); expect(() => @@ -1281,6 +1298,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, taskIdFactory: () => taskId, }); @@ -1337,12 +1355,14 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const service = new EvolutionEpisodeService({ evolutionRepo, taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, goalService: createGoalService(), }); const serviceOnlyEpisode = evolutionRepo.createEpisode({ @@ -1361,6 +1381,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, goalService: failingGoalService, }); const applied = service.applyRollupGoalUpdate({ @@ -1418,6 +1439,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const scope = scopeService.createScopeFromGoal({ spaceGoalId: goal.id, @@ -1487,6 +1509,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, goalService: createGoalService(), judgeEpisode: async (input) => ({ title: 'Forge evidence lifecycle episode', @@ -1675,6 +1698,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, goalService: createGoalService(), judgeEpisode: async (input) => ({ title: 'Forge MVP dogfood episode', @@ -1789,6 +1813,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async (input) => { judgePreflight = input.preflight; return { @@ -1900,6 +1925,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, }); const input = service.buildEpisodeInput({ scopeId: scope.id, @@ -1999,6 +2025,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2065,6 +2092,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2129,6 +2157,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2179,6 +2208,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2253,6 +2283,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2316,6 +2347,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2374,6 +2406,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2435,6 +2468,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2499,6 +2533,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2557,6 +2592,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2618,6 +2654,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', @@ -2676,6 +2713,7 @@ describe('EvolutionEpisodeService', () => { taskRepo, workflowRunRepo, artifactRepo, + artifactProfile, judgeEpisode: async () => ({ title: 'Episode', outcomeSummary: '', diff --git a/packages/daemon/tests/unit/5-space/runtime/external-event-delivery-e2e.test.ts b/packages/daemon/tests/unit/5-space/runtime/external-event-delivery-e2e.test.ts index 5e45f6b3cb..a7bc01c13f 100644 --- a/packages/daemon/tests/unit/5-space/runtime/external-event-delivery-e2e.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/external-event-delivery-e2e.test.ts @@ -47,6 +47,7 @@ import { SpaceRuntimeService } from '../../../../src/lib/space/runtime/space-run import type { TaskAgentManager } from '../../../../src/lib/space/runtime/task-agent-manager'; import { ChannelCycleRepository } from '../../../../src/storage/repositories/channel-cycle-repository'; import { GateDataRepository } from '../../../../src/storage/repositories/gate-data-repository'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile'; import { GateOpenStateRepository } from '../../../../src/storage/repositories/gate-open-state-repository'; import { NodeExecutionRepository } from '../../../../src/storage/repositories/node-execution-repository'; import { SpaceAgentRepository } from '../../../../src/storage/repositories/space-agent-repository'; @@ -417,6 +418,7 @@ async function setupE2E(): Promise { gateDataRepo, gateOpenStateRepo, channelCycleRepo, + artifactProfile: new CodingArtifactProfile({ db, gateDataRepo }), internalEventBus: bus, commandBus, externalEventStore: eventStore, diff --git a/packages/daemon/tests/unit/5-space/runtime/post-approval-routing-integration.test.ts b/packages/daemon/tests/unit/5-space/runtime/post-approval-routing-integration.test.ts index b3bfd54222..46f0a0e402 100644 --- a/packages/daemon/tests/unit/5-space/runtime/post-approval-routing-integration.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/post-approval-routing-integration.test.ts @@ -64,6 +64,7 @@ import { SpaceWorkflowRunRepository } from '../../../../src/storage/repositories import { SpaceAgentRepository } from '../../../../src/storage/repositories/space-agent-repository.ts'; import { NodeExecutionRepository } from '../../../../src/storage/repositories/node-execution-repository.ts'; import { WorkflowRunArtifactRepository } from '../../../../src/storage/repositories/workflow-run-artifact-repository.ts'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile.ts'; import { SpaceAgentManager } from '../../../../src/lib/space/managers/space-agent-manager.ts'; import { SpaceWorkflowManager } from '../../../../src/lib/space/managers/space-workflow-manager.ts'; import { SpaceManager } from '../../../../src/lib/space/managers/space-manager.ts'; @@ -195,6 +196,7 @@ function buildHarness(opts: { spawnerThrows?: boolean } = {}): Harness { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile: new CodingArtifactProfile({ db, artifactRepo }), onTaskUpdated: async ({ task }) => { emitted.push({ taskId: task.id, status: task.status }); }, diff --git a/packages/daemon/tests/unit/5-space/runtime/space-runtime-completion.test.ts b/packages/daemon/tests/unit/5-space/runtime/space-runtime-completion.test.ts index 34b4870fbd..d174575093 100644 --- a/packages/daemon/tests/unit/5-space/runtime/space-runtime-completion.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/space-runtime-completion.test.ts @@ -15,6 +15,7 @@ import { runMigrations } from '../../../../src/storage/schema/index.ts'; import { SpaceWorkflowRepository } from '../../../../src/storage/repositories/space-workflow-repository.ts'; import { SpaceWorkflowRunRepository } from '../../../../src/storage/repositories/space-workflow-run-repository.ts'; import { WorkflowRunArtifactRepository } from '../../../../src/storage/repositories/workflow-run-artifact-repository.ts'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile.ts'; import { SpaceTaskRepository } from '../../../../src/storage/repositories/space-task-repository.ts'; import { SpaceAgentRepository } from '../../../../src/storage/repositories/space-agent-repository.ts'; import { EvolutionScopeService } from '../../../../src/lib/space/evolution-scope-service.ts'; @@ -380,6 +381,7 @@ describe('SpaceRuntime — completion detection & status transitions', () => { taskRepo, artifactRepo, nodeExecutionRepo, + artifactProfile: new CodingArtifactProfile({ db, artifactRepo }), internalEventBus: bus, taskAgentManager: new MockTaskAgentManager(nodeExecutionRepo) as unknown as TaskAgentManager, ...extraConfig, diff --git a/packages/daemon/tests/unit/5-space/runtime/space-runtime-event-driven-gate-evaluation.test.ts b/packages/daemon/tests/unit/5-space/runtime/space-runtime-event-driven-gate-evaluation.test.ts index 050d13c0f8..f68b5eb37c 100644 --- a/packages/daemon/tests/unit/5-space/runtime/space-runtime-event-driven-gate-evaluation.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/space-runtime-event-driven-gate-evaluation.test.ts @@ -26,6 +26,7 @@ import { SpaceRuntimeService } from '../../../../src/lib/space/runtime/space-run import { SpaceRuntime } from '../../../../src/lib/space/runtime/space-runtime'; import type { TaskAgentManager } from '../../../../src/lib/space/runtime/task-agent-manager'; import { GateDataRepository } from '../../../../src/storage/repositories/gate-data-repository'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile'; import { NodeExecutionRepository } from '../../../../src/storage/repositories/node-execution-repository'; import { ChannelCycleRepository } from '../../../../src/storage/repositories/channel-cycle-repository'; import { GateOpenStateRepository } from '../../../../src/storage/repositories/gate-open-state-repository'; @@ -172,6 +173,7 @@ async function setup(options: { gateDataRepo, gateOpenStateRepo, channelCycleRepo, + artifactProfile: new CodingArtifactProfile({ db, gateDataRepo }), internalEventBus: bus, commandBus, externalEventStore: eventStore, @@ -901,6 +903,7 @@ describe('SpaceRuntimeService event-driven gate evaluation', () => { workflowRunRepo, taskRepo, nodeExecutionRepo, + artifactProfile: new CodingArtifactProfile({ db }), internalEventBus: bus, commandBus, externalEventStore: new ExternalEventStore(db), @@ -1146,6 +1149,7 @@ describe('SpaceRuntimeService event-driven gate evaluation', () => { gateDataRepo, gateOpenStateRepo, channelCycleRepo, + artifactProfile: new CodingArtifactProfile({ db, gateDataRepo }), internalEventBus: bus, commandBus, externalEventStore: eventStore, diff --git a/packages/daemon/tests/unit/5-space/runtime/space-runtime-external-events.test.ts b/packages/daemon/tests/unit/5-space/runtime/space-runtime-external-events.test.ts index 8f980a873c..a6a68504be 100644 --- a/packages/daemon/tests/unit/5-space/runtime/space-runtime-external-events.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/space-runtime-external-events.test.ts @@ -22,6 +22,8 @@ import { SpaceTaskRepository } from '../../../../src/storage/repositories/space- import { SpaceWorkflowRepository } from '../../../../src/storage/repositories/space-workflow-repository'; import { SpaceWorkflowRunRepository } from '../../../../src/storage/repositories/space-workflow-run-repository'; import { WorkflowRunArtifactRepository } from '../../../../src/storage/repositories/workflow-run-artifact-repository'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile'; +import type { WorkflowArtifactProfile } from '../../../../src/lib/space/runtime/artifact-profile'; import { createSpaceTables } from '../../helpers/space-test-db'; setDefaultTimeout(10_000); @@ -163,6 +165,7 @@ describe('SpaceRuntime external event subscriptions', () => { let taskRepo: SpaceTaskRepository; let nodeExecutionRepo: NodeExecutionRepository; let artifactRepo: WorkflowRunArtifactRepository; + let artifactProfile: WorkflowArtifactProfile; let workflowManager: SpaceWorkflowManager; let runtime: SpaceRuntime; let eventStore: ExternalEventStore; @@ -248,6 +251,7 @@ describe('SpaceRuntime external event subscriptions', () => { }); tam = new MockTaskAgentManager(); artifactRepo = new WorkflowRunArtifactRepository(db); + artifactProfile = new CodingArtifactProfile({ db, artifactRepo }); spaceManager = new SpaceManager(db); runtime = new SpaceRuntime({ db, @@ -258,6 +262,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus, externalEventStore: eventStore, @@ -1095,6 +1100,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus: createInternalCommandBus(), externalEventStore: eventStore, @@ -1127,6 +1133,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus: createInternalCommandBus(), externalEventStore: eventStore, @@ -1422,6 +1429,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus: createInternalCommandBus(), externalEventStore: eventStore, @@ -1827,6 +1835,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus: createInternalCommandBus(), externalEventStore: eventStore, @@ -1871,6 +1880,7 @@ describe('SpaceRuntime external event subscriptions', () => { taskRepo, nodeExecutionRepo, artifactRepo, + artifactProfile, internalEventBus: bus, commandBus: createInternalCommandBus(), externalEventStore: eventStore, @@ -7240,6 +7250,7 @@ describe('SpaceRuntime event-driven gate evaluation', () => { taskRepo, nodeExecutionRepo, gateDataRepo, + artifactProfile: new CodingArtifactProfile({ db, gateDataRepo }), internalEventBus: bus, commandBus: localCommandBus, externalEventStore: eventStore, diff --git a/packages/daemon/tests/unit/5-space/runtime/space-runtime-rehydration.test.ts b/packages/daemon/tests/unit/5-space/runtime/space-runtime-rehydration.test.ts index 7cce08f7e6..cdd0bcb57c 100644 --- a/packages/daemon/tests/unit/5-space/runtime/space-runtime-rehydration.test.ts +++ b/packages/daemon/tests/unit/5-space/runtime/space-runtime-rehydration.test.ts @@ -22,6 +22,7 @@ import { SpaceAgentRepository } from '../../../../src/storage/repositories/space import { NodeExecutionRepository } from '../../../../src/storage/repositories/node-execution-repository.ts'; import { GateDataRepository } from '../../../../src/storage/repositories/gate-data-repository.ts'; import { WorkflowRunArtifactRepository } from '../../../../src/storage/repositories/workflow-run-artifact-repository.ts'; +import { CodingArtifactProfile } from '../../../../src/lib/space/workflows/coding-artifact-profile.ts'; import { ToolContinuationRecoveryRepository } from '../../../../src/storage/repositories/tool-continuation-recovery-repository.ts'; import { SpaceAgentManager } from '../../../../src/lib/space/managers/space-agent-manager.ts'; import { SpaceWorkflowManager } from '../../../../src/lib/space/managers/space-workflow-manager.ts'; @@ -101,6 +102,8 @@ describe('SpaceRuntime — crash recovery and rehydration', () => { const STEP_B = 'step-b'; function makeRuntime(overrides?: Partial): SpaceRuntime { + const artifactRepo = overrides?.artifactRepo; + const artifactProfile = new CodingArtifactProfile(artifactRepo ? { db, artifactRepo } : { db }); return new SpaceRuntime({ db, spaceManager, @@ -109,6 +112,7 @@ describe('SpaceRuntime — crash recovery and rehydration', () => { workflowRunRepo, taskRepo, nodeExecutionRepo: new NodeExecutionRepository(db), + artifactProfile, ...overrides, }); } diff --git a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts index 3af9f34c5f..0d610e9a27 100644 --- a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts +++ b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts @@ -4453,7 +4453,7 @@ describe('REVIEW_ONLY_WORKFLOW reviewer customPrompt requires gh pr review befor const agent = REVIEW_ONLY_WORKFLOW.nodes[0].agents[0]; const prompt = agent.customPrompt!.value; expect(prompt).toContain('visible GitHub review'); - expect(prompt).toContain('save a result artifact'); + expect(prompt).toContain('record the PR'); }); }); diff --git a/packages/daemon/tests/unit/5-space/workflow/end-node-handoff.test.ts b/packages/daemon/tests/unit/5-space/workflow/end-node-handoff.test.ts index 70a8cdaf43..7e11f012b9 100644 --- a/packages/daemon/tests/unit/5-space/workflow/end-node-handoff.test.ts +++ b/packages/daemon/tests/unit/5-space/workflow/end-node-handoff.test.ts @@ -408,7 +408,7 @@ describe('Post-approval merger template (redesigned: report blockers to Reviewer test('escalates to space-agent only on cycle-cap or unresolvable blocker', () => { const text = PR_MERGE_POST_APPROVAL_INSTRUCTIONS; expect(text).toContain('send_message(target="space-agent"'); - expect(text).toContain('type: "merge_blocked"'); + expect(text).toContain('shape: "note", kind: "merge_blocked"'); expect(text).toContain('exit_reason'); expect(text).toContain('Do NOT mark the task complete'); }); @@ -422,7 +422,7 @@ describe('Post-approval merger template (redesigned: report blockers to Reviewer expect(text).toContain('IS_FORK='); expect(text).toContain('git push origin --delete'); expect(text).toMatch(/BEST-EFFORT/); - expect(text).toContain('type: "result"'); + expect(text).toContain('shape: "link", kind: "merge"'); }); test('preserved: root-repo sync uses {{workspace_path}}, branch-agnostic $BASE', () => { From 19a723c2979e71521e52caff32da992a8f8378d9 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Sun, 2 Aug 2026 23:40:43 -0400 Subject: [PATCH 03/16] fix(space): review-trigger reads current payload; QA records PR as a link Address review feedback on #2345: - CodingArtifactProfile.onGateDataCommitted now reads review_url from the current send_message payload (messageData) instead of the merged gate state. The prior form would spuriously record an extra review round when a later send updated only comment_urls while a previous round's review_url lingered in the gate state. Adds a regression test for the follow-up-send case. - Fullstack QA success now records the PR as a first-class `link kind:'pr'` (consistent with the other reviewer nodes and robust to legacy-field removal) alongside the terminal `decision` outcome, instead of burying pr_url inside the decision data. - Rebase onto dev: drop the list_artifacts legacy kind-filter and its tests added in #2319 (they re-introduced `pr`/`review` kind names into core, which PR C removes); list_artifacts now filters by shape only. --- .../lib/space/workflows/built-in-workflows.ts | 13 +- .../workflows/coding-artifact-profile.ts | 14 +- .../5-space/agent/node-agent-tools.test.ts | 121 +++++++++--------- 3 files changed, 75 insertions(+), 73 deletions(-) diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index 1d4eb40e66..edbe408906 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1261,11 +1261,14 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = { 'carry the same approval semantic. Leave the workflow open for the next ' + 'Coding cycle.\n' + '8. If all green:\n' + - ' a. Call `save_artifact({ shape: "decision", summary, data: { pr_url: "", test_output: "", ui_changed: , dev_server_started: , browser_validation: "" } })` ' + - 'to record the terminal outcome. The `pr_url` inside `data` is what ' + - '`dispatchPostApproval` reads when interpolating `{{pr_url}}` into the ' + - 'merge template — top-level keys outside `data` are silently stripped by ' + - 'the tool schema, so nest it correctly.\n' + + ' a. Record the PR and the terminal QA outcome as two artifacts: ' + + '`save_artifact({ shape: "link", kind: "pr", data: { url: "" } })` ' + + '(the canonical PR record the post-approval merge step resolves as the ' + + 'primary link) and `save_artifact({ shape: "decision", summary, data: { ' + + 'test_output: "", ui_changed: , dev_server_started: , ' + + 'browser_validation: "" } })` (the terminal ' + + 'outcome summary). Top-level keys outside `data` are silently stripped by the ' + + 'tool schema, so nest fields correctly.\n' + ' b. Call `approve_task()` as your final action. If autonomy blocks self-close, ' + 'call `submit_for_approval({ reason: "..." })` instead — the runtime will ' + 'still route post-approval once the human approves. Do NOT run `gh pr merge` ' + diff --git a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts index 67e8bdbb7a..21aa3fd900 100644 --- a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts +++ b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts @@ -157,12 +157,16 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { async onGateDataCommitted(event: GateDataCommittedEvent): Promise { if (!this.artifactRepo) return; - const { runId, nodeId, gateId, gateData, messageData } = event; - // Multi-round review history: every time the reviewer writes a `review_url` - // to the review-posted-gate, persist one `decision kind:'review'` per cycle - // (round-0, round-1 …) keyed so each round is a distinct upsert. + const { runId, nodeId, gateId, messageData } = event; + // Multi-round review history: every time the reviewer DELIVERS a fresh + // `review_url` on the review-posted-gate, persist one `decision kind:'review'` + // per cycle (round-0, round-1 …) keyed so each round is a distinct upsert. + // Read the URL from the current `send_message` payload (messageData), NOT the + // merged gate state — a later send that only updates comment_urls would still + // see the prior round's review_url in the gate state and spuriously record an + // extra round. if (gateId !== REVIEW_POSTED_GATE) return; - const reviewUrl = gateData.review_url; + const reviewUrl = messageData?.review_url; if (typeof reviewUrl !== 'string' || reviewUrl.length === 0) return; try { diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index 273aebcf2f..171d832371 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -1128,49 +1128,6 @@ describe('node-agent-tools: list_artifacts', () => { }>; } - test('legacy { type: "pr" } post-filters to kind:pr links (excludes issue/preview)', async () => { - const handlers = await seedMixed(); - const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'pr' })); - // Before the kind post-filter, this over-returned all three links. - expect(artifacts).toHaveLength(1); - expect(artifacts[0].type).toBe('link'); - expect(artifacts[0].data.kind).toBe('pr'); - expect(artifacts[0].data.url).toBe('https://example.com/pr/1'); - }); - - test('legacy { type: "review" } post-filters to kind:review decisions (excludes gate)', async () => { - const handlers = await seedMixed(); - const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'review' })); - expect(artifacts).toHaveLength(1); - expect(artifacts[0].type).toBe('decision'); - expect(artifacts[0].data.kind).toBe('review'); - }); - - test('legacy { type: "result" } stays unfiltered (overloaded decision|link, all kinds)', async () => { - const handlers = await seedMixed(); - const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'result' })); - // 3 links + 2 decisions, including non-pr links and the gate decision — - // i.e. no kind post-filter is applied to the overloaded `result` type. - expect(artifacts).toHaveLength(5); - expect(artifacts.some((a) => a.data.kind === 'gate')).toBe(true); - expect(artifacts.some((a) => a.data.kind === 'issue')).toBe(true); - }); - - test('legacy { type: "progress" } stays unfiltered (note)', async () => { - const handlers = await seedMixed(); - const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'progress' })); - expect(artifacts).toHaveLength(1); - expect(artifacts[0].type).toBe('note'); - }); - - test('legacy { type: "pr" } save round-trips through { type: "pr" } list', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - await handlers.save_artifact({ type: 'pr', data: { pr_url: 'https://example.com/pr/7' } }); - const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'pr' })); - expect(artifacts).toHaveLength(1); - expect(artifacts[0].data.kind).toBe('pr'); - }); - test('direct shape name { type: "link" } returns all links (no kind filter)', async () => { const handlers = await seedMixed(); const artifacts = artifactsOf(await handlers.list_artifacts({ type: 'link' })); @@ -1183,26 +1140,6 @@ describe('node-agent-tools: list_artifacts', () => { const artifacts = artifactsOf(await handlers.list_artifacts({})); expect(artifacts).toHaveLength(6); }); - - test('kind-less link/decision rows are excluded from legacy pr/review filters', async () => { - const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); - // Direct shape writes with no kind tag are legitimately-not-PR / not-review: - // the backfill migration (migrations.ts:11290-11299) only tags - // pr/review/result-derived rows, so a genuinely kind-less link/decision is - // an untagged direct write that the legacy pr/review filters must exclude. - // Documents the intentional narrowing at the migration boundary. - await handlers.save_artifact({ shape: 'link', data: { url: 'https://example.com/doc' } }); - await handlers.save_artifact({ - shape: 'decision', - data: { recommendation: 'ship it' }, - }); - - expect(artifactsOf(await handlers.list_artifacts({ type: 'pr' }))).toHaveLength(0); - expect(artifactsOf(await handlers.list_artifacts({ type: 'review' }))).toHaveLength(0); - // Direct shape queries are unfiltered and still return them. - expect(artifactsOf(await handlers.list_artifacts({ type: 'link' }))).toHaveLength(1); - expect(artifactsOf(await handlers.list_artifacts({ type: 'decision' }))).toHaveLength(1); - }); }); // --------------------------------------------------------------------------- @@ -3927,6 +3864,64 @@ describe('node-agent-tools: review-posted-gate multi-round artifact history', () expect(artifacts).toHaveLength(0); }); + test('a follow-up send with only comment_urls does not append a spurious review round', async () => { + // Regression: the trigger must read the CURRENT write payload's review_url, + // not the merged gate state. After round-0 persists review_url in the gate, + // a later send that only updates comment_urls must NOT create a round-1 + // decision (the prior code read gateData.review_url and would double-count). + const { WorkflowRunArtifactRepository } = await import( + '../../../../src/storage/repositories/workflow-run-artifact-repository.ts' + ); + const artifactRepo = new WorkflowRunArtifactRepository(ctx.db); + const gate: Gate = { + id: 'review-posted-gate', + fields: [ + { name: 'review_url', type: 'string', writers: ['reviewer'], check: { op: 'exists' } }, + { name: 'comment_urls', type: 'string', writers: ['reviewer'], check: { op: 'exists' } }, + ], + resetOnCycle: false, + }; + const workflow: SpaceWorkflow = { + id: 'wf-followup', + spaceId: ctx.spaceId, + name: 'Test', + description: '', + nodes: [], + startNodeId: '', + rules: [], + tags: [], + channels: [ + { id: 'ch-review-coder', from: 'reviewer', to: 'coder', gateId: 'review-posted-gate' }, + ], + gates: [gate], + }; + const config = makeConfig(ctx, { + workflow, + myAgentName: 'reviewer', + mySessionId: ctx.reviewerSessionId, + artifactRepo, + }); + const handlers = createNodeAgentToolHandlers(config); + + // Round 0: deliver a review_url → one review decision. + await handlers.send_message({ + target: 'coder', + message: 'reviewed', + data: { review_url: 'https://github.com/acme/app/pull/42#pullrequestreview-1' }, + }); + // Follow-up: only comment_urls, no fresh review_url in the payload. + await handlers.send_message({ + target: 'coder', + message: 'thread replies', + data: { comment_urls: ['https://github.com/acme/app/pull/42#discussion_r1'] }, + }); + + // Still exactly one review decision — no spurious round-1. + const artifacts = artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'decision' }); + expect(artifacts).toHaveLength(1); + expect(artifacts[0]?.artifactKey).toBe('review:round-0'); + }); + test('skips artifact append for non-review-posted-gate gates (no false positives)', async () => { const { WorkflowRunArtifactRepository } = await import( '../../../../src/storage/repositories/workflow-run-artifact-repository.ts' From 28deeb3ab629d829a180d769509e36a058f2c1d2 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Sun, 2 Aug 2026 23:54:33 -0400 Subject: [PATCH 04/16] fix(space): classify legacy merge_conflict_loop notes with warning tone The live-query warning-tone filter added `merge_conflict` to the new-kind path but the legacy `_legacyType` backcompat list was missing `merge_conflict_loop`, so old backfilled conflict rows stayed 'progress' while new shape rows became 'warning'. Add the legacy type name so both render the same tone. --- packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts b/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts index a19a39888e..92c696d8e4 100644 --- a/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts +++ b/packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts @@ -1226,7 +1226,9 @@ artifact_rows AS ( AND json_valid(wra.data) AND ( json_extract(wra.data, '$.kind') IN ('merge_blocked', 'merge_conflict', 'cleanup_warning') - OR json_extract(wra.data, '$._legacyType') IN ('merge_blocked', 'cleanup_warning') + OR json_extract(wra.data, '$._legacyType') IN ( + 'merge_blocked', 'merge_conflict_loop', 'cleanup_warning' + ) ) THEN 'warning' WHEN wra.artifact_type = 'note' THEN 'progress' From 9c89d3c3ff5ac3b590c9844b9530dd68c2c4f867 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Sun, 2 Aug 2026 23:59:59 -0400 Subject: [PATCH 05/16] fix(space): migrate end-node REC + list_artifacts schema off legacy type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review REQUEST_CHANGES on #2345 — two prompt sites the legacy type/append shim was still propping up: - task-agent-manager end-node Runtime Execution Contract (injected into every end-node session) still emitted save_artifact({ type: "result", append: true, summary }) — now rejected with the shim gone, so the terminal outcome was never recorded. Use shape: "decision". - ListArtifactsSchema.type .describe() advertised legacy 'progress'/'result'/ 'review', which now silently return empty. Update to the shape vocabulary. --- packages/daemon/src/lib/space/runtime/task-agent-manager.ts | 4 ++-- .../daemon/src/lib/space/tools/node-agent-tool-schemas.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts index aafaf362b6..c05b8b0bc0 100644 --- a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts +++ b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts @@ -3111,11 +3111,11 @@ export class TaskAgentManager { if (isEndNode) { if (approveUnlocked) { lines.push( - 'When your work is complete: (1) call save_artifact({ type: "result", append: true, summary: "..." }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "..." }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' ); } else { lines.push( - 'When your work is complete: (1) call save_artifact({ type: "result", append: true, summary: "..." }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "..." }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' ); } } diff --git a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts index 65c8cbcb43..5ee3fc5d02 100644 --- a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts +++ b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts @@ -421,10 +421,10 @@ export type ReadGateInput = z.infer; export const ListArtifactsSchema = z.object({ /** Filter by originating node ID. */ nodeId: z.string().describe('Filter by node ID').optional(), - /** Filter by artifact type (generic string, e.g. 'progress', 'result', 'review'). */ + /** Filter by artifact shape from the closed vocabulary (link/commit_set/check/metric/decision/note). */ type: z .string() - .describe('Filter by artifact type (e.g. "progress", "result", "review")') + .describe('Filter by artifact shape (e.g. "link", "decision", "note")') .optional(), }); From 28f7cdbdc36125475f17d6fd7bbfc562b9620c24 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Wed, 5 Aug 2026 23:57:10 -0400 Subject: [PATCH 06/16] fix(space): outcome decisions carry data.recommendation so save_artifact persists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review P1 on #2345: validateArtifactShape('decision') requires data.recommendation, so every migrated outcome `decision` that omitted it was silently rejected by save_artifact — the terminal outcome was never recorded before approve_task / submit_for_approval. Add recommendation to the four live sites: - built-in-workflows dispatcher (stack + 2nd mention): recommendation 'dispatched' - built-in-workflows Fullstack QA all-green: recommendation 'pass' - task-agent-manager end-node Runtime Execution Contract (both autonomy branches): recommendation 'completed' Adding recommendation does not set a kind, so summarizeRunOutcome still treats these as the terminal outcome. --- packages/daemon/src/lib/space/runtime/task-agent-manager.ts | 4 ++-- .../daemon/src/lib/space/workflows/built-in-workflows.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts index c05b8b0bc0..47e0487677 100644 --- a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts +++ b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts @@ -3111,11 +3111,11 @@ export class TaskAgentManager { if (isEndNode) { if (approveUnlocked) { lines.push( - 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "..." }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' ); } else { lines.push( - 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "..." }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' ); } } diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index edbe408906..f118142cf6 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -338,7 +338,7 @@ const PD_TASK_DISPATCHER_PROMPT = '4. Collect the returned task IDs. Build a stack map: ' + '{ prefix, items: [{ title, task_id, branch, base_branch, position }] }.\n' + '5. Call `save_artifact({ shape: "decision", summary: "Created N tasks from plan: ", ' + - 'data: { created_task_ids: [], stack_prefix: "", ' + + 'data: { recommendation: "dispatched", created_task_ids: [], stack_prefix: "", ' + 'stack_branches: ["plan//", "plan//", ...] } })` to record the dispatch outcome.\n' + '6. Call `approve_task()` as your final action. If autonomy blocks self-close, call ' + '`submit_for_approval({ reason: "..." })` instead.\n\n' + @@ -1085,7 +1085,7 @@ export const PLAN_AND_DECOMPOSE_WORKFLOW: SpaceWorkflow = { '\n\n' + 'Expected inputs: An approved plan PR (all 4 reviewers sent approved votes).\n' + 'Expected outputs: One standalone task per actionable work item in the plan, ' + - 'then save_artifact({ shape: "decision", data: { created_task_ids: [...] } }).\n\n' + + 'then save_artifact({ shape: "decision", summary: "Dispatched N tasks", data: { recommendation: "dispatched", created_task_ids: [...] } }).\n\n' + 'Tool contract:\n' + "- `create_standalone_task` is available from the space's MCP server and " + 'creates a task owned by the same space as this workflow.', @@ -1265,7 +1265,7 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = { '`save_artifact({ shape: "link", kind: "pr", data: { url: "" } })` ' + '(the canonical PR record the post-approval merge step resolves as the ' + 'primary link) and `save_artifact({ shape: "decision", summary, data: { ' + - 'test_output: "", ui_changed: , dev_server_started: , ' + + 'recommendation: "pass", test_output: "", ui_changed: , dev_server_started: , ' + 'browser_validation: "" } })` (the terminal ' + 'outcome summary). Top-level keys outside `data` are silently stripped by the ' + 'tool schema, so nest fields correctly.\n' + From 8626073a03c66356ffda7cab51edff5c303eb7ba Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Wed, 5 Aug 2026 23:57:50 -0400 Subject: [PATCH 07/16] fix(space): honor explicit note key so per-attempt audit notes don't overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review P2 on #2345: deriveArtifactKey('note', …) always keyed 'current' and ignored `key`, so each merge-conflict retry from the post-approval node overwrote the prior attempt's approved_head_oid / conflicting_files — a regression from the dropped append semantics, and the loop was no longer auditable. Extend deriveArtifactKey so `note` honors an explicit key (namespaced by kind, like `decision` does), while a note WITHOUT an explicit key stays the single rolling 'current' row. The merge-conflict prompt now passes key: "attempt-", so each attempt persists as a distinct row. SaveArtifactSchema docs updated to describe the multi-instance note case. --- .../lib/space/tools/node-agent-tool-schemas.ts | 10 ++++++---- packages/shared/src/artifact-shapes.ts | 16 ++++++++++++---- packages/shared/tests/artifact-shapes.test.ts | 16 ++++++++++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts index 5ee3fc5d02..04c01c1c85 100644 --- a/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts +++ b/packages/daemon/src/lib/space/tools/node-agent-tool-schemas.ts @@ -160,14 +160,16 @@ export type GetExternalEventInput = z.infer; * `ci`, `review`). Infra never enumerates domain kinds. * * Identity is shape-aware and derived automatically (see examples below), so a - * `note` is a single rolling status that upserts in place (no per-round growth), - * a `link` is one-per-kind, and a `decision` is single-terminal unless you pass - * an explicit multi-round `key` (e.g. 'round-0'). + * `note` is a single rolling status that upserts in place, a `link` is + * one-per-kind, and a `decision` is single-terminal — unless you pass an + * explicit multi-instance `key` (e.g. a decision 'round-0', or a note + * 'attempt-0' for a per-attempt audit trail). * * PR / preview / doc: save_artifact({ shape: 'link', kind: 'pr', data: { url, title } }) * CI / tests: save_artifact({ shape: 'check', data: { name: 'ci', status: 'pass', counts } }) * Review verdict: save_artifact({ shape: 'decision', kind:'review', data: { recommendation: 'approve', summary } }) * Multi-round history: save_artifact({ shape: 'decision', kind:'review', key: 'round-0', data: {...} }) + * Per-attempt audit: save_artifact({ shape: 'note', kind:'merge_conflict', key: 'attempt-0', data: { text } }) * Rolling status: save_artifact({ shape: 'note', data: { text: 'writing tests' } }) */ export const SaveArtifactSchema = z.object({ @@ -200,7 +202,7 @@ export const SaveArtifactSchema = z.object({ key: z .string() .describe( - "Identity key override. Derived from the shape by default. Pass an explicit value only for multi-round history (e.g. decision key: 'round-0')." + "Identity key override. Derived from the shape by default. Pass an explicit value only for multi-instance shapes — multi-round history (decision key: 'round-0') or per-attempt audit trails (note key: 'attempt-0')." ) .optional(), /** ≤1 sentence human note. Stored under data.summary (note/decision). */ diff --git a/packages/shared/src/artifact-shapes.ts b/packages/shared/src/artifact-shapes.ts index d72260b1b6..57065cf852 100644 --- a/packages/shared/src/artifact-shapes.ts +++ b/packages/shared/src/artifact-shapes.ts @@ -206,7 +206,9 @@ export function normalizeLinkData(data: Record): Record): Record { ).toBe('gate:round-0'); }); - test('non-decision shapes ignore explicitKey (note stays a single rolling row)', () => { - expect(deriveArtifactKey('note', { text: 'a' }, 'round-5')).toBe('current'); + test('note without explicitKey stays a single rolling row; with explicitKey is multi-instance', () => { + // Default: single rolling 'current' row regardless of kind. + expect(deriveArtifactKey('note', { text: 'a' })).toBe('current'); + expect(deriveArtifactKey('note', { text: 'b', kind: 'status' })).toBe('current'); + // Explicit key opts into a bounded multi-instance audit trail, namespaced + // by kind so distinct note streams (e.g. merge-conflict attempts) never + // collapse onto each other. + expect(deriveArtifactKey('note', { text: 'a', kind: 'merge_conflict' }, 'attempt-0')).toBe( + 'merge_conflict:attempt-0' + ); + expect(deriveArtifactKey('note', { text: 'a' }, 'attempt-0')).toBe('attempt-0'); + }); + + test('other non-decision shapes ignore explicitKey (derived identity only)', () => { expect(deriveArtifactKey('link', { url: 'u', kind: 'pr' }, 'override')).toBe('pr'); expect(deriveArtifactKey('check', { name: 'ci', status: 'pass' }, 'override')).toBe('ci'); }); From 678469f34ddcc004abb9a97f239d6954254915d0 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Wed, 5 Aug 2026 23:58:31 -0400 Subject: [PATCH 08/16] fix(space): resolve freshest PR across sources; gate hook reads committed fields Two related review/Codex findings on #2345: - resolvePrimaryLinkUrl returned a gate-data pr_url immediately (sequential priority) instead of comparing freshness across sources. dispatchPostApproval resolves {{pr_url}} through it, so a stale handoff URL could launch the merge session against the wrong PR when a newer link kind:'pr' artifact existed. Rewrite to gather candidates from gate data, hook state, and artifacts, and return the one with the greatest updatedAt. - onGateDataCommitted read review_url from the raw send payload (messageData), so a field the agent sent but was not authorized to write could still trigger a review decision. Pass the committed gate-field subset (committedData) and read review_url from it; comment_urls (non-gate metadata) still travels via messageData. Equivalent for review-posted-gate (review_url is an authorized field), strictly more correct for the general case. --- .../src/lib/space/runtime/artifact-profile.ts | 9 ++- .../src/lib/space/tools/node-agent-tools.ts | 5 ++ .../workflows/coding-artifact-profile.ts | 64 ++++++++++--------- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/packages/daemon/src/lib/space/runtime/artifact-profile.ts b/packages/daemon/src/lib/space/runtime/artifact-profile.ts index c938f1d1c1..120581201a 100644 --- a/packages/daemon/src/lib/space/runtime/artifact-profile.ts +++ b/packages/daemon/src/lib/space/runtime/artifact-profile.ts @@ -24,7 +24,14 @@ export interface GateDataCommittedEvent { gateId: string; /** The committed gate data (after the field merge). */ gateData: Record; - /** The `data` payload from the originating `send_message` call. */ + /** + * The gate-declared fields the sender was authorized to write in this + * `send_message` (the subset of `data` that actually committed to gate_data). + * Domain hooks key side-artifacts off these, not the raw payload, so a field + * the agent sent but was not authorized to write cannot trigger a side-effect. + */ + committedData?: Record; + /** The raw `data` payload from the originating `send_message` call. */ messageData?: Record; } diff --git a/packages/daemon/src/lib/space/tools/node-agent-tools.ts b/packages/daemon/src/lib/space/tools/node-agent-tools.ts index 0b068f61d8..da29e1df02 100644 --- a/packages/daemon/src/lib/space/tools/node-agent-tools.ts +++ b/packages/daemon/src/lib/space/tools/node-agent-tools.ts @@ -1057,6 +1057,11 @@ export function createNodeAgentToolHandlers(config: NodeAgentToolsConfig) { nodeId: workflowNodeId, gateId, gateData: updated.data, + // The gate-declared fields the sender was authorized to + // write in this send — the committed write, not the raw + // payload, so a non-authorizable field can't trigger a + // domain side-artifact. + committedData: authorizedData, messageData: data, }); } catch (err) { diff --git a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts index 21aa3fd900..9439958ba5 100644 --- a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts +++ b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts @@ -62,13 +62,24 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { } resolvePrimaryLinkUrl(runId: string): string { - // 1. Gate data — most recently updated record carrying a PR URL. + // The primary link is the FRESHEST eligible PR URL across ALL sources + // (gate data, hook state, artifacts), compared by updatedAt — so a newer + // `link kind:'pr'` artifact supersedes a stale gate-data `pr_url` (and vice + // versa). A generic `url` on a non-pr artifact never qualifies. + type Candidate = { url: string; updatedAt: number }; + let best: Candidate | null = null; + // Pure fresher (no closure mutation) so TS control-flow tracks `best`. + const fresher = (prev: Candidate | null, url: string, updatedAt: number): Candidate | null => { + if (!url) return prev; + if (!prev || updatedAt > prev.updatedAt) return { url, updatedAt }; + return prev; + }; + + // 1. Gate data — records carrying a legacy pr_url/prUrl. try { const gateDataRepo = this.sharedGateDataRepo ?? new GateDataRepository(this.db); - const gateRecords = gateDataRepo.listByRun(runId).sort((a, b) => b.updatedAt - a.updatedAt); - for (const record of gateRecords) { - const candidate = legacyPrUrl(record.data); - if (candidate) return candidate; + for (const record of gateDataRepo.listByRun(runId)) { + best = fresher(best, legacyPrUrl(record.data), record.updatedAt); } } catch (err) { log.warn( @@ -80,12 +91,8 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { // successful send_message even when the gate schema does not declare it. try { const hookStateRepo = new WorkflowHookStateRepository(this.db); - const hookStates = hookStateRepo - .listByRun(runId) - .sort((a, b) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)); - for (const snapshot of hookStates) { - const candidate = legacyPrUrl(snapshot.localState); - if (candidate) return candidate; + for (const snapshot of hookStateRepo.listByRun(runId)) { + best = fresher(best, legacyPrUrl(snapshot.localState), snapshot.updatedAt ?? 0); } } catch (err) { log.warn( @@ -93,26 +100,19 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { ); } - // 3. Artifacts — the most recently updated eligible candidate. A `link` - // kind:'pr' (read via data.url) qualifies, as does a legacy row carrying - // pr_url/prUrl, so a newer legacy PR is never shadowed by an older shape - // link (or vice versa). A generic data.url on a non-pr artifact never - // qualifies. + // 3. Artifacts — a `link kind:'pr'` (read via data.url) or a legacy row + // carrying pr_url/prUrl. if (this.artifactRepo) { try { - const artifacts = this.artifactRepo.listByRun(runId); - let best: { url: string; updatedAt: number } | null = null; - for (const a of artifacts) { + for (const a of this.artifactRepo.listByRun(runId)) { const url = a.artifactType === 'link' && a.data.kind === 'pr' ? typeof a.data.url === 'string' ? a.data.url : '' : legacyPrUrl(a.data); - if (!url) continue; - if (!best || a.updatedAt > best.updatedAt) best = { url, updatedAt: a.updatedAt }; + best = fresher(best, url, a.updatedAt); } - if (best) return best.url; } catch (err) { log.warn( `resolvePrimaryLinkUrl: failed to read artifacts for run ${runId}: ${err instanceof Error ? err.message : String(err)}` @@ -120,7 +120,7 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { } } - return ''; + return best?.url ?? ''; } summarizeRunOutcome(runId: string): string | null { @@ -157,16 +157,18 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { async onGateDataCommitted(event: GateDataCommittedEvent): Promise { if (!this.artifactRepo) return; - const { runId, nodeId, gateId, messageData } = event; - // Multi-round review history: every time the reviewer DELIVERS a fresh + const { runId, nodeId, gateId, committedData, messageData } = event; + // Multi-round review history: every time the reviewer COMMITS a fresh // `review_url` on the review-posted-gate, persist one `decision kind:'review'` // per cycle (round-0, round-1 …) keyed so each round is a distinct upsert. - // Read the URL from the current `send_message` payload (messageData), NOT the - // merged gate state — a later send that only updates comment_urls would still - // see the prior round's review_url in the gate state and spuriously record an - // extra round. + // Read the URL from `committedData` — the gate-declared fields the sender was + // authorized to write in THIS send (not the raw payload, and not the merged + // gate state). This avoids two failure modes: (a) a later send that only + // updates comment_urls would see the prior round's review_url in the merged + // gate state and spuriously record an extra round; (b) a field the agent sent + // but was not authorized to write must not trigger a side-artifact. if (gateId !== REVIEW_POSTED_GATE) return; - const reviewUrl = messageData?.review_url; + const reviewUrl = committedData?.review_url; if (typeof reviewUrl !== 'string' || reviewUrl.length === 0) return; try { @@ -188,6 +190,8 @@ export class CodingArtifactProfile implements WorkflowArtifactProfile { cycle, submittedAt: new Date().toISOString(), }; + // comment_urls is review metadata, not a gate field, so it travels in the + // raw payload (messageData) rather than committedData. const rawCommentUrls = messageData?.comment_urls; if (Array.isArray(rawCommentUrls) && rawCommentUrls.every((u) => typeof u === 'string')) { artifactData.comment_urls = rawCommentUrls; From 7735b2954c56d87be631fd591af76c65e034cf32 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Wed, 5 Aug 2026 23:59:15 -0400 Subject: [PATCH 09/16] =?UTF-8?q?test(space):=20prompt=E2=86=92validator?= =?UTF-8?q?=20parity=20regression=20for=20migrated=20save=5Fartifact=20pay?= =?UTF-8?q?loads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review test-gap on #2345: CI stayed green while the outcome `decision` sites omitted data.recommendation because nothing exercised the path from prompt text to the real save_artifact validator. Add a test that runs the exact payloads emitted by the migrated coding-workflow prompts (dispatcher decision, QA PR link + decision, end-node REC decision, per-attempt merge-conflict notes, post-merge audit link) through the real handler, asserting success:true and that two conflict attempts persist as distinct rows. --- .../5-space/agent/node-agent-tools.test.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index 171d832371..2ff65e36c5 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -1084,6 +1084,96 @@ describe('node-agent-tools: save_artifact', () => { expect(data.success).toBe(false); expect(data.error).toContain('shape'); }); + + // ── Prompt → validator parity ─────────────────────────────────────────── + // Runs the EXACT save_artifact payloads emitted by the migrated coding-workflow + // prompts (built-in-workflows, post-approval-merge-template) and the end-node + // Runtime Execution Contract through the real handler. CI was green while the + // `decision` sites omitted `data.recommendation` only because nothing exercised + // prompt→validator; this test is the regression guard for that class of bug. + + test('every migrated prompt payload is accepted and persists', async () => { + const handlers = createNodeAgentToolHandlers(makeConfig(ctx)); + const run = async (payload: Parameters[0]) => { + const parsed = JSON.parse((await handlers.save_artifact(payload)).content[0].text); + expect(parsed.success).toBe(true); + return parsed; + }; + + // Dispatcher (built-in-workflows) — terminal outcome decision. + await run({ + shape: 'decision', + summary: 'Created 2 tasks from plan: foo, bar', + data: { + recommendation: 'dispatched', + created_task_ids: ['t1', 't2'], + stack_prefix: 'plan-x', + stack_branches: ['plan/x/foo', 'plan/x/bar'], + }, + }); + + // Fullstack QA all-green (built-in-workflows) — PR link + terminal outcome. + await run({ shape: 'link', kind: 'pr', data: { url: 'https://github.com/o/r/pull/9' } }); + await run({ + shape: 'decision', + summary: 'QA passed', + data: { + recommendation: 'pass', + test_output: 'ok', + ui_changed: true, + dev_server_started: true, + browser_validation: 'exercised login flow', + }, + }); + + // End-node Runtime Execution Contract (task-agent-manager) — both branches + // carry data.recommendation. + await run({ + shape: 'decision', + summary: 'Done', + data: { recommendation: 'completed' }, + }); + + // Merge-conflict attempt (post-approval-merge-template) — per-attempt note. + await run({ + shape: 'note', + kind: 'merge_conflict', + key: 'attempt-0', + summary: 'Merge conflict attempt 0 on PR https://github.com/o/r/pull/9', + data: { + pr_url: 'https://github.com/o/r/pull/9', + base_branch: 'dev', + approved_head_oid: 'abc', + conflicting_files: ['a.ts'], + attempt: 0, + }, + }); + await run({ + shape: 'note', + kind: 'merge_conflict', + key: 'attempt-1', + summary: 'Merge conflict attempt 1 on PR https://github.com/o/r/pull/9', + data: { pr_url: 'https://github.com/o/r/pull/9', attempt: 1 }, + }); + + // Post-merge audit (post-approval-merge-template) — link kind:'merge'. + await run({ + shape: 'link', + kind: 'merge', + data: { + url: 'https://github.com/o/r/pull/9', + merged_at: '2026-01-01', + approval_source: 'human', + }, + }); + + // Per-attempt conflict notes must persist as DISTINCT rows (not overwrite). + const notes = ctx.artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'note' }); + expect(notes.map((n) => n.artifactKey).sort()).toEqual([ + 'merge_conflict:attempt-0', + 'merge_conflict:attempt-1', + ]); + }); }); // --------------------------------------------------------------------------- From 28dc5cfe8b1b612a762bd3bf906876393f67bb17 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 00:19:44 -0400 Subject: [PATCH 10/16] fix(space): derive CodingArtifactProfile db type from repo constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI typecheck (round 4) flagged CodingArtifactProfile's `db: BunDatabase` field as bun:sqlite.Database not assignable to the sqlite-node-flavored Database the repositories expect — a Bun type-resolution drift that only surfaces in CI, not locally. Rather than import `Database` from `bun:sqlite` directly, derive the field type from the repository constructor (`ConstructorParameters[0]`) so it is structurally identical to what every other repo caller passes and what `getDatabase()` returns under any Bun resolution — the same type the passing `new GateDataRepository(deps.db.getDatabase())` wiring already uses. --- .../lib/space/workflows/coding-artifact-profile.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts index 9439958ba5..81db2aaf2d 100644 --- a/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts +++ b/packages/daemon/src/lib/space/workflows/coding-artifact-profile.ts @@ -17,7 +17,6 @@ * each time the review-posted-gate receives a `review_url`. */ -import type { Database as BunDatabase } from 'bun:sqlite'; import { deriveArtifactKey } from '@hyperneo/shared'; import { Logger } from '../../logger'; import type { GateDataCommittedEvent, WorkflowArtifactProfile } from '../runtime/artifact-profile'; @@ -30,8 +29,17 @@ const log = new Logger('coding-artifact-profile'); /** Gate id that records a multi-round review decision per cycle. */ const REVIEW_POSTED_GATE = 'review-posted-gate'; +/** + * The SQLite database type this profile consumes. Derived from the repository + * constructor rather than imported directly from `bun:sqlite` so it is + * structurally identical to what every other repo caller passes (and what + * `getDatabase()` returns) under any Bun type resolution — avoiding a + * bun:sqlite ↔ node:sqlite type-surface mismatch that only surfaces in CI. + */ +type ArtifactDb = ConstructorParameters[0]; + export interface CodingArtifactProfileConfig { - db: BunDatabase; + db: ArtifactDb; artifactRepo?: WorkflowRunArtifactRepository; /** Optional shared gate-data repo; created from `db` when omitted. */ gateDataRepo?: GateDataRepository; @@ -51,7 +59,7 @@ function legacyPrUrl(data: Record | undefined): string { } export class CodingArtifactProfile implements WorkflowArtifactProfile { - private readonly db: BunDatabase; + private readonly db: ArtifactDb; private readonly artifactRepo?: WorkflowRunArtifactRepository; private readonly sharedGateDataRepo?: GateDataRepository; From 9cfb95b66de8246ae516c941230b91a3d8b7f194 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 00:20:24 -0400 Subject: [PATCH 11/16] fix(space): key QA failure notes per cycle so they don't overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review P2 on #2345: the Fullstack QA failure note used `shape: "note", kind: "qa"` with no key, so it keyed 'current' and each failure cycle erased the prior cycle's repro evidence — the same overwrite class as the merge-conflict note fixed last round. Add `key: "cycle-"` ( = QA round, 1-based). Extend the prompt→validator parity test to cover two QA failure cycles persisting as distinct rows alongside the merge-conflict attempts. --- .../lib/space/workflows/built-in-workflows.ts | 2 +- .../5-space/agent/node-agent-tools.test.ts | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index f118142cf6..259e2b6e22 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1256,7 +1256,7 @@ export const FULLSTACK_QA_LOOP_WORKFLOW: SpaceWorkflow = { '5. If `ui_changed` is true, start HyperNeo with `make dev PORT= DB_PATH=/tmp/hyperneo-qa-.db` and exercise the changed flow in a browser (golden path, relevant edge cases, nearby regressions)\n' + '6. Validate CI and mergeability\n' + '7. If fail: send detailed failures and repro steps to Coding, then call ' + - '`save_artifact({ shape: "note", kind: "qa", summary: "QA failed: ..." })` to record the audit entry (a note, never a terminal decision). Do ' + + '`save_artifact({ shape: "note", kind: "qa", key: "cycle-", summary: "QA failed (cycle ): ..." })` to record the audit entry — a note, never a terminal decision, and keyed per cycle ( = this QA round, 1-based) so each failure cycle keeps its own repro evidence instead of overwriting the last. Do ' + 'NOT call `approve_task` or `submit_for_approval` — both are TERMINAL and ' + 'carry the same approval semantic. Leave the workflow open for the next ' + 'Coding cycle.\n' + diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index 2ff65e36c5..156565e0d0 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -1167,11 +1167,28 @@ describe('node-agent-tools: save_artifact', () => { }, }); - // Per-attempt conflict notes must persist as DISTINCT rows (not overwrite). + // Fullstack QA failure (built-in-workflows) — per-cycle note so each failure + // cycle keeps its own repro evidence. + await run({ + shape: 'note', + kind: 'qa', + key: 'cycle-1', + summary: 'QA failed (cycle 1): login redirect broken', + }); + await run({ + shape: 'note', + kind: 'qa', + key: 'cycle-2', + summary: 'QA failed (cycle 2): flaky test on retry', + }); + + // Per-attempt / per-cycle notes must persist as DISTINCT rows (not overwrite). const notes = ctx.artifactRepo.listByRun(ctx.workflowRunId, { artifactType: 'note' }); expect(notes.map((n) => n.artifactKey).sort()).toEqual([ 'merge_conflict:attempt-0', 'merge_conflict:attempt-1', + 'qa:cycle-1', + 'qa:cycle-2', ]); }); }); From def702b4b1b8d332e542f8b58df60a1491b2d639 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 00:49:11 -0400 Subject: [PATCH 12/16] fix(space): restamp legacy save_artifact type-API prompts to shapes on upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review P1 on #2345: dev shipped seeded built-in prompts on the legacy save_artifact({ type: "result", append: true, ... }) API. This PR rewrote them to shape:, but the restamp registry gained no type->shape entry, and the cutover rewrote whole call sites (even expanding some one-call steps into two) that are fragile as substring pairs. So on upgrade, existing seeded spaces kept directing agents to call save_artifact with type:, which the new schema rejects for missing shape — terminal artifacts never persisted and post-approval merge could not resolve the PR. Swap any persisted built-in prompt still using the legacy freeform-type API to the current template during restamp (such a prompt is broken regardless, so restamping is the correct fix). Add a regression test that seeds a stale type:"result" prompt and asserts it swaps to shape: on merge. --- .../lib/space/workflows/built-in-workflows.ts | 28 ++++++++++- .../workflow/built-in-workflows.test.ts | 47 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index 259e2b6e22..2b3f7b39f9 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1983,8 +1983,32 @@ function patchKnownBuiltInPromptDrift { ); }); + test('re-stamp swaps a legacy save_artifact({ type: ... }) prompt to the shape API', () => { + // The type→shape cutover rewrote whole call sites (and expanded some into + // two-call steps), which is fragile to capture as exact substring pairs. + // A persisted built-in prompt still on the legacy freeform-type API is + // rejected by the new schema, so restamp swaps it to the current template. + seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + const workflow = manager + .listWorkflows(SPACE_ID) + .find((w) => w.name === FULLSTACK_QA_LOOP_WORKFLOW.name)!; + const qaNode = workflow.nodes.find((n) => n.name === 'QA')!; + const templatePrompt = FULLSTACK_QA_LOOP_WORKFLOW.nodes.find((n) => n.name === 'QA')!.agents[0] + .customPrompt!.value; + // Simulate a dev-era persisted prompt: a current shape call replaced by the + // legacy `type: "result"` API the new schema rejects. + const stalePrompt = templatePrompt.replace( + 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } })', + 'save_artifact({ type: "result", data: { pr_url: "" } })' + ); + expect(stalePrompt).not.toBe(templatePrompt); + expect(stalePrompt).toContain('save_artifact({ type: "result"'); + + manager.updateWorkflow(workflow.id, { + nodes: workflow.nodes.map((n) => + n.id !== qaNode.id + ? n + : { + ...n, + agents: n.agents.map((a, i) => + i === 0 ? { ...a, customPrompt: { value: stalePrompt } } : a + ), + } + ), + }); + db.prepare(`UPDATE space_workflows SET template_hash = ? WHERE id = ?`).run( + 'stale-legacy-type-api-hash', + workflow.id + ); + + const result = seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + expect(result.restamped).toContain(FULLSTACK_QA_LOOP_WORKFLOW.name); + + const after = manager.getWorkflow(workflow.id)!; + const afterPrompt = after.nodes.find((n) => n.id === qaNode.id)!.agents[0].customPrompt?.value; + expect(afterPrompt).toBe(templatePrompt); + expect(afterPrompt).not.toContain('save_artifact({ type: "result"'); + }); + test('re-stamp patches exact retired built-in Fullstack reviewer prompt text', () => { seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); const workflow = manager From 8f84b275d3bb6b55c23d59d27ac7ebb47606f1b0 Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 00:50:00 -0400 Subject: [PATCH 13/16] fix(space): key end-node REC outcome decision so it doesn't clobber the slot terminal Review P2 on #2345: the end-node Runtime Execution Contract (injected into every end-node session) wrote an unkeyed decision (recommendation "completed") -> key 'current'. The Fullstack QA end node's slot prompt also writes an unkeyed terminal decision (recommendation "pass" + test_output/ui_changed/browser_validation) -> key 'current'. An agent following both overwrites the QA evidence with the generic "completed" right before approval; the Plan & Decompose dispatcher collides the same way. Give the REC decision a distinct key ("outcome") so the two coexist as separate rows. Updated the prompt-parity test to match. --- packages/daemon/src/lib/space/runtime/task-agent-manager.ts | 4 ++-- .../daemon/tests/unit/5-space/agent/node-agent-tools.test.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts index 47e0487677..26e576f4e5 100644 --- a/packages/daemon/src/lib/space/runtime/task-agent-manager.ts +++ b/packages/daemon/src/lib/space/runtime/task-agent-manager.ts @@ -3111,11 +3111,11 @@ export class TaskAgentManager { if (isEndNode) { if (approveUnlocked) { lines.push( - 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", key: "outcome", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call approve_task({}) as your FINAL action to close the task. The runtime — not your artifact — decides the terminal status via completion actions.' ); } else { lines.push( - 'When your work is complete: (1) call save_artifact({ shape: "decision", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' + 'When your work is complete: (1) call save_artifact({ shape: "decision", key: "outcome", summary: "...", data: { recommendation: "completed" } }) to record the outcome, then (2) call submit_for_approval({ reason: "..." }) as your FINAL action. approve_task is NOT available at this autonomy level; only a human can finalize.' ); } } diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index 156565e0d0..fcd94d236e 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -1127,9 +1127,11 @@ describe('node-agent-tools: save_artifact', () => { }); // End-node Runtime Execution Contract (task-agent-manager) — both branches - // carry data.recommendation. + // carry data.recommendation and a distinct key so the generic outcome + // decision doesn't clobber a slot-prompt terminal decision (key 'current'). await run({ shape: 'decision', + key: 'outcome', summary: 'Done', data: { recommendation: 'completed' }, }); From 9dad0acf76604435fb4389b5f841bb628657599e Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 01:22:42 -0400 Subject: [PATCH 14/16] fix(space): migrate legacy type:"result" prompts via exact restamp pairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 on #2345: the round-6 unconditional swap (any prompt containing save_artifact({ type: ... })) wholesale-replaced persisted prompts with the template, silently discarding operator customizations. Revert that broad check and instead add type→shape patch-variant pairs to BUILT_IN_PROMPT_PATCH_VARIANTS for each rewritten call site (PR link, review-only link, dispatcher stack/short, QA failed note, and the expanded QA all-green two-call step). Only an EXACT retired variant swaps to the current template, so customizations are preserved. Add a regression test for the expanded QA all-green region (the most fragile pair), including a toContain sanity check that the current-text transcription matches the live template. --- .../lib/space/runtime/post-approval-router.ts | 4 +- .../lib/space/workflows/built-in-workflows.ts | 87 +++++++++++++------ .../runtime/post-approval-router.test.ts | 9 +- .../workflow/built-in-workflows.test.ts | 77 ++++++++++++++-- 4 files changed, 138 insertions(+), 39 deletions(-) diff --git a/packages/daemon/src/lib/space/runtime/post-approval-router.ts b/packages/daemon/src/lib/space/runtime/post-approval-router.ts index c6457ed5fc..1b9165aa0c 100644 --- a/packages/daemon/src/lib/space/runtime/post-approval-router.ts +++ b/packages/daemon/src/lib/space/runtime/post-approval-router.ts @@ -200,8 +200,8 @@ const POST_APPROVAL_COMPLETION_INSTRUCTIONS = `task from \`approved\` to \`done\`. If you are blocked and cannot complete the\n` + `work, do NOT call mark_complete — the post-approval node-agent surface has no\n` + `request-human tool, so surface the blocker via send_message(target="space-agent")\n` + - `and save a NON-result artifact describing the block (e.g. type:"blocked"). A\n` + - `"result" artifact would be picked up as the task result on a later mark_complete,\n` + + `and save a NON-result artifact describing the block (e.g. shape:"note", kind:"blocked"). A\n` + + `kindless \`decision\` would be picked up as the task result on a later mark_complete,\n` + `poisoning completion. Then stop.\n\n` + `Do NOT call approve_task; the task has already been approved upstream.`; diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index 2b3f7b39f9..d4df1cb5c3 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1878,6 +1878,56 @@ const RETIRED_CODEX_REACTION_APPROVAL_GUIDANCE = 'only with a warning recorded in your result artifact. Do not close the task ' + 'before codex[bot] has `+1` unless that timeout has elapsed.'; +// type→shape save_artifact API migration. `dev` shipped the seeded built-in +// prompts on the legacy freeform-type API; the shape cutover rewrote each call +// site (and expanded the QA all-green step from one result call into a link + +// decision pair). Each pair is `[currentShapeText, retiredTypeResultText]`; +// `buildRetiredBuiltInPromptValues` reverse-applies them (current→retired) to +// recognize a persisted dev-era prompt and swap it to the current template. Only +// an EXACT retired variant swaps, so operator customizations to surrounding +// prose are preserved (a customized prompt that no longer matches a retired +// variant is left untouched). +const SHAPE_PR_LINK = 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } })'; +const RETIRED_TYPE_RESULT_PR_LINK = 'save_artifact({ type: "result", data: { pr_url: "" } })'; +// Review-only reviewer prompt also changed the trailing prose ("to save a result +// artifact" → "to record the PR"), so its pair carries that tail to stay exact. +const SHAPE_PR_LINK_REVIEW_ONLY = + 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } }) to record the PR'; +const RETIRED_TYPE_RESULT_PR_LINK_REVIEW_ONLY = + 'save_artifact({ type: "result", data: { pr_url: "" } }) to save a result artifact'; +const SHAPE_DECISION_DISPATCHER_STACK = + 'save_artifact({ shape: "decision", summary: "Created N tasks from plan: ", ' + + 'data: { recommendation: "dispatched", created_task_ids: [], stack_prefix: "", ' + + 'stack_branches: ["plan//", "plan//", ...] } })` to record the dispatch outcome'; +const RETIRED_TYPE_RESULT_DISPATCHER_STACK = + 'save_artifact({ type: "result", append: true, summary: "Created N tasks from plan: ", ' + + 'created_task_ids: [], stack_prefix: "", ' + + 'stack_branches: ["plan//", "plan//", ...] })` to record the dispatch audit entry'; +const SHAPE_DECISION_DISPATCHER_SHORT = + 'save_artifact({ shape: "decision", summary: "Dispatched N tasks", data: { recommendation: "dispatched", created_task_ids: [...] } })'; +const RETIRED_TYPE_RESULT_DISPATCHER_SHORT = + 'save_artifact({ type: "result", append: true, created_task_ids: [...] })'; +const SHAPE_NOTE_QA_FAILED = + '`save_artifact({ shape: "note", kind: "qa", key: "cycle-", summary: "QA failed (cycle ): ..." })` to record the audit entry — a note, never a terminal decision, and keyed per cycle ( = this QA round, 1-based) so each failure cycle keeps its own repro evidence instead of overwriting the last. Do '; +const RETIRED_TYPE_RESULT_QA_FAILED = + '`save_artifact({ type: "result", append: true, summary: "QA failed: ..." })` to record the audit entry. Do '; +const SHAPE_QA_ALL_GREEN = + 'a. Record the PR and the terminal QA outcome as two artifacts: ' + + '`save_artifact({ shape: "link", kind: "pr", data: { url: "" } })` ' + + '(the canonical PR record the post-approval merge step resolves as the ' + + 'primary link) and `save_artifact({ shape: "decision", summary, data: { ' + + 'recommendation: "pass", test_output: "", ui_changed: , dev_server_started: , ' + + 'browser_validation: "" } })` (the terminal ' + + 'outcome summary). Top-level keys outside `data` are silently stripped by the ' + + 'tool schema, so nest fields correctly.\n'; +const RETIRED_TYPE_RESULT_QA_ALL_GREEN = + 'a. Call `save_artifact({ type: "result", append: true, summary, data: { ' + + 'pr_url: "", test_output: "", ui_changed: , dev_server_started: , ' + + 'browser_validation: "" } })` to record the audit entry. The ' + + '`pr_url` inside `data` is what `dispatchPostApproval` reads when interpolating `{{pr_url}}` into the ' + + 'merge template — top-level keys outside `data` are silently stripped by the tool schema, so nest it ' + + 'correctly.\n'; + const BUILT_IN_PROMPT_PATCH_VARIANTS = [ [[REVIEW_THREAD_RESOLUTION_GUIDANCE, RETIRED_REVIEW_THREAD_RESOLUTION_GUIDANCE]], [ @@ -1974,6 +2024,15 @@ const BUILT_IN_PROMPT_PATCH_VARIANTS = [ // new paragraph, so removal reconstructs the prior prompt byte-for-byte. [[REVIEWER_POST_APPROVAL_BLOCKER_PARAGRAPH, '']], [[FULLSTACK_QA_POST_APPROVAL_PARAGRAPH, '']], + // type→shape save_artifact API migration (dev→shape cutover). Each swaps a + // persisted legacy type:"result" call site to its shape equivalent; only an + // exact retired variant matches, so customizations are preserved. + [[SHAPE_PR_LINK, RETIRED_TYPE_RESULT_PR_LINK]], + [[SHAPE_PR_LINK_REVIEW_ONLY, RETIRED_TYPE_RESULT_PR_LINK_REVIEW_ONLY]], + [[SHAPE_DECISION_DISPATCHER_STACK, RETIRED_TYPE_RESULT_DISPATCHER_STACK]], + [[SHAPE_DECISION_DISPATCHER_SHORT, RETIRED_TYPE_RESULT_DISPATCHER_SHORT]], + [[SHAPE_NOTE_QA_FAILED, RETIRED_TYPE_RESULT_QA_FAILED]], + [[SHAPE_QA_ALL_GREEN, RETIRED_TYPE_RESULT_QA_ALL_GREEN]], ] as const; function patchKnownBuiltInPromptDrift( @@ -1983,32 +2042,8 @@ function patchKnownBuiltInPromptDrift { // — that tool is not registered on the post-approval node-agent surface, so // referencing it sends the reviewer into an unregistered tool. expect(delegates.spawned[0].kickoffMessage).not.toContain('request_human_input'); - // A blocked-path artifact must be a NON-result type — mark_complete derives - // the task result from the latest result-artifact summary, so a "blocked" - // result artifact would poison a later successful completion. + // A blocked-path artifact must be a NON-terminal shape — mark_complete derives + // the task result from the latest kindless-decision summary, so a "blocked" + // decision would poison a later successful completion. A keyed note is the + // non-terminal form. expect(delegates.spawned[0].kickoffMessage).toMatch(/NON-result artifact/); - expect(delegates.spawned[0].kickoffMessage).toContain('type:"blocked"'); + expect(delegates.spawned[0].kickoffMessage).toContain('shape:"note", kind:"blocked"'); const final = taskRepo.getTask(task.id); expect(final?.postApprovalSessionId).toBe('spawned-session-1'); diff --git a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts index 610835e45c..7b06a43122 100644 --- a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts +++ b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts @@ -2127,11 +2127,12 @@ describe('seedBuiltInWorkflows()', () => { ); }); - test('re-stamp swaps a legacy save_artifact({ type: ... }) prompt to the shape API', () => { - // The type→shape cutover rewrote whole call sites (and expanded some into - // two-call steps), which is fragile to capture as exact substring pairs. - // A persisted built-in prompt still on the legacy freeform-type API is - // rejected by the new schema, so restamp swaps it to the current template. + test('re-stamp swaps a legacy save_artifact({ type: "result" }) call to the shape API', () => { + // dev shipped seeded prompts on the legacy freeform-type API; the shape + // cutover rewrote each call site. The type→shape patch pairs recognize a + // persisted dev-era prompt (an exact retired variant) and swap it to the + // current template, preserving any operator customization that no longer + // matches a retired variant. seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); const workflow = manager .listWorkflows(SPACE_ID) @@ -2139,8 +2140,8 @@ describe('seedBuiltInWorkflows()', () => { const qaNode = workflow.nodes.find((n) => n.name === 'QA')!; const templatePrompt = FULLSTACK_QA_LOOP_WORKFLOW.nodes.find((n) => n.name === 'QA')!.agents[0] .customPrompt!.value; - // Simulate a dev-era persisted prompt: a current shape call replaced by the - // legacy `type: "result"` API the new schema rejects. + // Simulate a dev-era persisted prompt: a current PR-link shape call replaced + // by the legacy `type: "result"` call the new schema rejects. const stalePrompt = templatePrompt.replace( 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } })', 'save_artifact({ type: "result", data: { pr_url: "" } })' @@ -2174,6 +2175,68 @@ describe('seedBuiltInWorkflows()', () => { expect(afterPrompt).not.toContain('save_artifact({ type: "result"'); }); + test('re-stamp swaps the expanded QA all-green step (two shape calls → one legacy result)', () => { + // The QA all-green step was one legacy `type: "result"` call in dev and is + // now a `link kind:"pr"` + `decision` pair. Verify the whole-region patch + // pair recognizes the dev-era block and restores the current two-call form. + seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + const workflow = manager + .listWorkflows(SPACE_ID) + .find((w) => w.name === FULLSTACK_QA_LOOP_WORKFLOW.name)!; + const qaNode = workflow.nodes.find((n) => n.name === 'QA')!; + const templatePrompt = FULLSTACK_QA_LOOP_WORKFLOW.nodes.find((n) => n.name === 'QA')!.agents[0] + .customPrompt!.value; + const SHAPE_QA_ALL_GREEN = + 'a. Record the PR and the terminal QA outcome as two artifacts: ' + + '`save_artifact({ shape: "link", kind: "pr", data: { url: "" } })` ' + + '(the canonical PR record the post-approval merge step resolves as the ' + + 'primary link) and `save_artifact({ shape: "decision", summary, data: { ' + + 'recommendation: "pass", test_output: "", ui_changed: , dev_server_started: , ' + + 'browser_validation: "" } })` (the terminal ' + + 'outcome summary). Top-level keys outside `data` are silently stripped by the ' + + 'tool schema, so nest fields correctly.\n'; + const RETIRED_TYPE_RESULT_QA_ALL_GREEN = + 'a. Call `save_artifact({ type: "result", append: true, summary, data: { ' + + 'pr_url: "", test_output: "", ui_changed: , dev_server_started: , ' + + 'browser_validation: "" } })` to record the audit entry. The ' + + '`pr_url` inside `data` is what `dispatchPostApproval` reads when interpolating `{{pr_url}}` into the ' + + 'merge template — top-level keys outside `data` are silently stripped by the tool schema, so nest it ' + + 'correctly.\n'; + // Sanity: the current-text transcription actually matches the live template. + expect(templatePrompt).toContain(SHAPE_QA_ALL_GREEN); + const stalePrompt = templatePrompt.replace( + SHAPE_QA_ALL_GREEN, + RETIRED_TYPE_RESULT_QA_ALL_GREEN + ); + expect(stalePrompt).not.toBe(templatePrompt); + expect(stalePrompt).toContain('save_artifact({ type: "result"'); + + manager.updateWorkflow(workflow.id, { + nodes: workflow.nodes.map((n) => + n.id !== qaNode.id + ? n + : { + ...n, + agents: n.agents.map((a, i) => + i === 0 ? { ...a, customPrompt: { value: stalePrompt } } : a + ), + } + ), + }); + db.prepare(`UPDATE space_workflows SET template_hash = ? WHERE id = ?`).run( + 'stale-qa-all-green-hash', + workflow.id + ); + + const result = seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + expect(result.restamped).toContain(FULLSTACK_QA_LOOP_WORKFLOW.name); + + const after = manager.getWorkflow(workflow.id)!; + const afterPrompt = after.nodes.find((n) => n.id === qaNode.id)!.agents[0].customPrompt?.value; + expect(afterPrompt).toBe(templatePrompt); + expect(afterPrompt).toContain(SHAPE_QA_ALL_GREEN); + }); + test('re-stamp patches exact retired built-in Fullstack reviewer prompt text', () => { seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); const workflow = manager From e23eff3bb3f7ecba73a59ca1915d948ccf8f906a Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Thu, 6 Aug 2026 01:36:35 -0400 Subject: [PATCH 15/16] fix(space): restamp the Coding/Research reviewer preceding sentence + PR-link call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 on #2345: the type→shape cutover rewrote BOTH the "Use save_artifact every cycle…" sentence (Nest pr_url → record the PR as a link) AND the PR-link call in the Coding and Research reviewer prompts. Reversing the call alone leaves the new sentence, so the generated retired variant did not match the real dev-era prompt and the upgrade left these two workflows on the rejected legacy type:"result" API. Add a patch-variant set that reverses the sentence and the call together, and a regression test that swaps both and asserts the restamp. --- .../lib/space/workflows/built-in-workflows.ts | 16 ++++++ .../workflow/built-in-workflows.test.ts | 53 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts index d4df1cb5c3..00da309e44 100644 --- a/packages/daemon/src/lib/space/workflows/built-in-workflows.ts +++ b/packages/daemon/src/lib/space/workflows/built-in-workflows.ts @@ -1889,6 +1889,15 @@ const RETIRED_CODEX_REACTION_APPROVAL_GUIDANCE = // variant is left untouched). const SHAPE_PR_LINK = 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } })'; const RETIRED_TYPE_RESULT_PR_LINK = 'save_artifact({ type: "result", data: { pr_url: "" } })'; +// Coding + Research reviewer prompts also rewrote the sentence preceding the +// PR-link call ("Nest pr_url inside artifact data…" → "record the PR as a +// link…"). Reversing the call alone leaves the new sentence, so the generated +// variant would not match the real dev prompt — pair the sentence with the call +// so the whole region reconstructs the dev-era reviewer prompt. +const SHAPE_PR_EVERY_CYCLE = + 'Use save_artifact every cycle to record the PR as a `link` so post-approval dispatch can resolve it.\n\n'; +const RETIRED_TYPE_RESULT_EVERY_CYCLE = + 'Use save_artifact every cycle. Nest pr_url inside artifact data for post-approval dispatch.\n\n'; // Review-only reviewer prompt also changed the trailing prose ("to save a result // artifact" → "to record the PR"), so its pair carries that tail to stay exact. const SHAPE_PR_LINK_REVIEW_ONLY = @@ -2028,6 +2037,13 @@ const BUILT_IN_PROMPT_PATCH_VARIANTS = [ // persisted legacy type:"result" call site to its shape equivalent; only an // exact retired variant matches, so customizations are preserved. [[SHAPE_PR_LINK, RETIRED_TYPE_RESULT_PR_LINK]], + // Coding + Research reviewer prompts rewrote BOTH the preceding "every cycle" + // sentence and the PR-link call, so both must reverse together to reconstruct + // the dev-era prompt. + [ + [SHAPE_PR_EVERY_CYCLE, RETIRED_TYPE_RESULT_EVERY_CYCLE], + [SHAPE_PR_LINK, RETIRED_TYPE_RESULT_PR_LINK], + ], [[SHAPE_PR_LINK_REVIEW_ONLY, RETIRED_TYPE_RESULT_PR_LINK_REVIEW_ONLY]], [[SHAPE_DECISION_DISPATCHER_STACK, RETIRED_TYPE_RESULT_DISPATCHER_STACK]], [[SHAPE_DECISION_DISPATCHER_SHORT, RETIRED_TYPE_RESULT_DISPATCHER_SHORT]], diff --git a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts index 7b06a43122..9558d5d969 100644 --- a/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts +++ b/packages/daemon/tests/unit/5-space/workflow/built-in-workflows.test.ts @@ -2237,6 +2237,59 @@ describe('seedBuiltInWorkflows()', () => { expect(afterPrompt).toContain(SHAPE_QA_ALL_GREEN); }); + test('re-stamp swaps a Coding reviewer prompt whose preceding sentence AND call both changed', () => { + // The type→shape cutover rewrote both the "Use save_artifact every cycle…" + // sentence and the PR-link call in the Coding (and Research) reviewer + // prompts. Reversing the call alone leaves the new sentence, so the + // generated variant would not match the real dev prompt; the sentence and + // call must reverse together. + seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + const workflow = manager.listWorkflows(SPACE_ID).find((w) => w.name === CODING_WORKFLOW.name)!; + const reviewNode = workflow.nodes.find((n) => n.name === 'Review')!; + const templatePrompt = CODING_WORKFLOW.nodes.find((n) => n.name === 'Review')!.agents[0] + .customPrompt!.value; + const SHAPE_PR_EVERY_CYCLE = + 'Use save_artifact every cycle to record the PR as a `link` so post-approval dispatch can resolve it.\n\n'; + const RETIRED_EVERY_CYCLE = + 'Use save_artifact every cycle. Nest pr_url inside artifact data for post-approval dispatch.\n\n'; + const SHAPE_PR_LINK = 'save_artifact({ shape: "link", kind: "pr", data: { url: "" } })'; + const RETIRED_PR_LINK = 'save_artifact({ type: "result", data: { pr_url: "" } })'; + expect(templatePrompt).toContain(SHAPE_PR_EVERY_CYCLE); + expect(templatePrompt).toContain(SHAPE_PR_LINK); + const stalePrompt = templatePrompt + .replace(SHAPE_PR_EVERY_CYCLE, RETIRED_EVERY_CYCLE) + .replace(SHAPE_PR_LINK, RETIRED_PR_LINK); + expect(stalePrompt).not.toBe(templatePrompt); + expect(stalePrompt).toContain('save_artifact({ type: "result"'); + + manager.updateWorkflow(workflow.id, { + nodes: workflow.nodes.map((n) => + n.id !== reviewNode.id + ? n + : { + ...n, + agents: n.agents.map((a, i) => + i === 0 ? { ...a, customPrompt: { value: stalePrompt } } : a + ), + } + ), + }); + db.prepare(`UPDATE space_workflows SET template_hash = ? WHERE id = ?`).run( + 'stale-coding-reviewer-hash', + workflow.id + ); + + const result = seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); + expect(result.restamped).toContain(CODING_WORKFLOW.name); + + const after = manager.getWorkflow(workflow.id)!; + const afterPrompt = after.nodes.find((n) => n.id === reviewNode.id)!.agents[0].customPrompt + ?.value; + expect(afterPrompt).toBe(templatePrompt); + expect(afterPrompt).toContain(SHAPE_PR_EVERY_CYCLE); + expect(afterPrompt).not.toContain('save_artifact({ type: "result"'); + }); + test('re-stamp patches exact retired built-in Fullstack reviewer prompt text', () => { seedBuiltInWorkflows(SPACE_ID, manager, resolveAgentId); const workflow = manager From ea269be89c9faac3b383404af5dcd4560927eaeb Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Fri, 7 Aug 2026 18:58:28 -0400 Subject: [PATCH 16/16] fix(test): pass artifactProfile to evaluateTerminalGateFeatures post-rebase After rebasing onto dev (#2362 added a terminal-validator-gate test that calls evaluateTerminalGateFeatures with ctx.artifactRepo as the 7th arg), the call no longer matched my refactored signature (7th param is artifactProfile, not the repo). Pass ctx.artifactProfile like the sibling calls in this file. --- .../daemon/tests/unit/5-space/agent/node-agent-tools.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts index fcd94d236e..305dfbc438 100644 --- a/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts +++ b/packages/daemon/tests/unit/5-space/agent/node-agent-tools.test.ts @@ -3522,7 +3522,7 @@ describe('node-agent-tools: async gate evaluation', () => { mockExecutor, { workspacePath: '/tmp', runId: ctx.workflowRunId, gateId: gate.id }, 'node-coder', - ctx.artifactRepo + ctx.artifactProfile ); expect(result).not.toBeNull(); const data = JSON.parse(result!.content[0].text);