From 27e08443ac244fe6938a1591c59c875bc4b9693b Mon Sep 17 00:00:00 2001 From: dvd233 <111864431+dvd233@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:33:45 -0700 Subject: [PATCH] fix(push): honor explicit branch and protect dirty team clones --- docs/usage-guide.md | 3 + docs/usage-guide.zh-CN.md | 3 + src/__tests__/push-pending-pr.test.ts | 12 +++- src/__tests__/push-role.test.ts | 36 +++++++++--- src/index.ts | 1 + src/push.ts | 79 ++++++++++++++++++++++----- 6 files changed, 112 insertions(+), 22 deletions(-) diff --git a/docs/usage-guide.md b/docs/usage-guide.md index dc2238e65..9937b62b3 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -573,8 +573,11 @@ Exclusion rules take effect after role and tag filtering. When running `teamai p teamai push # Scan for new/modified resources, create an MR teamai push --all # Skip confirmation, push directly teamai push --role pm # Push this skill to skills/pm// +teamai push --branch feature/gitee-destination # Use an explicit destination branch ``` +`--branch` names the branch that receives a new push; an existing open PR is always updated on its recorded branch. TeamAI refuses to start a push when the team-repo clone has user changes (modified, staged, untracked, or conflicted files); TeamAI-owned `teamai.yaml` and sync-lock state are handled separately. Commit or stash other local changes first. + **Namespace selection (new skills):** When pushing a new skill, the CLI automatically detects available namespaces and offers an interactive choice: ``` diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 8af8c6a5f..72a1a1eb6 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -551,8 +551,11 @@ excludedSkills: teamai push # 扫描新增/修改的资源，创建 MR teamai push --all # 跳过确认，直接推送 teamai push --role pm # 将本次 skill 推送到 skills/pm// +teamai push --branch feature/gitee-destination # 使用显式目标分支 ``` +`--branch` 指定新推送使用的分支；已有开放 PR 始终沿用其记录的分支进行更新。如果团队仓库 clone 存在用户修改、暂存、未跟踪或冲突文件，TeamAI 会在 push 前拒绝执行；TeamAI 自己管理的 `teamai.yaml` 和 sync-lock 状态会单独处理。其他本地改动请先提交或 stash。 + **命名空间选择（新 skill）：** 推送新 skill 时，CLI 会自动检测可用的命名空间并提供交互式选择： ``` diff --git a/src/__tests__/push-pending-pr.test.ts b/src/__tests__/push-pending-pr.test.ts index e1f85d57a..8ace8d450 100644 --- a/src/__tests__/push-pending-pr.test.ts +++ b/src/__tests__/push-pending-pr.test.ts @@ -308,7 +308,7 @@ describe('push() with an open PR', () => { it('updates the open PR instead of opening a second one', async () => { mockLoadStateForScope.mockResolvedValue(makeState([makeEntry()])); - await push({}); + await push({ branch: 'feature/should-not-override' }); expect(mockCreatePullRequest).not.toHaveBeenCalled(); expect(mockPushRepoBranch).toHaveBeenCalledTimes(1); @@ -370,6 +370,16 @@ describe('push() with an open PR', () => { })]); }); + it('uses --branch for a new push instead of generating a timestamp branch', async () => { + mockLoadStateForScope.mockResolvedValue(makeState()); + + await push({ all: true, branch: 'feature/gitee-destination' }); + + expect(mockPushRepoBranch.mock.calls[0][3]).toBe('feature/gitee-destination'); + const saved = mockSaveStateForScope.mock.calls.at(-1)?.[0] as State; + expect(saved.pendingPushes[0].branch).toBe('feature/gitee-destination'); + }); + it('re-pushes as a new PR once the recorded branch is gone from origin', async () => { mockRemoteBranchExists.mockResolvedValue(false); mockLoadStateForScope.mockResolvedValue(makeState([makeEntry()])); diff --git a/src/__tests__/push-role.test.ts b/src/__tests__/push-role.test.ts index 8b796769e..8a747ffec 100644 --- a/src/__tests__/push-role.test.ts +++ b/src/__tests__/push-role.test.ts @@ -158,6 +158,13 @@ function mockSkillHandler(pushedItems?: Array>) { describe('push namespace routing', () => { beforeEach(() => { vi.clearAllMocks(); + mockGitStatus.mockResolvedValue({ + modified: [], + not_added: [], + created: [], + conflicted: [], + staged: [], + }); mockPullRepo.mockResolvedValue('Already up to date.'); mockPushRepoBranch.mockResolvedValue(true); mockCheckoutMaster.mockResolvedValue(undefined); @@ -524,7 +531,7 @@ it('blocks skills that exist in non-allowed namespaces', async () => { consoleSpy.mockRestore(); }); - it('resets dirty team repo to clean master before pull', async () => { + it('aborts before resetting a dirty team repo', async () => { mockAutoDetectInit.mockResolvedValue({ localConfig: makeLocalConfig({ primaryRole: undefined }), teamConfig: makeTeamConfig(), @@ -532,21 +539,34 @@ it('blocks skills that exist in non-allowed namespaces', async () => { mockSkillHandler(); mockScanTeamRepoNamespaces.mockResolvedValue([]); + const previousExitCode = process.exitCode; + mockGitStatus.mockResolvedValue({ + modified: ['local-edit.txt'], + not_added: [], + created: [], + conflicted: [], + staged: [], + }); + await push({ all: true }); - // Should have called resetToCleanMaster before pull - expect(mockResetToCleanMaster).toHaveBeenCalled(); - expect(mockPullRepo).toHaveBeenCalled(); - // resetToCleanMaster must be called before pullRepo - const resetOrder = mockResetToCleanMaster.mock.invocationCallOrder[0]; - const pullOrder = mockPullRepo.mock.invocationCallOrder[0]; - expect(resetOrder).toBeLessThan(pullOrder); + expect(mockResetToCleanMaster).not.toHaveBeenCalled(); + expect(mockPullRepo).not.toHaveBeenCalled(); + expect(mockPushRepoBranch).not.toHaveBeenCalled(); + process.exitCode = previousExitCode; }); }); describe('push item selection', () => { beforeEach(() => { vi.clearAllMocks(); + mockGitStatus.mockResolvedValue({ + modified: [], + not_added: [], + created: [], + conflicted: [], + staged: [], + }); mockPullRepo.mockResolvedValue('Already up to date.'); mockPushRepoBranch.mockResolvedValue(true); mockCheckoutMaster.mockResolvedValue(undefined); diff --git a/src/index.ts b/src/index.ts index 496c5b275..358831049 100644 --- a/src/index.ts +++ b/src/index.ts @@ -79,6 +79,7 @@ program .option('--skill ', 'Push a specific skill by path (e.g., ~/.claude/skills/hai/my-skill or skills/hai_dev/my-skill)') .option('--role ', 'Target role namespace for pushed project skills') .option('--project ', 'Target a project: push skills into the project\'s skills namespace (from manifest/projects.yaml)') + .option('--branch ', 'Push to this destination branch instead of a generated teamai/push branch') .action(async (cmdOpts) => { const globalOpts = program.opts() as GlobalOptions; const { push } = await import('./push.js'); diff --git a/src/push.ts b/src/push.ts index a6001f591..fda1d557f 100644 --- a/src/push.ts +++ b/src/push.ts @@ -140,6 +140,45 @@ async function createPrWithFallback( export { createPrWithFallback }; +type PushRepoStatus = { + conflicted?: string[]; + modified?: string[]; + not_added?: string[]; + created?: string[]; + deleted?: string[]; + staged?: string[]; + renamed?: Array; +}; + +/** Return every path that would be at risk before a destructive push reset. */ +function collectDirtyPaths(status: PushRepoStatus): string[] { + const paths = new Set(); + for (const values of [ + status.conflicted, + status.modified, + status.not_added, + status.created, + status.deleted, + status.staged, + ]) { + for (const value of values ?? []) paths.add(value); + } + for (const renamed of status.renamed ?? []) { + if (typeof renamed === 'string') paths.add(renamed); + else { + paths.add(renamed.from); + paths.add(renamed.to); + } + } + return [...paths].sort(); +} + +const TEAMAI_OWNED_DIRTY_PATHS = new Set(['teamai.yaml', '.teamai/.sync-lock']); + +function isTeamaiOwnedDirtyPath(filePath: string): boolean { + return TEAMAI_OWNED_DIRTY_PATHS.has(filePath.replaceAll('\\', '/')); +} + /** * Push each selected resource into the team repo, commit it on a branch, and * open (or update) the matching PR. Returns false when the push failed, after @@ -151,8 +190,9 @@ async function pushGroup(args: { localConfig: LocalConfig; pushState: State; includeTeamConfig: boolean; + branch?: string; }): Promise { - const { group, teamConfig, localConfig, pushState, includeTeamConfig } = args; + const { group, teamConfig, localConfig, pushState, includeTeamConfig, branch } = args; const { items, reuse } = group; // pushItem copies files into the team repo's working tree. If any later @@ -199,7 +239,7 @@ async function pushGroup(args: { // sources / publicSkills changes ride along in the same PR as the resources. const configFiles = includeTeamConfig ? ['teamai.yaml'] : []; const gitFiles = [...new Set([...pushedFiles, ...existingSweepers, ...configFiles])]; - const branchName = reuse?.branch ?? generateBranchName(localConfig.username); + const branchName = reuse?.branch ?? branch ?? generateBranchName(localConfig.username); const commitMsg = `[teamai] Push ${items.length} resource(s) from ${localConfig.username}`; const hasChanges = await pushRepoBranch( @@ -280,7 +320,7 @@ async function pushGroup(args: { } } -export async function push(options: GlobalOptions & { all?: boolean; role?: string; project?: string }): Promise { +export async function push(options: GlobalOptions & { all?: boolean; role?: string; project?: string; branch?: string }): Promise { // Auto-detect scope: project scope if cwd has project config, else user scope const { localConfig, teamConfig } = await autoDetectInit(); assertNotReadOnly(localConfig, 'teamai push'); @@ -415,7 +455,7 @@ export async function push(options: GlobalOptions & { all?: boolean; role?: stri async function pushCore( localConfig: LocalConfig, teamConfig: TeamaiConfig, - options: GlobalOptions & { all?: boolean; role?: string }, + options: GlobalOptions & { all?: boolean; role?: string; branch?: string }, initialPendingTeamConfig: string | null = null, ): Promise { const selfMode = localConfig.repo.kind === 'self'; @@ -426,15 +466,11 @@ async function pushCore( // - Unmerged (conflicted) files without MERGE_HEAD (incomplete merge) // - Stuck on a stale push branch instead of master // - Uncommitted changes (e.g. votes written by autoUpvote) - // We recover from all of these before pulling. + // We reject dirty repos before pulling so no reset/clean operation can + // silently discard the user's work. // In self mode the worktree is already a fresh detached checkout of // origin/, so resetToCleanMaster/pullRepo (which assume a normal // clone on a branch) are neither needed nor safe — skip them. - // Uncommitted teamai.yaml edits (e.g. from `teamai source add`, which writes the - // file but does not commit) live in the team repo working tree. resetToCleanMaster - // below does `git reset --hard`, which would silently destroy them. Capture the - // working-tree content before the reset and restore it after pull, so config edits - // survive and get committed alongside resources (see gitFiles construction below). let pendingTeamConfig: string | null = initialPendingTeamConfig; if (!selfMode) { const pullSpin = spinner('Pulling latest changes...').start(); @@ -460,10 +496,20 @@ async function pushCore( pendingTeamConfig = workingContent; } } + const dirtyPaths = collectDirtyPaths(await git.status()); + const unsafeDirtyPaths = dirtyPaths.filter((filePath) => !isTeamaiOwnedDirtyPath(filePath)); + if (unsafeDirtyPaths.length > 0) { + pullSpin.fail( + 'Cannot push: the team repo has uncommitted changes. Commit or stash them first. ' + + `Paths: ${unsafeDirtyPaths.join(', ')}`, + ); + process.exitCode = 1; + return; + } await resetToCleanMaster(git, repoPath); await pullRepo(repoPath); if (pendingTeamConfig !== null) { - // Re-apply the user's config edits on top of the freshly pulled default branch. + // Re-apply the TeamAI-owned config edit after refreshing the default branch. await writeFile(yamlPath, pendingTeamConfig); } pullSpin.succeed('Up to date'); @@ -774,6 +820,12 @@ async function pushCore( // can happen in one run, so editing a resource under review updates its PR // without dragging unrelated resources into that review. const groups = planPushGroups(selectedItems, pendingPushes); + const newGroupCount = groups.filter((group) => !group.reuse).length; + if (options.branch && newGroupCount > 1) { + log.error('`--branch` can only target one new push branch at a time; select one resource group or omit it.'); + process.exitCode = 2; + return; + } for (const group of groups) { if (!group.reuse) continue; log.info( @@ -893,6 +945,7 @@ async function pushCore( localConfig, pushState, includeTeamConfig: configRider, + branch: options.branch, }); if (!ok) { // The branch/PR for earlier groups is already on the remote, so their @@ -932,7 +985,7 @@ async function pushCore( async function pushTeamConfigOnly( localConfig: LocalConfig, teamConfig: TeamaiConfig, - options: GlobalOptions, + options: GlobalOptions & { branch?: string }, ): Promise { console.log(''); console.log('Found team config change to push:'); @@ -945,7 +998,7 @@ async function pushTeamConfigOnly( } const pushSpin = spinner('Pushing team config...').start(); - const branchName = generateBranchName(localConfig.username); + const branchName = options.branch ?? generateBranchName(localConfig.username); const commitMsg = `[teamai] Update team config from ${localConfig.username}`; try {