Allow member-owned workspace directories - #434
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
bingran-you
left a comment
There was a problem hiding this comment.
Review result: changes requested
The intended happy path works, and top-level members/* directories remain strict. However, the new nested-node discriminator creates a validation bypass for malformed member identities.
Blocking: do not use a valid type value as the prerequisite for validating type
At apps/cli/src/commands/tree/validate-members.ts:156, a nested NODE.md is sent to validateMember only when getMemberType() already returns one of VALID_TYPES. That makes invalid or missing type fields classify the node as a workspace and skip validation entirely.
For example, both of these nested identity nodes now return { exitCode: 0, errors: [] }:
type: personal-assitant # typoand:
# type is missingBefore this change, any nested directory with a NODE.md reached validateMember, so these cases produced the existing invalid type / missing 'type' field errors. The added assistant test only covers an already-valid personal_assistant value and therefore cannot catch this regression.
Please make the non-member workspace boundary explicit rather than inferring it from “not a valid member type” (for example, a dedicated workspace marker or an explicit, documented workspace kind), and add regression cases for a misspelled and a missing nested member type. Otherwise the validator can only validate nested member identities after the field under validation is already correct.
Base-branch note
The repository default branch is currently dev, and dev already contains PR #416 (8de8347), which relaxed nested non-node directories with a broader seven-case test suite. This PR targets the older main release line at c257182. If this is an intentional backport plus a new workspace-node policy, please state that explicitly and reconcile the behavior/tests with #416; otherwise it should be retargeted and rebased onto the active branch. The validation bypass above applies either way.
Checks run at b71f2ba90c3b8e6c8299a6933981a30698f954dc
pnpm ci:check— passed (lint, typecheck, 498 GitHub-scan tests, and 104 CLI tests).pnpm exec oxfmt apps/cli/src/commands/tree/validate-members.ts apps/cli/tests/tree-maintenance.test.ts --check— passed.git diff --check origin/main...HEAD— passed.- Focused adversarial Vitest cases expecting the existing invalid/missing-type diagnostics — both fail because the function returns exit code
0.
Once the classification/validation ambiguity is removed and those regressions are covered, this should be ready for another pass.
Summary
Tests