Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/product/generation/master-workflow-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
DEFAULT_RETRY_MAX_ATTEMPTS,
} from '../../shared/constants.js';
import { planMasterExecution, type ChildWorkflowPlan, type MasterExecutionPlan } from '../orchestration/index.js';
import { deriveTestCommand } from './template-renderer.js';
import type {
DeterministicGate,
PatternDecision,
Expand Down Expand Up @@ -82,7 +83,7 @@ export function renderMasterExecutionWorkflow(input: RenderMasterWorkflowInput):
});
const channel = `wf-ricky-${slug}`;
const tasks = buildMasterTasks(plan);
const gates = buildMasterGates(artifactsDir, plan);
const gates = buildMasterGates(artifactsDir, plan, input.spec);
const skillApplicationEvidence = buildMasterSkillEvidence(input.skills);
const toolSelections = buildMasterToolSelections(plan);
const content = renderMasterSource({
Expand Down Expand Up @@ -283,7 +284,7 @@ function renderMasterSource(input: {
` command: ${literal([
'set -e',
TYPECHECK_COMMAND,
'npm test',
deriveTestCommand(input.spec),
'git diff --name-only',
`grep -F RICKY_MASTER_REVIEW_READY ${shellQuote(`${input.artifactsDir}/review-codex.md`)}`,
'echo RICKY_MASTER_FINAL_VALIDATION_READY',
Expand Down Expand Up @@ -478,16 +479,17 @@ function buildMasterTasks(plan: MasterExecutionPlan): WorkflowTask[] {
];
}

function buildMasterGates(artifactsDir: string, plan: MasterExecutionPlan): DeterministicGate[] {
function buildMasterGates(artifactsDir: string, plan: MasterExecutionPlan, spec: NormalizedWorkflowSpec): DeterministicGate[] {
const testCommand = deriveTestCommand(spec);
return [
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', `{ ${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', `{ ${TYPECHECK_COMMAND}; } && npm test`, 'exit_code', true, ['final-review-pass-gate'], 'final'),
gate('final-hard-validation', `{ ${TYPECHECK_COMMAND}; } && ${testCommand}`, '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'),
gate('regression-gate', testCommand, 'exit_code', true, ['git-diff-gate'], 'regression'),
];
}

Expand Down
82 changes: 82 additions & 0 deletions src/product/generation/pipeline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,88 @@ describe('workflow generation pipeline', () => {
expect(rendered.content).toContain('.run({ cwd: process.cwd() })');
});

// Regression: master-rendered final-hard-validation used to hardcode
// `npm test`, which walks the entire repo's test suite from the cwd.
// For monorepo specs that scope work to a few `packages/<pkg>/` files,
// any pre-existing or transient failure in an *unrelated* workspace
// package then blocks the workflow's final gate — work no agent in the
// generated workflow can sensibly repair because it isn't in the spec's
// declared scope. The renderer now derives the test command from the
// spec's targetFiles: when those targets share workspace prefixes
// (`packages/<pkg>/`, `apps/<pkg>/`, `services/<pkg>/`), emit
// `npm test --workspace=<pkg>` for each unique workspace; otherwise fall
// back to the previous unscoped behavior.
it('scopes master-rendered final-hard-validation tests to workspaces touched by the spec', () => {
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: ['Tests pass.'],
targetFiles: [
'packages/backend/src/services/autofix/index.ts',
'packages/backend/src/services/remediation-service.ts',
'packages/shared/src/autofix.ts',
],
}),
artifactPath: 'workflows/generated/runtime-master.ts',
});
expect(result.masterExecutionPlan).toBeDefined();
const rendered = artifact(result);

// Assert against the structured gate command + parsed step body, not
// raw rendered text — semantics over formatting. (CodeRabbit feedback
// on PR #91.)
const gateCommand = gate(rendered, 'final-hard-validation').command;
const stepBody = renderedStepCommand(rendered.content, 'final-hard-validation');

for (const target of [gateCommand, stepBody]) {
// Each unique workspace touched by the spec gets its own scoped run.
expect(target).toContain("npm test --workspace='packages/backend'");
expect(target).toContain("npm test --workspace='packages/shared'");
// Unrelated packages are not validated by this workflow's gate.
expect(target).not.toContain("npm test --workspace='packages/webapp'");
expect(target).not.toContain("npm test --workspace='packages/mobile'");
// The unscoped `npm test` whole-suite invocation must not survive
// anywhere in the validation surface — that's the exact pattern that
// produced the original cross-package failure. (`npm test
// --workspace=…` is fine; the negative lookahead allows it.)
expect(target).not.toMatch(/(?:^|[\s&|;])npm test(?!\s*--workspace)/);
}
});

it('uniqueWorkspacesFromTargetFiles handles npm-scoped workspace paths', () => {
// CodeRabbit flagged that the previous regex
// /^((?:packages|apps|services)\/[^\/]+)\//
// mis-parsed `packages/@scope/pkg/...` (matched only the `@scope`
// segment). The corrected regex allows an optional `@scope/` segment
// so scoped workspaces are recognised end-to-end.
const result = generate({
spec: spec({
description: 'Implement small slices across npm-scoped workspaces.',
constraints: ['Use independent child workflows.'],
acceptanceGates: ['Tests pass.'],
targetFiles: [
'packages/@agentworkforce/runtime/src/index.ts',
'packages/@agentworkforce/runtime/src/policy.ts',
'apps/@msd/web/src/index.ts',
'services/billing/src/index.ts',
],
}),
artifactPath: 'workflows/generated/scoped-master.ts',
});
expect(result.masterExecutionPlan).toBeDefined();
const command = gate(artifact(result), 'final-hard-validation').command;

expect(command).toContain("npm test --workspace='packages/@agentworkforce/runtime'");
expect(command).toContain("npm test --workspace='apps/@msd/web'");
expect(command).toContain("npm test --workspace='services/billing'");
// The previous bug surfaced as `--workspace='packages/@agentworkforce'`
// (truncated at the scope segment) — guard against regression.
expect(command).not.toContain("npm test --workspace='packages/@agentworkforce'");
expect(command).not.toContain("npm test --workspace='apps/@msd'");
});

// 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
Expand Down
43 changes: 42 additions & 1 deletion src/product/generation/template-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1236,15 +1236,56 @@ function describeImplementation(spec: NormalizedWorkflowSpec): string {
return `Edit declared targets: ${spec.targetFiles.join(', ')}`;
}

function deriveTestCommand(spec: NormalizedWorkflowSpec): string {
/**
* Derive the test command for a generated workflow based on the spec.
*
* Resolution order:
*
* 1. Explicit `vitest`/`npm test` acceptance gate → `mapAcceptanceGateToCommand`.
* 2. Test-file targets in `spec.targetFiles` → `npx vitest run <files>`.
* 3. Source-file targets that live under monorepo workspaces
* (`packages/<pkg>/`, `apps/<pkg>/`, `services/<pkg>/`, including
* scoped `packages/@scope/<pkg>/` layouts) → one
* `npm test --workspace=<pkg>` per unique workspace, chained with
* `&&`. This keeps validation focused on the spec's declared scope
* so unrelated failures in sibling packages cannot block the
* generated workflow's final-hard-validation gate.
* 4. Fallback for any other target shape → `npx vitest run`.
*
* Exported so the master workflow renderer can reuse the same logic
* instead of forking another implementation.
*/
export function deriveTestCommand(spec: NormalizedWorkflowSpec): string {
const explicitTestGate = spec.acceptanceGates.find((gate) => /\b(vitest|npm test)\b/i.test(gate.gate));
if (explicitTestGate) return mapAcceptanceGateToCommand(explicitTestGate.gate);

const testTargets = spec.targetFiles.filter((file) => /\.(test|spec)\.(ts|tsx|js|jsx)$/i.test(file));
if (testTargets.length > 0) return `npx vitest run ${testTargets.map(shellQuote).join(' ')}`;

const workspaces = uniqueWorkspacesFromTargetFiles(spec.targetFiles);
if (workspaces.length > 0) {
return workspaces.map((ws) => `npm test --workspace=${shellQuote(ws)}`).join(' && ');
}

return 'npx vitest run';
}

/**
* Extract the unique monorepo workspace paths touched by a list of target
* files. Recognises the three workspace-root conventions
* (`packages/`, `apps/`, `services/`) plus optional npm scope prefixes
* (e.g. `packages/@scope/<pkg>/`). Returns the workspace paths sorted for
* deterministic command emission.
*/
function uniqueWorkspacesFromTargetFiles(files: string[]): string[] {
const workspaces = new Set<string>();
for (const file of files) {
const match = file.match(/^((?:packages|apps|services)\/(?:@[^\/]+\/)?[^\/]+)\//);
if (match) workspaces.add(match[1]);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return [...workspaces].sort();
}

function mapAcceptanceGateToCommand(gateText: string): string {
const inlineCommand = extractInlineShellCommand(gateText);
if (inlineCommand) return inlineCommand;
Expand Down
Loading