Repository navigation
fix(init): anchor workspace onboarding to repository roots - #1344
Conversation
|
No findings in the specified PR diff. The PR description satisfies the testing requirement: it records 16 built-CLI E2E cases, including repository-root placement and ancestor-config protection, plus unit tests, type checking, and lint. Review was read-only; I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Thanks, approving. This fixes the init anchoring part of #1338. #1349 covers its test-isolation part.
We built this head, its merge-base 83f6d1db and main 54622c3a, and ran #1338's scenarios with the real CLI: init from a repository subdirectory, the --agent claude case, the child-folder overwrite and the nested repository. All four fail on base and main and pass here. The new muse-support cases pass 16/16 here, and 9 of them fail on main. The init unit tests (96), tsc, lint and CI pass. In a full local e2e run, every failing test also fails on main, and none is in a file this PR changes.
Suggestions:
[P2 non-blocking]When$HOMEis itself a Git repository (dotfiles), init from~/proj(not a repo) now resolves the root to$HOME. Without--scope, it falls back to a user-wide install with "cwd is your home directory; using user scope", which is untrue there, and--scope projectnow fails with "Cannot use the workspace scope in your home directory". Base printedScope: workspace (~/proj)but wrote into$HOME, so this is arguably better. #1338's comment asked the PR to name this change, though. A sentence in the body and a message such as "the repository at your home directory is the workspace root; using user scope" would cover it.[P3 nit]import YAML from 'yaml'sits among thenode:imports inmuse-support.test.ts. The getting-started paragraph is dense.
#1327 also changes src/init.ts in different hunks and merges cleanly with this PR. #1345 and #1346 conflict with it in muse-support.test.ts.
CI review: it reported no findings and accepted the testing record. We agree nothing blocks. The home-as-repo note above is our addition.
#1345 merged first, so this conflicts with |
- Resolve Git and HTTP workspace onboarding against the nearest repository or linked-worktree root, so saved configuration, printed paths and skill delivery agree with later commands. - Reject child-folder reinitialization of an existing ancestor even with --force. Inspect ancestor state read-only and preserve invalid or legacy-role configs without migration, partition adoption or API-key writes. - Cover ordinary folders, nested repositories, linked worktrees and Git/HTTP rejection paths, including byte-identical ancestor-config regression cases; update both onboarding guides and setup skill guidance. - Verify the built CLI with all 16 Muse-support E2E cases, 96 focused init unit tests, TypeScript and lint. Refs: Tencent#1338
1aa6f56 to
a07c430
Compare
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
|
No findings in the specified PR diff. The PR description satisfies the testing requirement: it documents built-CLI E2E runs covering repository-root placement and ancestor-config protection, plus unit tests, type checking, and lint. The earlier review raised no findings, so none require resolution. Review was read-only. I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Approving again at a07c430. Since 1aa6f56 the PR's own content changed only by a moved import in the test file, and the rest is the rebase. Build, tsc, lint and the init unit files pass. The Muse e2e passes 17/17 here and fails 9 on the base. With the built CLI, init from a repository subdirectory now stores and prints the repository root and leaves nothing in the subdirectory, and overwriting a child folder is refused with exit 1.
- [P2 non-blocking] The help for the workspace default (
src/index.ts:176) and the generatedskill-data/core/references/commands.md:26still say<cwd>/.teamai, which is no longer true inside a git subdirectory. Regenerate withnpx vitest run commands-reference -uafter changing the help. - [P2 non-blocking] From the earlier review, unchanged: when HOME is itself a git repository,
initfrom~/projinstalls at user scope with a message saying the cwd is the home directory.
The CI review reports no findings. We agree that nothing blocks, but our earlier review did raise the HOME message above. All CI jobs pass. This PR and #1346 conflict in muse-support.test.ts, so whichever lands second needs a rebase.
…1306 Conflicts, resolved so both features hold: - step-state.ts: RunningSchema takes Tencent#1303's expiresAt and limitsFrom; DeliveringSchema and the shared variant list stay. CancelledSchema keeps settledFields, which now carry Tencent#1303's unenforced. cancel() keeps a delivering step's settlement and uses Tencent#1303's stoppedLease for a leased one. stopWhere, which Tencent#1303 moved here, cancels a delivering step as before. - engine.ts: this branch's stopWhere is dropped for Tencent#1303's. The drain, the gate and the report vetting take the run's secrets and reconcile with a time, as updateRun, queueRunRecord and reconcileRun now require. leaseNext resolves the env before the drain, so its writes mask the same secrets. parseReport reads unenforced and still names unread changes fields. A report marks its copy used and may start a delivery. - types.ts, snapshot.ts: copies hold from and used; deliveries, creating, releasedTokens, stoppedAttempts, limitsFrom, unenforced and expiresAt each keep a field rule. - conformance.ts, conformance-interpreter.ts and both runners: the runner moved into Tencent#1303's interpreter. Scenarios send base instead of workspace: true, since every profile can take workspaces, and the interpreter fills a report's changes from its saved lease. - parse.ts, records.ts, run-controls.ts, workspace.ts, the handoff and engine tests and docs/dev/workflow-engine.md keep both sides. - Tests call startRun, cancelRun, retryStep and updateRun with secrets, and pass workspace modes in can.
teamai initfromrepo/packages/websaved and printed the subdirectory, while pull and uninstall subsequently anchored torepo. It could also replace an existing ancestor workspace config from a plain child folder with--force.Resolve the nearest repository root for both Git and HTTP onboarding, and print the actual partition config path. Refuse child-folder replacement of an existing ancestor before writes or migrations. Nested repositories and linked worktrees retain their own roots; directories outside Git retain their own scope.
Validation:
npm run buildandnpx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/muse-support.test.ts: 16 passed through the built CLI and local bare Git fixtures. Covers root/subdirectory placement, linked and nested repositories, ordinary folders, Git/HTTP force and non-force protection, and byte-identical invalid/legacy config preservation.npx vitest run src/__tests__/init.test.ts src/__tests__/init-scope-default.test.ts: 96 passed on the final source.npx tsc --noEmit,npm run lint, andgit diff --check: passed.State review: init reads through
resolveInitContext/loadLocalConfigForScopeand saves through the existing scope-aware config writers. Pull, doctor and uninstall already use the repository anchor. Ancestor preflight disables role migration, partition adoption and self-bootstrap before rejecting, including before HTTP token persistence. Both onboarding guides and setup skill guidance are updated.Broader regression:
npx vitest run --maxWorkers=6in a frozen integration worktree containing the five changes finished with 500 files passed, 12,285 tests passed, 36 skipped, 2 failed. Both failures are the pre-existingsubmodule-checkout.test.tsempty legacy hook-file cases; a clean83f6d1dbworktree independently reproduced 24 passed / the same 2 failed. This is an integration-suite record, not a claim that the baseline is fully green. The final read-only ancestor preflight was added after that integration snapshot; the final source then passed the 96 init unit cases, all 16 built-CLI E2E cases, TypeScript and lint listed above.Refs #1338. One independently reviewable defect from the issue; the audit/issue remains open for its other items.
Follow-up verification (2026-10-25)
muse-support.test.ts: 17 passed, including subdirectory, linked-worktree, nested-repository, invalid parent config and ancestor-protection cases.Updated head:
a07c4305b096c38b43d5ca3488818995d838a926. Rebased onto main6ebb31ce.