From 8333cf55ec6185580874955286f60f013f8adb75 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Fri, 15 May 2026 17:19:52 +0200 Subject: [PATCH] fix(auto-fix): don't repair non-executable --spec-file paths as workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `ricky --spec-file foo.md` fails at the intake stage (e.g. unresolved clarification questions), the auto-fix loop used to fall back to `request.specPath` in `resolveArtifactPath` and hand the source markdown spec to the workflow repairer. The repaired markdown was then re-fed as `source: 'workflow-artifact'`, losing the CLI spec-file routing in `toRawSpecPayload`. The next attempt parsed the spec body as natural language; failure-vocabulary keywords ("logs", "stderr", "evidence") misrouted it to debug, which failed with the misleading "Debug routing requires failed-run evidence" message — repeated up to maxAttempts (7×) before erroring. Gate the `request.specPath` fallback on `isExecutableWorkflowPath` so only paths that actually name a workflow (`workflows/**/*.{ts,js}` or `*.workflow.{ts,js,yaml,yml}`) qualify. When generation fails before producing an artifact and no executable path exists, auto-fix bails on attempt 1 with the original failure reason instead of looping. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/local/auto-fix-loop.test.ts | 50 +++++++++++++++++++++++++++++++++ src/local/auto-fix-loop.ts | 11 +++++++- src/local/entrypoint.ts | 2 +- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/local/auto-fix-loop.test.ts b/src/local/auto-fix-loop.test.ts index 3c98edf9..3a341d59 100644 --- a/src/local/auto-fix-loop.test.ts +++ b/src/local/auto-fix-loop.test.ts @@ -1020,6 +1020,40 @@ describe('runWithAutoFix', () => { } }); + it('does not treat a non-executable --spec-file path as a workflow artifact to repair', async () => { + // Regression: when `ricky --spec-file docs/foo.md` failed at the intake + // stage (e.g. unresolved clarification questions), `resolveArtifactPath` + // used to fall back to `request.specPath`, which pointed at the source + // spec markdown. Auto-fix then handed the markdown to the workflow + // repairer, re-fed the "repaired" content as source=workflow-artifact, + // and looped 7× while the natural-language intent detector misrouted the + // spec body to debug. With no executable workflow path available, there + // is nothing to repair and auto-fix should bail on attempt 1. + const specFileRequest: LocalInvocationRequest = { + ...baseRequest, + spec: '# Some markdown spec\n\nUnresolved question?', + specPath: 'docs/some-spec.md', + }; + const runSingleAttempt = vi.fn().mockResolvedValueOnce(generationOnlyFailureResponse()); + const workflowRepairer = vi.fn().mockResolvedValue(workflowRepair('should-not-run')); + const artifactWriter = vi.fn().mockResolvedValue(undefined); + + const result = await runWithAutoFix(specFileRequest, { + maxAttempts: 7, + runSingleAttempt, + classifyFailure: fakeClassification, + debugWorkflowRun: directDebugger, + workflowRepairer, + artifactWriter, + }); + + expect(runSingleAttempt).toHaveBeenCalledTimes(1); + expect(workflowRepairer).not.toHaveBeenCalled(); + expect(artifactWriter).not.toHaveBeenCalled(); + expect(result.ok).toBe(false); + expect(result.auto_fix?.attempts).toHaveLength(1); + }); + }); describe('isSyntheticStageId', () => { @@ -1061,6 +1095,22 @@ function successResponse(runId: string): LocalResponse { }; } +function generationOnlyFailureResponse(): LocalResponse { + return { + ok: false, + artifacts: [], + logs: [], + warnings: ['routing: Spec has unresolved workflow authoring questions'], + nextActions: ['Clarify the local workflow request and retry.'], + generation: { + stage: 'generate', + status: 'needs_clarification', + error: 'routing: Spec has unresolved workflow authoring questions', + }, + exitCode: 2, + }; +} + function blockerResponse(code: LocalClassifiedBlocker['code'], runId: string | undefined, failedStep: string): LocalResponse { const blocker: LocalClassifiedBlocker = { code, diff --git a/src/local/auto-fix-loop.ts b/src/local/auto-fix-loop.ts index ca7c1206..0e6d51b0 100644 --- a/src/local/auto-fix-loop.ts +++ b/src/local/auto-fix-loop.ts @@ -7,6 +7,7 @@ import ts from 'typescript'; import type { LocalInvocationRequest } from './request-normalizer.js'; import type { LocalClassifiedBlocker, LocalExecutionEvidence, LocalResponse } from './entrypoint.js'; +import { isExecutableWorkflowPath } from './entrypoint.js'; import { classifyFailure as defaultClassifyFailure } from '../runtime/failure/classifier.js'; import type { FailureClassification } from '../runtime/failure/types.js'; import { debugWorkflowRun as defaultDebugWorkflowRun } from '../product/specialists/debugger/debugger.js'; @@ -1586,12 +1587,20 @@ async function resolveWorkflowRepairTarget( } function resolveArtifactPath(request: LocalInvocationRequest, response: LocalResponse): string | undefined { + // `request.specPath` is the LAST resort because for CLI invocations like + // `--spec-file docs/foo.md` it points at the source spec (markdown, etc.), + // not an executable workflow. Treating that as the "artifact to repair" + // makes auto-fix hand the markdown spec to the workflow repairer and then + // re-feed the result as source=workflow-artifact, which loses the original + // CLI intent and routes the spec body through natural-language intent + // detection — where failure-vocabulary keywords misroute it to debug. + // Only fall back to specPath when it actually names an executable workflow. return ( response.execution?.execution.workflow_file ?? response.execution?.execution.artifact_path ?? response.generation?.artifact?.path ?? response.artifacts[0]?.path ?? - request.specPath + (request.specPath && isExecutableWorkflowPath(request.specPath) ? request.specPath : undefined) ); } diff --git a/src/local/entrypoint.ts b/src/local/entrypoint.ts index b4348c31..9fe20c2a 100644 --- a/src/local/entrypoint.ts +++ b/src/local/entrypoint.ts @@ -1700,7 +1700,7 @@ function artifactPathOverrideFor(request: LocalInvocationRequest): string | unde return typeof candidate === 'string' && candidate.trim() ? candidate : undefined; } -function isExecutableWorkflowPath(path: string): boolean { +export function isExecutableWorkflowPath(path: string): boolean { return /(?:^|\/)workflows\/.+\.(?:ts|js)$|\.workflow\.(?:ts|js|yaml|yml)$/i.test(path); }