diff --git a/CHANGELOG.md b/CHANGELOG.md index 8624b3cc..3f82cc3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Changelog -## 0.8.4 - Unreleased +## 0.8.5 - Unreleased + +## 0.8.4 - 2026-10-21 + +**Highlights:** Package-less nested TypeScript sources receive deterministic ownership without crossing explicit project boundaries. + +- Fixed deterministic mapping of package-less nested Node/TypeScript sources under `packages/*`, assigning unowned files to bounded source groups while preserving explicit project boundaries, thanks @ptahdunbar for the report (#243). ## 0.8.3 - 2026-10-20 diff --git a/docs/feature-mapping.md b/docs/feature-mapping.md index 07e834c7..6fb94b15 100644 --- a/docs/feature-mapping.md +++ b/docs/feature-mapping.md @@ -123,6 +123,14 @@ clawpatch next --project web When an Nx project target is available, nearby tests use the project-scoped command, such as `yarn nx test web`, instead of a repository-wide test command. +Package-less projects under `packages/*` also cover nested Node/TypeScript +sources outside conventional folders, such as `packages/core/governance/types.ts`. +After conventional and framework mapping, remaining sources get one bounded +source-group owner in the nearest discovered package-like directory. Explicit +`package.json` and `project.json` boundaries retain control of their subtrees; +fallback mapping does not expand an explicit project's declared source scope. +Workspace and configured excludes, generated files, and symlinks remain excluded. + When Turborepo metadata is available, mapped workspace features use filtered Turbo validation commands such as `pnpm turbo run test --filter web`. Clawpatch does not execute Turbo during mapping and leaves task dependency expansion to diff --git a/package.json b/package.json index fa42c17e..493961f1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "clawpatch", - "version": "0.8.3", + "version": "0.8.4", "description": "Automated code review that lands fixes.", "license": "MIT", "repository": { diff --git a/src/mapper.ts b/src/mapper.ts index 4d7c3ace..76690426 100644 --- a/src/mapper.ts +++ b/src/mapper.ts @@ -16,7 +16,7 @@ import { laravelSeeds } from "./mappers/laravel.js"; import { mavenSeeds } from "./mappers/maven.js"; import { nextSeeds } from "./mappers/next.js"; import { nodeRouteSeeds } from "./mappers/node-routes.js"; -import { nodeSeeds } from "./mappers/node.js"; +import { nodeSeeds, packageLessSourceSeeds } from "./mappers/node.js"; import { pythonSeeds } from "./mappers/python.js"; import { reactSeeds } from "./mappers/react.js"; import { createMapperContext } from "./mappers/context.js"; @@ -329,7 +329,12 @@ async function collectSeeds( return seeds; }), ); - return dedupeFeatureSeeds(groups.flat()); + const seeds = groups.flat(); + // Fill only ownership gaps after all conventional and framework mappers have run. + if (await runNodeMappers) { + seeds.push(...(await packageLessSourceSeeds(root, context, seeds))); + } + return dedupeFeatureSeeds(seeds); } async function shouldRunNodeMappers(root: string, project: ProjectRecord): Promise { diff --git a/src/mappers/node-package-less.test.ts b/src/mappers/node-package-less.test.ts new file mode 100644 index 00000000..ea1d0a89 --- /dev/null +++ b/src/mappers/node-package-less.test.ts @@ -0,0 +1,185 @@ +import { symlink } from "node:fs/promises"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { detectProject } from "../detect.js"; +import { mapFeatures } from "../mapper.js"; +import { fixtureRoot, writeFixture } from "../test-helpers.js"; +import type { FeatureRecord } from "../types.js"; + +function owners(features: FeatureRecord[], path: string): FeatureRecord[] { + return features.filter((feature) => feature.ownedFiles.some((file) => file.path === path)); +} + +describe("package-less nested Node sources", () => { + it.each([true, false])( + "maps the reproduction once with workspace metadata: %s", + async (workspace) => { + const root = await fixtureRoot("clawpatch-nested-package-less-"); + if (workspace) { + await writeFixture( + root, + "package.json", + '{"name":"portable-mapping-fixture","private":true,"type":"module"}\n', + ); + await writeFixture(root, "pnpm-workspace.yaml", "packages:\n - 'packages/*'\n"); + } + const path = "packages/core/governance/types.ts"; + await writeFixture(root, path, "export type Policy = { enabled: boolean };\n"); + const project = await detectProject(root); + const first = await mapFeatures(root, project, []); + expect(owners(first.features, path)).toHaveLength(1); + expect(owners(first.features, path)[0]?.tags).toContain("project-root:packages/core"); + const second = await mapFeatures(root, project, first.features); + expect(second.created).toBe(0); + expect(second.changed).toBe(0); + expect(second.features.map((feature) => feature.featureId)).toEqual( + first.features.map((feature) => feature.featureId), + ); + }, + ); + + it("keeps conventional sources and explicit nested project owners", async () => { + const root = await fixtureRoot("clawpatch-nested-owners-"); + await writeFixture(root, "pnpm-workspace.yaml", "packages:\n - 'packages/**'\n"); + await writeFixture(root, "packages/core/src/index.ts", "export const core = true;\n"); + await writeFixture( + root, + "packages/core/governance/types.ts", + "export type Policy = boolean;\n", + ); + await writeFixture( + root, + "packages/core/governance/types.test.ts", + "test('policy', () => {});\n", + ); + await writeFixture( + root, + "packages/core/governance/nested/project.json", + JSON.stringify({ + name: "explicit-policy", + sourceRoot: "packages/core/governance/nested/implementation", + targets: { test: {} }, + }), + ); + await writeFixture( + root, + "packages/core/governance/nested/implementation/policy.ts", + "export const policy = true;\n", + ); + await writeFixture( + root, + "packages/core/governance/nested/outside.ts", + "export const outside = true;\n", + ); + await writeFixture(root, "packages/core/child/package.json", '{"name":"child"}\n'); + await writeFixture(root, "packages/core/child/src/child.ts", "export const child = true;\n"); + const result = await mapFeatures(root, await detectProject(root), []); + for (const path of [ + "packages/core/src/index.ts", + "packages/core/governance/types.ts", + "packages/core/governance/nested/implementation/policy.ts", + "packages/core/child/src/child.ts", + ]) { + expect(owners(result.features, path), path).toHaveLength(1); + } + expect( + owners(result.features, "packages/core/governance/nested/implementation/policy.ts")[0]?.tags, + ).toContain("project:explicit-policy"); + expect(owners(result.features, "packages/core/governance/nested/outside.ts")).toHaveLength(0); + expect(owners(result.features, "packages/core/governance/types.ts")[0]?.tests).toContainEqual({ + path: "packages/core/governance/types.test.ts", + command: null, + }); + }); + + it("respects configured and workspace excludes, generated files, and symlinks", async () => { + const root = await fixtureRoot("clawpatch-nested-excludes-"); + await writeFixture( + root, + "pnpm-workspace.yaml", + "packages:\n - 'packages/*'\n - '!packages/ignored'\n - '!packages/core/excluded'\n", + ); + const source = "packages/core/governance/types.ts"; + await writeFixture(root, source, "export type Policy = boolean;\n"); + const ignored = [ + "packages/ignored/governance/types.ts", + "packages/core/private/secret.ts", + "packages/core/excluded/policy.ts", + "packages/core/dist/output.ts", + "packages/core/node_modules/dependency/index.ts", + "packages/core/generated/types.ts", + "packages/core/governance/types.generated.ts", + "packages/core/fixtures/sample.ts", + "packages/core/governance/types.d.ts", + "packages/core/governance/types.test.ts", + ]; + for (const path of ignored) await writeFixture(root, path, "export const ignored = true;\n"); + if (process.platform !== "win32") { + await symlink( + join(root, "packages/core/governance"), + join(root, "packages/core/linked"), + "dir", + ); + await symlink(join(root, source), join(root, "packages/core/link.ts")); + } + const result = await mapFeatures(root, await detectProject(root), [], { + filters: { include: ["**/*"], exclude: ["packages/core/private/**"] }, + }); + expect(owners(result.features, source)).toHaveLength(1); + for (const path of [...ignored, "packages/core/linked/types.ts", "packages/core/link.ts"]) + expect(owners(result.features, path), path).toHaveLength(0); + }); + + it("bounds source groups and leaves package manifests authoritative", async () => { + const root = await fixtureRoot("clawpatch-nested-bounds-"); + const paths = Array.from( + { length: 29 }, + (_, index) => `packages/core/governance/policy${index}.ts`, + ); + for (const path of paths) await writeFixture(root, path, "export const policy = true;\n"); + await writeFixture(root, "packages/explicit/package.json", '{"name":"explicit"}\n'); + await writeFixture( + root, + "packages/explicit/governance/types.ts", + "export type Policy = boolean;\n", + ); + const result = await mapFeatures(root, await detectProject(root), []); + for (const path of paths) expect(owners(result.features, path), path).toHaveLength(1); + expect( + result.features.every( + (feature) => feature.source !== "node-source-group" || feature.ownedFiles.length <= 12, + ), + ).toBe(true); + expect(owners(result.features, "packages/explicit/governance/types.ts")).toHaveLength(0); + }); + + it("respects nested manifests outside the default workspace discovery scope", async () => { + const root = await fixtureRoot("clawpatch-nested-manifest-boundary-"); + await writeFixture(root, "pnpm-workspace.yaml", "packages:\n - 'packages/*'\n"); + const path = "packages/core/governance/types.ts"; + await writeFixture(root, path, "export type Policy = boolean;\n"); + await writeFixture(root, "packages/core/child/package.json", '{"name":"child"}\n'); + await writeFixture( + root, + "packages/core/child/governance/types.ts", + "export type Child = boolean;\n", + ); + await writeFixture( + root, + "packages/core/scripts/child/package.json", + '{"name":"script-child"}\n', + ); + await writeFixture( + root, + "packages/core/scripts/child/src/index.ts", + "export const child = true;\n", + ); + await writeFixture(root, "packages/core/src/child/package.json", '{"name":"source-child"}\n'); + await writeFixture(root, "packages/core/src/child/index.ts", "export const child = true;\n"); + const result = await mapFeatures(root, await detectProject(root), []); + expect(owners(result.features, path)).toHaveLength(1); + expect(owners(result.features, "packages/core/child/governance/types.ts")).toHaveLength(0); + expect(owners(result.features, "packages/core/scripts/child/src/index.ts")).toHaveLength(0); + expect(owners(result.features, "packages/core/src/child/index.ts")).toHaveLength(0); + }); +}); diff --git a/src/mappers/node.ts b/src/mappers/node.ts index d97adff8..6a799f1f 100644 --- a/src/mappers/node.ts +++ b/src/mappers/node.ts @@ -10,15 +10,18 @@ import { packageKind, packageTrustBoundaries, pathMatchesPrefix, + shouldSkip, walk, } from "./shared.js"; import { packageRelativePath, + nodeWorkspaceExcludes, projectContextFiles, projectDisplayName, projectTags, projectTargetCommand, } from "./projects.js"; +import { isExcludedWorkspace } from "./workspace-patterns.js"; import type { NodePackageJson, NodeProjectInfo } from "./projects.js"; import type { WorkspaceTaskGraph } from "./task-graph.js"; import { @@ -34,6 +37,12 @@ type PackageInfo = NodeProjectInfo & { packageJson: NodePackageJson; }; +type SourceGroupOptions = { + sourceRoots?: string[]; + files?: string[]; + includeFile?: (path: string) => boolean; +}; + const sourceDirectories = ["src", "lib", "app", "pages", "scripts", "server", "api"] as const; const testDirectories = ["test", "tests", "__tests__"] as const; const sourceGroupMaxOwnedFiles = 12; @@ -71,6 +80,9 @@ export async function nodeSeeds(root: string, context: MapperContext): Promise { + const projects = await context.nodeProjects(); + const taskGraph = await context.nodeTaskGraph(); + const excludes = await nodeWorkspaceExcludes(root); + const ownedPaths = new Set( + existingSeeds.flatMap((seed) => + (seed.ownedFiles ?? [{ path: seed.entryPath }]).map((file) => file.path), + ), + ); + const seeds: FeatureSeed[] = []; + for (const info of projects) { + if (!isPackageLessProject(info)) { + continue; + } + const otherRoots = projects + .filter((project) => project !== info && project.root !== ".") + .flatMap((project) => [ + project.root, + ...(project.sourceRoot === null ? [] : [project.sourceRoot]), + ]) + .filter((path) => path !== info.root && pathMatchesPrefix(path, info.root)); + const files = await walk( + root, + [info.root], + (path) => + shouldSkip(path) || + isExcludedWorkspace(path, excludes) || + otherRoots.some((other) => pathMatchesPrefix(path, other)), + ); + // A nested manifest is a boundary even when workspace globs did not discover it. + const manifestRoots = files + .filter((path) => ["package.json", "project.json"].includes(basename(path))) + .map((path) => dirname(path)); + const projectFiles = files.filter( + (path) => !manifestRoots.some((manifestRoot) => pathMatchesPrefix(path, manifestRoot)), + ); + const conventional = await sourceGroupSeeds(root, info, taskGraph, { + files: projectFiles, + includeFile: (path) => !ownedPaths.has(path), + }); + seeds.push(...conventional); + for (const seed of conventional) { + for (const file of seed.ownedFiles ?? []) ownedPaths.add(file.path); + } + seeds.push( + ...(await sourceGroupSeeds(root, info, taskGraph, { + sourceRoots: [info.root], + files: projectFiles, + includeFile: (path) => !ownedPaths.has(path), + })), + ); + } + return seeds; +} + +function isPackageLessProject(info: NodeProjectInfo): boolean { + return ( + info.packageJsonPath === null && + info.projectJsonPath === null && + info.root.startsWith("packages/") + ); +} + function hasNodePackage(project: NodeProjectInfo): project is PackageInfo { return project.packageJsonPath !== null && project.packageJson !== null; } @@ -193,20 +272,23 @@ async function sourceGroupSeeds( root: string, info: NodeProjectInfo, taskGraph: WorkspaceTaskGraph, + options: SourceGroupOptions = {}, ): Promise { const packageName = projectDisplayName(info); const testCommand = projectTargetCommand(info, "test", taskGraph); - const testFiles = await packageTestFiles(root, info); + const testFiles = await packageTestFiles(root, info, options); const railsPackage = await isRailsPackage(root, info.root); const seeds: FeatureSeed[] = []; - for (const sourceRoot of packageSourceRoots(info, railsPackage)) { + for (const sourceRoot of options.sourceRoots ?? packageSourceRoots(info, railsPackage)) { if (!(await pathExists(join(root, sourceRoot)))) { continue; } - const files = (await walk(root, [sourceRoot])).filter( + const files = (options.files ?? (await walk(root, [sourceRoot]))).filter( (path) => + (options.files === undefined || pathMatchesPrefix(path, sourceRoot)) && isReviewableNodeSourceFile(path) && + (options.includeFile?.(path) ?? true) && !isRailsExcludedNodeSourcePath(info, railsPackage, sourceRoot, path), ); if (files.length === 0) { @@ -422,13 +504,24 @@ function isRailsExcludedNodeSourcePath( ); } -async function packageTestFiles(root: string, info: NodeProjectInfo): Promise { +async function packageTestFiles( + root: string, + info: NodeProjectInfo, + options: SourceGroupOptions = {}, +): Promise { const railsPackage = await isRailsPackage(root, info.root); const prefixes = [ - ...packageSourceRoots(info, railsPackage), + ...(options.sourceRoots ?? packageSourceRoots(info, railsPackage)), ...testDirectories.map((dir) => packageRelativePath(info.root, dir)), ]; - return (await walk(root, prefixes)).filter(isNodeTestPath).slice(0, 200); + return (options.files ?? (await walk(root, prefixes))) + .filter( + (path) => + (options.files === undefined || + prefixes.some((prefix) => pathMatchesPrefix(path, prefix))) && + isNodeTestPath(path), + ) + .slice(0, 200); } async function isRailsPackage(root: string, packageRoot: string): Promise { diff --git a/src/mappers/projects.ts b/src/mappers/projects.ts index 386eda22..3fae934c 100644 --- a/src/mappers/projects.ts +++ b/src/mappers/projects.ts @@ -474,6 +474,15 @@ async function discoverGenericProjectRoots( return roots; } +export async function nodeWorkspaceExcludes(root: string): Promise { + return (await declaredWorkspacePatterns(root, await readPackageJson(root))) + .filter((pattern) => pattern.startsWith("!")) + .flatMap((pattern) => { + const normalized = normalizeWorkspacePattern(pattern.slice(1)); + return normalized === null ? [] : [normalized]; + }); +} + function isNestedUnderRoots(projectRoot: string, roots: Iterable): boolean { return [...roots].some( (existingRoot) => @@ -749,6 +758,9 @@ async function hasGenericProjectSignal( return true; } } + if (/^packages\/[^/]+$/u.test(projectRoot)) { + return (await sourceLikeFiles(root, projectRoot)).length > 0; + } return false; }