Repository navigation
Conversation
- Compare the first configured origin URL before insteadOf transport rewrites so reinitializing the same repository preserves its dirty clone. - Lock sync, reports and learnings writes, prove checkout ownership and archive the complete old clone with its owned worktrees before cloning another team repository. - Repair Git links in the backup, retain uncommitted files and unpublished refs, refuse foreign or unrelated checkouts, and restore the original layout when a move fails. - Verify 85 focused tests, TypeScript, lint and build; real init CLI fixtures cover same-repository reuse and switching repositories with pending reports/learnings data, with bilingual recovery guidance.
|
No findings in the scoped PR diff. The PR description documents sufficient testing: build, type checking, lint, focused unit tests, and a representative real-CLI run covering clone reuse and repository switching with preserved worktrees. Additional provider/agent coverage is explicitly deferred to CI/manual verification. 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 The replacement path preserves the old clone and linked worktrees in the reviewed scenarios, but CI is red at 9fe2a02d.
- [P1 blocking] E2E shard 2/3 fails two cases in
src/__tests__/e2e/self-mode-worktrees-808.test.ts. The queue-switch case still searches the old queue location and finds no note; the foreign-learnings-checkout case still expects the old refusal after the replacement path archives that checkout. Both fail again on retry. Please reconcile these expectations with the intended archive behavior while preserving their data-retention and cross-team-isolation assertions, then get CI green. The aggregate E2E job fails as a result. Whether either failure also occurs on the base was not established from this CI run.
The focused real-CLI verification passed the 7 init-ends-with-pull.test.ts cases and exercised behind, unpushed and dirty side worktrees. Those results do not supersede the failing regression cases.
CI review reported no findings and accepted the testing record. We agree a representative real-CLI record is present, but the later failing CI jobs block approval.
#1327 and this PR are alternatives for #1320. #1327 owns retirement by push/removal; this PR owns preservation by archiving. We favor preserving the old data, subject to green regression checks. Only one replacement path should land.
@nateEc #1327 now fixes #1320: at |
Summary
Switching team repositories previously removed the old clone while leaving
reports-wtandlearnings-wtpointing into its deleted.gitdirectory. A Giturl.*.insteadOfrewrite could also make reinitializing the same repository take that destructive path.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses completely (two unchanged local baseline failures; see below)Added/updated tests for the change
RED: a real
initfixture lost an uncommitted file when reinitialized throughinsteadOf.85 focused unit tests pass, covering pending data, ownership, dangling symlinks, all three live locks and rollback.
TypeScript, lint and build pass.
Real CLI:
init-ends-with-pull.test.tspassed all 7 cases; the final focused switch case also passes after the ownership guard updates. It initializes A, reuses A without losing its dirty file, switches to B, and verifies pending reports/learnings data and working Git links in the archive.Real-CLI command:
npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/init-ends-with-pull.test.ts.Repository-wide regression: the five-patch combined run on
origin/main6c57c3b7completed with 12,268 passed, 36 skipped, 2 failed across 503 files. Both failures are the existingsubmodule-checkoutcases for preserving empty old Claude/Codex hook files (git config --get hook.teamai-post-checkout.command). An independent pristine checkout of the same base reproduces exactly those 2 failures (24 other submodule tests pass). No new full-suite failure was introduced. The final ZCode exclusion follow-up is additionally covered by 267 uninstall/lifecycle tests and a post-uninstall pull through the real CLI.The final combined build, TypeScript and lint checks also pass. A final real-CLI run with all five patches passes 6 critical cases across all 5 affected E2E files, including the post-uninstall pull.
Related Issues
Fixes #1320.
Notes for Reviewers
Compare the first configured origin URL before transport rewrites. When the repository really changes, lock sync/reports/learnings writes, prove ownership, and archive the whole clone and its owned worktrees under a printed
previous-team-*directory. Repair the Git links there so dirty files and unpublished refs remain usable. Refuse unproven siblings or unrelated registered checkouts before moving anything, and restore the old layout if a move fails. Config handoff is idempotent; English/Chinese guides explain recovery.The backup is retained for recovery; no remote branch is deleted and nothing is merged.
Local runtime tests use the Git provider and isolated filesystem fixtures; additional providers and live agent applications are deferred to CI/manual verification.