Skip to content

feat(01KVTB3Z): resolve each target repo's own build toolchain structurally instead of hardcoding pnpm at plugins/flow#441

Merged
jackmcintyre merged 1 commit into
mainfrom
worktree-agent-a237fa1fc4ceee571
Jun 23, 2026
Merged

feat(01KVTB3Z): resolve each target repo's own build toolchain structurally instead of hardcoding pnpm at plugins/flow#441
jackmcintyre merged 1 commit into
mainfrom
worktree-agent-a237fa1fc4ceee571

Conversation

@jackmcintyre

Copy link
Copy Markdown
Owner

What

Flow's Stage-1 pre-PR build/test/bloat gates and the reviewer's vitest runner were hardcoded to assume the target IS the Flow repo: a pnpm monorepo rooted at plugins/flow. Run Flow against any external repo (an npm project with no plugins/ dir) and the pre-PR build gate failed before a PR could open — the dev's only escape was fabricating a fake plugins/flow/ package, contaminating the target.

This adds one toolchain resolverplugins/flow/mcp-server/src/lib/resolve-project-toolchain.ts — consumed by both the dev pre-PR gates and the reviewer test runner, so they agree on where and how to build/test any target repo.

Resolution order

  1. Config override (escape hatch only): if .flow/config.yaml has a build: block, it is zod-validated and its packageManager/cwd/buildCmd/testCmd/knipCmd win over structural + lockfile detection. An unrecognised packageManager raises a typed ToolchainConfigError (no silent fallback).
  2. Structural build-home detection (no config): prefer a pnpm-workspace.yaml member whose package.json owns a build script; else the nearest package.json (root downward) with a build script; else the repo root.
  3. Package-manager detection at the resolved cwd: pnpm-lock.yaml→pnpm, package-lock.json→npm, yarn.lock→yarn, bun.lockb→bun, default npm (flagged pmAssumed).

Returns { packageManager, cwd, buildCmd, testCmd, knipCmd | null, pmAssumed, source }. knipCmd is null (bloat gate skipped) when no build.knipCmd, no knip script, and no knip config apply.

How each AC is met

  • AC1 (external npm repo): runProjectBuild/runProjectTests run npm run build / npm test at the REPO ROOT; runProjectBloatCheck is a no-op (skipped: true, no subprocess) because knipCmd is null. No fabricated plugins/flow/ and no stray pnpm-lock.yaml. Covered in run-project-build-toolchain.test.ts + resolve-project-toolchain.test.ts.
  • AC2 (config wins): a build: block overrides structural + lockfile detection (incl. a partial block that overrides only packageManager but still derives the structural cwd).
  • AC3 (typed error): a malformed build: block (bad packageManager, unknown field) raises ToolchainConfigError rather than falling back.
  • AC4 (THE critical one — the gitignored-config correction): .flow/config.yaml is gitignored, so it does NOT reach a clean checkout or a per-story worktree. Structural detection alone resolves packageManager=pnpm and cwd=plugins/flow for the crew repo, purely from the on-disk plugins/flow/pnpm-workspace.yaml + pnpm-lock.yaml + the plugins/flow package.json's build script — verified against the REAL repo root on this clean worktree (no .flow/config.yaml present), and in resolve-project-toolchain.test.ts's Flow-shaped fixture.

Reviewer alignment

run-reviewer-session.ts's runVitestCheck no longer hardcodes pnpm vitest. It resolves the vitest binary from the package root's node_modules/.bin/vitest first, else the resolved package manager's run command (resolveVitestInvocation). The findPackageRoot re-export is preserved (the discipline validator depends on it) and lib/find-package-root.ts is untouched.

Flow dogfood path unchanged

The dev pre-PR gate and reviewer still build/test at plugins/flow with pnpm — now via the structural resolver rather than a hardcode. Existing pre-PR-gate / reviewer-vitest tests were re-pointed (Flow-shaped fixtures seeded so the resolver lands on the same place). FLOW_REPO_DISK_SENTINEL in write-native-story.ts was left untouched (out of scope).

Checks

  • Build green; committed dist/index.js + dist/cli.js rebuilt.
  • Full suite green: 3826 tests.
  • Lint + typecheck + knip clean.
  • Coverage: 86.26% lines / 85.44% branches (thresholds 86 / 85).

https://claude.ai/code/session_01E5VyKQMGJSL6fRfRmCzuvs

…urally instead of hardcoding pnpm at plugins/flow

Add resolve-project-toolchain.ts: one resolver consumed by BOTH the dev pre-PR
build/test/bloat gates and the reviewer's vitest runner, so they agree on where
and how to build/test any target repo. Resolution order: (1) optional .flow/
config.yaml build: block (escape hatch, zod-validated, typed ToolchainConfigError
on a bad packageManager); (2) structural build-home detection (pnpm-workspace
member owning a build script, else nearest package.json with a build script, else
repo root); (3) package-manager detection by lockfile at the resolved cwd.

The Flow dogfood path no longer depends on the gitignored .flow/config.yaml:
structural detection alone resolves pnpm + plugins/flow for the crew repo on a
clean worktree. An external npm repo resolves to npm + repo root with the bloat
(knip) gate skipped when no dead-code check applies.

run-reviewer-session.ts aligns its vitest invocation to the resolved toolchain
(local node_modules/.bin/vitest first, else the package manager's run command)
while preserving the findPackageRoot re-export and leaving find-package-root.ts
intact.

Claude-Session: https://claude.ai/code/session_01E5VyKQMGJSL6fRfRmCzuvs
@jackmcintyre jackmcintyre merged commit 44cc261 into main Jun 23, 2026
1 check passed
@jackmcintyre jackmcintyre deleted the worktree-agent-a237fa1fc4ceee571 branch June 23, 2026 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant