Conversation
`--project all` expands to every id manifest/projects.yaml declares and persists that snapshot, so a monorepo keeps one line in its onboarding docs instead of a project list that has to be edited whenever the manifest changes. `all` is reserved: a mixed list (`all,backend`) is rejected as redundant or a typo, and a project whose id is literally `all` is still covered by the expansion but selected on its own through `teamai projects set all`, which takes plain ids. It stays an explicit operator choice — activate everything, project-private learnings included — rather than the auto-activation the multi-project design rules out. Snapshot semantics also keep the existing rule that re-running `init` is what re-resolves the active set. Scope is `init` only; `teamai projects set --all` is deliberately not added. Docs: the usage guide (en + zh-CN) flag table and multi-project section, the CLI help text, and the multi-project design note.
The unit cases in `init-projects.test.ts` drive `resolveActiveProjects` directly. This adds the end-to-end leg for the user-facing command: build the CLI, run `init <repo> --project all` against a real team repo, and read the ids back out of `.teamai/config.yaml`. Offline, without weakening the test: the remote is a synthetic HTTPS URL that git's `url.<base>.insteadOf` rewrites to a local bare repo. init rejects plain HTTP outright, and a plain-HTTP static server (the first attempt) never gets past the URL validator. The rewrite target is a filesystem path rather than `file:///C:/…`, which on Git for Windows parses as `/C:/…` and fails the clone. Two cases: `all` expands to every manifest id in manifest order (the fixture is deliberately not alphabetical, so an accidental sort is caught), and an explicit id still replaces the previous selection instead of merging with it. On `main` the first case fails with `Unknown project "all". Available projects: gamma, alpha, billing` while the second passes — the case that fails is the new selector, not the harness.
|
Local review of [P2] Re-running Reproduction using the built CLI and an isolated local bare Git remote:
The existing-clone path in The current E2E does not cover this path: its Git URL rewrite makes the detected remote a local filesystem path, so the second init treats it as a different repository and replaces the clone under Please refresh the reused clone before resolving the project snapshot, and add a real-CLI regression covering a remote manifest change between two initializations. Validation on macOS:
I did not run the full Agent / Provider matrix. |
Re-running init --project all against an existing clone previously expanded the stale local manifest. pullRepo before resolveActiveProjects so newly added remote projects are picked up.
… all Regression for PR Tencent#518 review: re-init with an existing clone must pick up projects added to the remote between runs.
|
Addressed in 09627a5.. (follow-up commit on this branch):
CI watching. |
jeff-r2026
left a comment
jeff-r2026
left a comment
There was a problem hiding this comment.
I reviewed the latest head (2d9c3362) and reproduced two issues locally. I recommend addressing these before merging.
Validation on macOS:
npm run build: passed.npx tsc --noEmit: passed.npx vitest run src/__tests__/init-projects.test.ts: 13 passed.npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/init-project-all.test.ts: 2 passed, 1 failed at line 188.- Additional real-CLI regression check for preserving a tracked local edit: passed against
origin/main(b9cd9ea6), failed against this PR. The affected init invocation used an explicit project ID and omitted--force.
The regression harness used isolated temporary HOME/project directories and a local bare remote. HTTPS transport was redirected locally; a test-only Git wrapper returned the synthetic origin for remote get-url so Git's insteadOf expansion would not accidentally take the different-repository/reclone path. Pull, fetch, and reset ran through real Git. No live team repository was modified.
| // (re-running init after a new project is added would otherwise keep the | ||
| // old project list — see PR #518 review). | ||
| try { | ||
| const pullResult = await pullRepo(localPath); |
There was a problem hiding this comment.
[P1] Avoid the destructive pull fallback during init
pullRepo() is not just a refresh: when git pull --ff-only fails, it can fetch and run reset --hard origin/<branch> on a dedicated clone. This new unconditional call therefore changes ordinary init behavior, including explicit project IDs and invocations without --force.
Local reproduction: initialize a project-scoped clone with --project billing; advance the remote manifest; append an uncommitted edit to the local manifest; remove only the fixture's local config to exercise init without --force while retaining the clone; initialize again with --project billing. This PR prints realigning discards 0 local commit(s) and 1 uncommitted change(s), deletes the local edit, and exits successfully. The same scenario on main preserves the edit.
Please use a non-destructive refresh here and stop with an actionable error if the clone cannot be fast-forwarded safely. Add a regression asserting that tracked local edits survive a failed refresh; retaining the existing pullRepo() hard-reset fallback makes reinitialization capable of losing work.
| expect(first.code, first.output).toBe(0); | ||
| expect(readProjects(projectRoot)).toEqual(['gamma', 'alpha', 'billing']); | ||
|
|
||
| const teamRepo = path.join(home, '.teamai', 'team-repo'); |
There was a problem hiding this comment.
[P2] Resolve the clone path from the project-scoped config
This test initializes with --scope project, but looks for the clone under the user-scoped home/.teamai/team-repo. Running the submitted E2E file fails at the next assertion (expected false to be true), before it can exercise the new refresh behavior. Read repo.localPath from the generated config instead of assuming a user-scope location.
Please also verify that the repaired offline fixture actually reaches using existing clone: locally, fixing the path alone still took the replacement branch because git remote get-url expands the insteadOf URL to the local filesystem path. Restoring the raw synthetic origin URL did not prevent that expansion.
The default Vitest configuration excludes src/__tests__/e2e/**, so the green default CI run does not establish that this added test passes. Please rerun the explicit E2E command after fixing the fixture and update the validation results.
Address jeff-r2026 review on Tencent#518: - init clone-reuse path uses pullRepoFastForward (ff-only only; never reset --hard) - e2e reads repo.localPath from project config; git wrapper keeps remotesMatch on synthetic origin - regression: tracked local edits survive a failed refresh
|
Addressed the latest review on
Validation:
|
jeff-r2026
left a comment
jeff-r2026
left a comment
There was a problem hiding this comment.
Rechecked dab812b2 locally. The P1 data-loss issue is fixed: the independent real-CLI reproduction now exits nonzero and preserves the tracked edit, including when retried with --force. Build, typecheck, and all 13 selector unit tests passed.
The E2E clone-location and origin-rewrite fixes are in place, but the submitted E2E file still reports 3 passed / 1 failed on macOS because the new path assertion compares /var/... with /private/var/.... In a local test copy, changing only that assertion to use fs.realpathSync on both sides made all 4 E2E cases pass, including remote manifest refresh and preservation of local edits. Please make that small fixture correction before merging.
One remaining recovery-message nit: init still suggests --force will replace this clone after refresh failure, but a matching origin always reuses the clone regardless of that flag. I verified that retrying the dirty-clone reproduction with --force fails again and preserves the edit. Please remove that replacement suggestion from this error (and the helper documentation), keeping the manual recovery advice. There is no need to add destructive replacement behavior to resolve the wording.
| const teamRepo = readClonePath(projectRoot); | ||
| expect(fs.existsSync(path.join(teamRepo, '.git'))).toBe(true); | ||
| // Project scope stores the clone under the project dataHome, not ~/.teamai. | ||
| expect(path.resolve(teamRepo)).toBe( |
There was a problem hiding this comment.
[P2] Canonicalize filesystem aliases before comparing clone paths
path.resolve() only normalizes path syntax; it does not resolve symlinks. On macOS, this test receives /private/var/folders/.../project/.teamai/team-repo from the CLI while the temporary fixture path is /var/folders/.../project/.teamai/team-repo. These identify the same directory, but this assertion fails before the remote-update scenario runs.
Reproduced with npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/init-project-all.test.ts --retry 0: 3 passed, 1 failed. Changing just this assertion in a local copy to the following produces 4 passing tests:
expect(fs.realpathSync(teamRepo)).toBe(
fs.realpathSync(path.join(projectRoot, '.teamai', 'team-repo')),
);2 participants
Summary
Implements
teamai init <repo> --project allfrom #509: the value expands to every id declared inmanifest/projects.yamland that snapshot is persisted intoLocalConfig.projects, so a monorepo's onboarding keeps one line instead of a project list that has to be edited whenever the manifest changes.Type of Change
Existing invocations are untouched: no flag → no projects, as before; an explicit id list → the same resolution as before.
Design framing (the note you asked for)
--project allis an explicit operator choice with snapshot semantics, not auto-activation:init.config.yamlstores the resolved id list, which is consistent with the point in your reply that re-runninginit --project allis what re-resolves against the current manifest. Adding a project to the manifest later does not silently change any directory's active set.LocalConfig.projectsas an explicit id list, so everything downstream is unchanged by construction: the member-roster append,pull's resource/learnings namespaces,recallscope, andcontribute's landing point all see a plain list of real project ids.The code comment says this out loud, so the next reader does not mistake the selector for an auto-activation back door.
Behaviour
allresolves insideresolveActiveProjects(src/init.ts), which already loads the manifest, validates ids and dedupes; the expansion islistProjectIds(manifest).--projectall[backend, frontend, gateway]— the manifest's own orderallallprojects: []allall,backendallalready covers every declared projectbackendTwo deliberate calls, both easy to change if you disagree:
allsubsumes the rest, so the other ids are redundant at best and a typo at worst — and silently accepting them would disarm the existing unknown-id check for exactly the invocation most likely to be scripted. The error names the offending list.allis shadowed by the selector. It is never silently dropped — the expansion still covers it, and it is still selectable alone throughteamai projects set all, which takes plain ids — but--project <...>can no longer express "only that project". A warning names the situation when the manifest is found to contain it. That is the unavoidable cost of reserving the word; the alternative (silently changing the meaning ofalldepending on the manifest) is worse for scripts.allis matched case-sensitively, like every project id, so a project namedAllis unaffected.Why a flag and not a new command
Per AGENTS.md's 奥卡姆剃刀: this extends the existing
--projectoption, adds no command and no new state.teamai projects set --allis left out, as you asked.Test Plan
npx tsc --noEmitpassesnpx vitest run— no new failures vsmain. The suite still exits non-zero on Windows because of a standing Windows-only failure pool (the CI matrix has no Windows runner); the set comparison below is the meaningful signal, not the exit code.src/__tests__/e2e/init-project-all.test.ts— the real-CLI leg for the command (not CI-gated, like every other file undere2e/): build, runinit <repo> --project allagainst an offline team repo, read the ids back out of.teamai/config.yaml. Onmainit fails withUnknown project "all". Available projects: gamma, alpha, billingwhile its explicit-id control case passes, so the failing case is the new selector and not the harness.npx tsc --noEmitnpm run buildnpx vitest run src/__tests__/init-projects.test.tsmain, so each one is a genuine before/afternpx vitest runfull suite,mainvs this branchdist/index.js, 9 scenariosresolveActiveProjectsexported, like the neighbouringresolveInitScope/resolveInitRepoStep 4 — full suite, before vs after
The suite has a large, noisy Windows-only failure pool, so a comparison by count is meaningless here (details below). Comparing test sets instead:
main(same tree,src/init.ts+src/index.tsreverted, new test file kept)main(13/13) and pass on the branch.pull-exclude,pull-learnings-deletion,pull-tombstone,remove,rules,scope,issue-82-84-94-regression). These are not improvements — they are the flaky pool: they all share HOME / temp state, and they flip between runs on the unmodified tree too (e.g.run 1of the branch had 3 failures thatrun 2did not). This is why the table above reports sets, not counts.ubuntu-latest+macos-latestonly, so nothing here is visible upstream; none of them are touched by this change and all of them are present onmain.Step 5 — real CLI
Nine scenarios against
npm run buildoutput. Each is a genuineteamai init <url> --scope project --force --agent claude [--project …]in afresh directory, and
config.yamlis read back afterwards:The run used a local bare repo reached through a fake
https://URL rewritten by git'sGIT_CONFIG_*override, so the real clone / identity / member-push path executed without touching the network or any developer's~/.gitconfig.Related Issues
Closes #509
Notes for Reviewers
One premise in the issue note is out of date. It says the
teamai projectscommand family "isn't onmainyet". It is:src/index.ts:298registersprojectswithlist/set [ids...]/members <id>, andsetis wired toprojectsSet(src/projects-cmd.ts:76). Soprojects set --allis blocked only by the missing flag, not by a missing command. I have still left it out of this PR as you asked — flagging it only so that call is made on current facts. This is also why the new warning points atteamai projects set all: that command exists today, and it is the escape hatch for the collision case.Docs touched (bilingual kept in sync per AGENTS.md):
docs/usage-guide.mdanddocs/usage-guide.zh-CN.md(the--projectflag table and the multi-project key points), the--projecthelp text insrc/index.ts, anddocs/designs/multi-project-management.md(theallsnapshot under "Entry point", plusprojects set --alladded to the explicit out-of-scope list).