Warn aipm init about ancestor pnpm workspaces; sync GUIDE.md with --help - #106
Merged
Merged
Conversation
A directory with no local package.json under an ancestor pnpm-workspace.yaml lets `pnpm add`/ `pnpm install` silently target the ANCESTOR's manifest and lockfile instead of a new repo's own. `aipm init` now detects and warns about this before telling the user to install, and the CLI README (published as GUIDE.md) documents the lint command, every flag, the quick start's install step, and points its architecture-spec link at GitHub instead of a path that 404s outside the monorepo checkout.
There was a problem hiding this comment.
Pull request overview
Addresses two onboarding hazards (issue #96) by (1) detecting and warning about ancestor pnpm-workspace.yaml workspaces during aipm init and (2) syncing the published CLI guide (packages/cli/README.md) with the actual aipm --help surface.
Changes:
- Add ancestor pnpm-workspace detection to
coreinit and return it asInitOutcome.ancestorWorkspace(public API now returnsPromise<InitOutcome>). - Emit a non-blocking stderr warning in the CLI when an ancestor workspace is detected, and add tests for both the detection and the warning behavior.
- Update CLI README and architecture spec to document the workspace hazard, add missing commands/flags, and fix the dead spec link.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/pipeline/types.ts | Introduces InitOutcome (public return type for init). |
| packages/core/src/pipeline/operations.ts | Updates init() signature to return Promise<InitOutcome>. |
| packages/core/src/pipeline/init.ts | Implements ancestor pnpm-workspace.yaml lookup and returns it via InitOutcome. |
| packages/core/src/pipeline/init.test.ts | Adds unit tests covering ancestor workspace detection behavior. |
| packages/core/src/index.ts | Exports InitOutcome from the public core entrypoint. |
| packages/core/api-report/core.api.md | Updates API report for the new init() signature and InitOutcome type. |
| packages/cli/src/run.ts | Prints a stderr warning when InitOutcome.ancestorWorkspace is set. |
| packages/cli/src/run.test.ts | Adds UAT coverage for warning/no-warning behavior and exit code. |
| packages/cli/README.md | Updates install/quick start + adds missing commands/flags + fixes spec link. |
| docs/specs/architecture.md | Syncs architecture spec API blocks and documents the new guard/warn. |
| .changeset/onboarding-workspace-hazard-and-guide-sync.md | Records a minor bump for the public API surface + doc sync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… consistency, comment accuracy - Regenerate packages/core/docs (init API return-type change plus a backlog of previously un-regenerated pages) so the published docs match packages/core/api-report/core.api.md. - README Install snippet now bootstraps into a directory distinct from the aipm init target, avoiding the non-empty-directory collision. - Use `pnpm exec aipm ...` consistently for every aipm invocation in the README quick start and in the CLI's own printed next-steps message — aipm is not on PATH for a non-global dev-dependency install. - Reword the ancestor-workspace-detection comment in init.ts to describe actual behavior (the outcome is only returned on success; a later throw still propagates as a rejection).
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two onboarding hazards from the DX study (issue #96):
package.jsonunder anancestor
pnpm-workspace.yamlhaspnpm add/pnpm installsilently target the ancestor'smanifest and lockfile — a first-time user following the old docs verbatim could corrupt an
unrelated parent repo.
packages/cli/README.md) drift vsaipm --help: missinglint, missing flags,missing the quick start's install step, and a dead relative link to the architecture spec (only
resolvable inside the monorepo checkout, 404s in the published npm package).
Changes
packages/core/src/pipeline/init.ts:runInitnow walks up from the target directory lookingfor an ancestor
pnpm-workspace.yamland returns it via a newInitOutcome.ancestorWorkspace.The local
package.jsonboundary is still always written regardless.packages/core/src/pipeline/{operations,types,index}.ts:init()'s public return type changesfrom
Promise<void>toPromise<InitOutcome>(additive; new exported type{ ancestorWorkspace?: string }).packages/cli/src/run.ts:aipm initprints a warning to stderr whenancestorWorkspaceisset, before the "Next: run
pnpm install" line, exit code stays 0 (informational, non-blocking).packages/cli/README.md(the "GUIDE.md" referenced in the issue — DX-study participantsliterally copied this file and named it
GUIDE.md): quick start now runsnpm init -ybeforethe install command with an explicit workspace-hazard callout; documents
lintand everysubcommand's flags (
--name,--refresh,--force,--force-downgrade,--as,--format,--rule,--verbose) to matchaipm --help; includes the quick start's install step; replacesthe dead
../../docs/specs/architecture.mdrelative link with an absolute GitHub URL.docs/specs/architecture.md§8.1: updated the pinnedinitsignature andInitOptions/InitOutcomeblock to match, plus a short note on the new guard..changeset/onboarding-workspace-hazard-and-guide-sync.md: minor bump for both packages (publicreturn-type change to
init()).Acceptance criteria mapping
npm init -ybefore install, with an explicit workspace-hazard note —packages/cli/README.mdInstall section (verified by reading the rendered file; no automateddoc-content test per repo convention for prose sections).
aipm initscaffolds a workspace boundary OR detects an ancestor workspace and warns —runInitalways writes a localpackage.json(pre-existing boundary) and now detects+warnson an ancestor
pnpm-workspace.yaml. Covered by:packages/core/src/pipeline/init.test.ts→describe('ancestor pnpm-workspace detection (issue #96)')(InitOutcome.ancestorWorkspaceset/unset, nested ancestor lookup)packages/cli/src/run.test.ts→'warns to stderr when an ancestor pnpm-workspace.yaml exists'and'does not warn when there is no ancestor pnpm-workspace.yaml'(UAT against the real CLI dispatcher/stderr contract)GUIDE.mdsynced withaipm --help: lint command, all flags, install step present; deadrelative link fixed — verified manually by diffing
packages/cli/README.mdagainstnode packages/cli/dist/bin.js --helpoutput (every command/flag row present) and confirmingthe architecture-spec link is an absolute, always-resolvable GitHub URL.
Test plan
pnpm run check(typecheck, lint, api-report, tests, knip, syncpack, prettier) — greenpnpm run fix:formatrun before pushpackages/core/src/pipeline/init.test.ts(ancestor detection: found nested, found deep, not found)packages/cli/src/run.test.ts(stderr warning content + exit code + no-warning case)packages/cli/README.mdcommand/flag tables cross-checked againstaipm --helpoutputRefs #96