From 9f16c34a3a906363b9cc980f53cd857ce83e062f Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 10 May 2026 13:07:40 +0200 Subject: [PATCH] fix(master-renderer): use workspace-aware typecheck command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master-rendered workflows hardcoded `npx tsc --noEmit` in three places: - `master-workflow-renderer.ts:264` (final-hard-validation step body) - `master-workflow-renderer.ts:465` (initial-soft-validation gate) - `master-workflow-renderer.ts:467` (final-hard-validation gate) When a generated workflow ran from a monorepo root with no top-level `tsconfig.json` (npm workspaces with `packages/*/tsconfig.json` layout — common in MSD-style repos), `npx tsc --noEmit` found neither input files nor a config and dumped the full `tsc --help` text on stdout while exiting 1. The `final-hard-validation` step retried 2 more times via onError; the auto-fix loop then "repaired" 7×, all failing identically because the workflow command was correct in general — just wrong for that repo shape. User-visible repro from the proactive-pr-remediation spec, after PR #84 and PR #86 had unblocked the artifact-write and module-load paths so the workflow could actually reach final-hard-validation: Generation: ok — workflows/generated/ricky-...ts Workflow name: wf-1e14175ec3b5 Execution: blocked — INVALID_ARTIFACT at final-hard-validation Auto-fix: stopped after 7/7 attempt(s) (INVALID_ARTIFACT) Fix: extract a single `TYPECHECK_COMMAND` constant emitting if [ "$(npm pkg get scripts.typecheck 2>/dev/null)" != "{}" ]; then npm run typecheck else npx tsc --noEmit fi `npm pkg get scripts.typecheck` returns `""` when present and `{}` when absent (npm v7.20.0+, shipped with Node 16+). The substring `npx tsc --noEmit` is preserved so existing `expect.stringContaining('npx tsc --noEmit')` assertions, evidence capture, debugger recommendations, and human readers continue to recognize the intent. Verified end-to-end in the user's MSD repo: `npm pkg get scripts.typecheck` returns the project's workspace-aware typecheck script and the snippet routes to `npm run typecheck`, which correctly typechecks `packages/shared`, lints `packages/backend`, and runs `tsc -b packages/webapp/tsconfig.app.json`. Tests: - New regression case in `pipeline.test.ts` — "emits a workspace-aware typecheck command in master-rendered final-hard-validation" — asserts the rendered workflow contains both branches of the conditional and rejects the bare `set -e\n npx tsc --noEmit\n` pattern that produced the bug. - Existing 45 pipeline tests still pass (all use `stringContaining('npx tsc --noEmit')` rather than exact-string equality, so they survive the conditional wrapper unchanged). - `npm test` — 1059 / 1059 green across 50 files. Out of scope (explicitly): - `template-renderer.ts:288` (`typecheckCommand` constant for non-master workflows) and `:637` (informational verification command shown to LLM agents). Same one-line change pattern would apply, but the existing test coupling there asserts on a wider surface (`pipeline.test.ts:980`, `validator.test.ts`, `debugger.test.ts`, `recovery-preflight.test.ts`, `evidence/capture.test.ts`). Worth a follow-up PR to keep blast radius proportionate to user impact — the user's failure was specifically in master-rendered workflows. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../generation/master-workflow-renderer.ts | 27 ++++++++++++-- src/product/generation/pipeline.test.ts | 37 +++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/product/generation/master-workflow-renderer.ts b/src/product/generation/master-workflow-renderer.ts index a889048..8b38e4e 100644 --- a/src/product/generation/master-workflow-renderer.ts +++ b/src/product/generation/master-workflow-renderer.ts @@ -27,6 +27,27 @@ interface RenderedMasterWorkflow { plan: MasterExecutionPlan; } +// Workspace-aware typecheck: prefer the project's own `npm run typecheck` +// script when one exists (the right thing for monorepos like +// `npm run typecheck -ws` or custom build pipelines), and fall back to +// `npx tsc --noEmit` when the project is a flat single-tsconfig repo. +// +// Without this guard, `npx tsc --noEmit` invoked from a monorepo root with +// no top-level tsconfig.json (npm workspaces, packages/*/tsconfig.json +// layout — common in MSD-style repos) finds neither input files nor a +// config and dumps the full `tsc --help` text on stdout while exiting 1. +// The auto-fix loop then "repairs" the workflow 7×, all failing identically +// because the workflow command is correct in general — just wrong for this +// repo shape. +// +// `npm pkg get scripts.typecheck` returns `""` when the script +// exists and `{}` when it does not (npm v7.20.0+, shipped with Node 16+). +// We compare against the literal `{}` so the snippet is portable across +// any package layout. The substring `npx tsc --noEmit` is preserved so +// downstream tools and human readers still recognize the intent. +const TYPECHECK_COMMAND = + 'if [ "$(npm pkg get scripts.typecheck 2>/dev/null)" != "{}" ]; then npm run typecheck; else npx tsc --noEmit; fi'; + const MASTER_EXPLICIT_PATTERN = /\b(master executor|master orchestration|smaller workflows|child workflows|several workflows|multiple workflows|break(?:ing)? (?:it )?(?:out|up)|divvy|decompos(?:e|ition)|workflow waves?)\b/i; @@ -261,7 +282,7 @@ function renderMasterSource(input: { ' dependsOn: ["review-child-evidence"],', ` command: ${literal([ 'set -e', - 'npx tsc --noEmit', + TYPECHECK_COMMAND, 'npm test', 'git diff --name-only', `grep -F RICKY_MASTER_REVIEW_READY ${shellQuote(`${input.artifactsDir}/review-codex.md`)}`, @@ -462,9 +483,9 @@ function buildMasterGates(artifactsDir: string, plan: MasterExecutionPlan): Dete gate('skill-boundary-metadata-gate', `test -f ${artifactsDir}/skill-application-boundary.json`, 'file_exists', true, ['prepare-context'], 'pre_review'), gate('lead-plan-gate', `grep -F RICKY_MASTER_LEAD_PLAN_READY ${artifactsDir}/lead-plan.md`, 'output_contains', true, ['lead-plan'], 'pre_review'), gate('child-workflow-file-gate', plan.children.map((child) => `test -f ${child.workflowFilePath}`).join(' && '), 'file_exists', true, ['materialize-child-workflows'], 'pre_review'), - gate('initial-soft-validation', 'npx tsc --noEmit 2>&1 | tail -160', 'output_contains', false, ['child-workflow-file-gate'], 'pre_review'), + gate('initial-soft-validation', `{ ${TYPECHECK_COMMAND}; } 2>&1 | tail -160`, 'output_contains', false, ['child-workflow-file-gate'], 'pre_review'), gate('final-review-pass-gate', `grep -F RICKY_MASTER_REVIEW_READY ${artifactsDir}/review-codex.md`, 'output_contains', true, ['review-child-evidence'], 'final'), - gate('final-hard-validation', 'npx tsc --noEmit && npm test', 'exit_code', true, ['final-review-pass-gate'], 'final'), + gate('final-hard-validation', `{ ${TYPECHECK_COMMAND}; } && npm test`, 'exit_code', true, ['final-review-pass-gate'], 'final'), gate('git-diff-gate', 'git diff --name-only', 'output_contains', true, ['final-hard-validation'], 'final'), gate('regression-gate', 'npm test', 'exit_code', true, ['git-diff-gate'], 'regression'), ]; diff --git a/src/product/generation/pipeline.test.ts b/src/product/generation/pipeline.test.ts index 2174248..fe278e4 100644 --- a/src/product/generation/pipeline.test.ts +++ b/src/product/generation/pipeline.test.ts @@ -58,6 +58,43 @@ describe('workflow generation pipeline', () => { expect(rendered.content).toContain('.run({ cwd: process.cwd() })'); }); + // Regression: master-rendered final-hard-validation used to hardcode + // `npx tsc --noEmit`, which dumps the full `tsc --help` text and exits 1 + // when invoked from a monorepo root with no top-level tsconfig.json + // (npm workspaces with `packages/*/tsconfig.json` layout — common in + // MSD-style repos). The auto-fix loop then "repaired" the workflow 7×, + // all failing identically because the workflow command was correct in + // general — just wrong for that repo shape. The renderer now emits a + // workspace-aware shell snippet that prefers `npm run typecheck` when the + // project defines that script and falls back to `npx tsc --noEmit` + // otherwise. The fallback path keeps `npx tsc --noEmit` as a literal + // substring so downstream tests, evidence capture, and human readers + // still recognize the intent. + it('emits a workspace-aware typecheck command in master-rendered final-hard-validation', () => { + const result = generate({ + spec: spec({ + description: + 'Implement nested runner, runtime policy, telemetry, evals, and insights as smaller workflows run by a master executor.', + constraints: ['Use independent child workflows with deterministic 80-to-100 validation.'], + acceptanceGates: ['npm test'], + }), + artifactPath: 'workflows/generated/runtime-master.ts', + }); + expect(result.masterExecutionPlan).toBeDefined(); + const rendered = artifact(result).content; + + // The final-hard-validation step body must include both branches of the + // workspace-aware fallback so monorepos and flat repos both succeed. + expect(rendered).toContain('npm pkg get scripts.typecheck'); + expect(rendered).toContain('npm run typecheck'); + expect(rendered).toContain('npx tsc --noEmit'); + + // The bare `npx tsc --noEmit` (without the conditional guard) must not + // appear as the first command after `set -e` in any rendered .step body. + // That pattern is what would dump the tsc help text in monorepo roots. + expect(rendered).not.toMatch(/command: "set -e\\nnpx tsc --noEmit\\n/); + }); + it('uses a master workflow for broad target-file specs and leaves narrow specs on the existing renderer', () => { const broad = generate({ spec: spec({