From 8129a9ef9ba8ad1f298633225a220f6467c6d3bc Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Mon, 28 Sep 2026 15:24:26 +0200 Subject: [PATCH 1/5] fix(push): publish the env files env add leaves in a standalone clone (#881) The #690 dirty-clone guard exempted only teamai.yaml and the sync lock, so `env add` then `push` in a standalone clone stopped at "Cannot push: the team repo has uncommitted changes" and the env edit was never published. Capture the dirty files EnvHandler.scanLocalForPush lists (content readable, mode unchanged), exempt them from the guard, and write them back after the reset and pull. The write-back runs in a finally: unlike teamai.yaml nothing later in the run holds the content, so a failed refresh after reset --hard would otherwise drop the edit. Deletions, mode changes and every other dirty path still stop the push. --- docs/usage-guide.md | 2 +- docs/usage-guide.zh-CN.md | 2 +- src/__tests__/push-env.test.ts | 186 +++++++++++++++++++++++++++++++++ src/push.ts | 33 +++++- 4 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/push-env.test.ts diff --git a/docs/usage-guide.md b/docs/usage-guide.md index e6f430ff9..6e6814383 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -766,7 +766,7 @@ teamai push --role pm # Push into the pm namespace (skills/pm/, rules/pm/, agen 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. +`--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`, the env files `teamai env add` edited, and sync-lock state are handled separately. Commit or stash other local changes first. **Namespace selection (new resources):** When pushing a new skill, rule or agent, 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 50d79ac2b..700907d68 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -695,7 +695,7 @@ teamai push --role pm # 推送到 pm namespace(skills/pm/、rules/pm/、agent teamai push --branch feature/gitee-destination # 使用显式目标分支 ``` -`--branch` 指定新推送使用的分支;已有开放 PR 始终沿用其记录的分支进行更新。如果团队仓库 clone 存在用户修改、暂存、未跟踪或冲突文件,TeamAI 会在 push 前拒绝执行;TeamAI 自己管理的 `teamai.yaml` 和 sync-lock 状态会单独处理。其他本地改动请先提交或 stash。 +`--branch` 指定新推送使用的分支;已有开放 PR 始终沿用其记录的分支进行更新。如果团队仓库 clone 存在用户修改、暂存、未跟踪或冲突文件,TeamAI 会在 push 前拒绝执行;TeamAI 自己管理的 `teamai.yaml`、`teamai env add` 修改的 env 文件和 sync-lock 状态会单独处理。其他本地改动请先提交或 stash。 **命名空间选择(新资源):** 推送新的 skill、rule 或 agent 时,CLI 会自动检测可用的命名空间并提供交互式选择: diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts new file mode 100644 index 000000000..e806d1b45 --- /dev/null +++ b/src/__tests__/push-env.test.ts @@ -0,0 +1,186 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; +import { simpleGit } from 'simple-git'; + +// Regression for #881: `env add` edits env/env.yaml in a standalone team clone +// without committing and leaves the commit to `push`. The #690 dirty-clone +// guard refused that file, so the edit never got published. Drives the real +// envAdd() and push() against a bare "remote" and a working clone, mocking only +// config detection and the provider's PR creation. + +const mockCreatePullRequest = vi.fn().mockResolvedValue('https://example.test/pr/1'); +const mockAutoDetectInit = vi.fn(); +const mockDetectProjectConfig = vi.fn(); + +vi.mock('../providers/index.js', () => ({ + getProvider: () => ({ + name: 'github', + parseRepoInput: (input: string) => ({ owner: 'acme', repo: 'team', httpsUrl: input }), + createPullRequest: (...args: unknown[]) => mockCreatePullRequest(...args), + }), +})); + +vi.mock('../config.js', async (importOriginal) => ({ + ...(await importOriginal()), + autoDetectInit: (...args: unknown[]) => mockAutoDetectInit(...args), + detectProjectConfig: (...args: unknown[]) => mockDetectProjectConfig(...args), + loadStateForScope: vi.fn(() => Promise.resolve({ + lastPush: null, pushedSkills: [], pushedRules: [], pushedEnvVars: [], + })), + saveStateForScope: vi.fn(() => Promise.resolve()), +})); + +vi.mock('../read-only.js', () => ({ assertNotReadOnly: vi.fn() })); +vi.mock('../utils/pre-push-sync.js', () => ({ syncTeamUpdatesToLocal: vi.fn() })); +vi.mock('../utils/prompt.js', () => ({ + isInteractive: vi.fn(() => true), + askQuestion: vi.fn(() => Promise.resolve('')), + askConfirmation: vi.fn(() => Promise.resolve(true)), + askSelection: vi.fn((_p: string, n: number, all?: boolean) => + Promise.resolve(all ? Array.from({ length: n }, (_x, i) => i) : null)), + parseSelection: vi.fn(), + closePrompt: vi.fn(), +})); + +async function initTeamRepos(root: string): Promise<{ teamRepo: string; remote: string }> { + const remote = path.join(root, 'remote.git'); + const seed = path.join(root, 'seed'); + const teamRepo = path.join(root, 'team-repo'); + await simpleGit().init(['--bare', '--initial-branch=main', remote]); + + fs.mkdirSync(path.join(seed, 'env'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'version: 1\n'); + fs.writeFileSync(path.join(seed, 'README.md'), '# team\n'); + fs.writeFileSync(path.join(seed, 'env', 'env.yaml'), 'variables:\n - key: TEAM_VAR\n value: first\n'); + const seedGit = simpleGit(seed); + await seedGit.init(); + await seedGit.addConfig('user.email', 't@t.com'); + await seedGit.addConfig('user.name', 't'); + await seedGit.add('.'); + await seedGit.commit('init'); + await seedGit.branch(['-M', 'main']); + await seedGit.addRemote('origin', remote); + await seedGit.push(['-u', 'origin', 'main']); + + await simpleGit().clone(remote, teamRepo); + const trGit = simpleGit(teamRepo); + await trGit.addConfig('user.email', 't@t.com'); + await trGit.addConfig('user.name', 't'); + return { teamRepo, remote }; +} + +/** What push printed to stderr, where the spinner reports the refusal. */ +function stderrOutput(): string { + return vi.mocked(process.stderr.write).mock.calls.map(([chunk]) => String(chunk)).join(''); +} + +async function pushBranches(remote: string): Promise { + const out = await simpleGit(remote).raw(['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/']); + return out.split('\n').filter(Boolean); +} + +describe('push publishes the env files env add leaves in a standalone clone (#881)', () => { + let tmpDir: string; + let teamRepo: string; + let remote: string; + let previousExitCode: typeof process.exitCode; + + beforeEach(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-push-env-')); + vi.clearAllMocks(); + mockCreatePullRequest.mockResolvedValue('https://example.test/pr/1'); + ({ teamRepo, remote } = await initTeamRepos(tmpDir)); + const localConfig = { + repo: { localPath: teamRepo, remote }, + username: 'alice', + scope: 'user', + }; + mockDetectProjectConfig.mockResolvedValue(localConfig); + mockAutoDetectInit.mockResolvedValue({ localConfig, teamConfig: { repo: 'acme/team', toolPaths: {} } }); + previousExitCode = process.exitCode; + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + vi.restoreAllMocks(); + process.exitCode = previousExitCode; + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('pushes the env.yaml edit from env add', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + await envAdd('TEAM_VAR', 'changed', {}); + expect(await simpleGit(teamRepo).raw(['status', '--porcelain'])).toContain('env/env.yaml'); + + await push({ all: true }); + + expect(process.exitCode).toBe(previousExitCode); + expect(mockCreatePullRequest).toHaveBeenCalledTimes(1); + const [branch] = await pushBranches(remote); + expect(branch).toBeDefined(); + const pushed = await simpleGit(remote).show([`${branch}:env/env.yaml`]); + expect(pushed).toContain('value: changed'); + }); + + it('keeps the env.yaml edit when the refresh fails after the reset', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + await envAdd('TEAM_VAR', 'changed', {}); + await simpleGit(teamRepo).remote(['set-url', 'origin', path.join(tmpDir, 'missing.git')]); + + await push({ all: true }); + + expect(stderrOutput()).toContain('Pull failed'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); + + it('still refuses when another path is dirty, and keeps the env edit', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + await envAdd('TEAM_VAR', 'changed', {}); + fs.appendFileSync(path.join(teamRepo, 'README.md'), 'local note\n'); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + const errors = stderrOutput(); + expect(errors).toContain('Cannot push: the team repo has uncommitted changes'); + expect(errors).toMatch(/Paths: README\.md$/m); + expect(mockCreatePullRequest).not.toHaveBeenCalled(); + expect(await pushBranches(remote)).toEqual([]); + expect(fs.readFileSync(path.join(teamRepo, 'README.md'), 'utf8')).toContain('local note'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); + + it('still refuses a deleted env.yaml', async () => { + const { push } = await import('../push.js'); + fs.rmSync(path.join(teamRepo, 'env', 'env.yaml')); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Paths: env/env.yaml'); + expect(await pushBranches(remote)).toEqual([]); + }); + + it('still refuses a mode change on env.yaml', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + await git.addConfig('core.fileMode', 'true'); + await envAdd('TEAM_VAR', 'changed', {}); + fs.chmodSync(path.join(teamRepo, 'env', 'env.yaml'), 0o755); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Paths: env/env.yaml'); + expect(await pushBranches(remote)).toEqual([]); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); +}); diff --git a/src/push.ts b/src/push.ts index 0ad93fd72..30e9de668 100644 --- a/src/push.ts +++ b/src/push.ts @@ -346,13 +346,17 @@ function isTeamaiOwnedDirtyPath( filePath: string, pendingTeamConfig: string | null, modeChangedPaths: ReadonlySet, + pendingEnvPaths: ReadonlySet, ): boolean { const normalized = filePath.replaceAll('\\', '/'); // The sync lock is disposable TeamAI state. teamai.yaml is different: it is // safe to restore only when its content was captured above and its mode is // unchanged. Deletion, mode-only, and content+mode changes must stop before - // reset --hard, or the user's change is silently lost (#690 review). + // reset --hard, or the user's change is silently lost (#690 review). The + // env files `env add` left for push follow the same rule; the caller only + // lists the ones it captured with an unchanged mode (#881). if (normalized === '.teamai/.sync-lock') return true; + if (pendingEnvPaths.has(normalized)) return true; return normalized === 'teamai.yaml' && pendingTeamConfig !== null && !modeChangedPaths.has(normalized); @@ -362,9 +366,10 @@ export function collectUnsafeDirtyPaths( status: PushRepoStatus, pendingTeamConfig: string | null, modeChangedPaths: ReadonlySet = new Set(), + pendingEnvPaths: ReadonlySet = new Set(), ): string[] { return collectDirtyPaths(status) - .filter((filePath) => !isTeamaiOwnedDirtyPath(filePath, pendingTeamConfig, modeChangedPaths)); + .filter((filePath) => !isTeamaiOwnedDirtyPath(filePath, pendingTeamConfig, modeChangedPaths, pendingEnvPaths)); } /** @@ -894,10 +899,22 @@ async function pushCore( if (pendingTeamConfig !== null && await hasGitModeChange(git, 'teamai.yaml')) { modeChangedPaths.add('teamai.yaml'); } + // `env add` edits env files in the clone and leaves the commit to push: + // capture what the env scan will push, to restore it after the refresh + // below as teamai.yaml is (#881). A mode change is not captured, so it + // stays dirty and stops the push. + const pendingEnvFiles = new Map(); + for (const item of await getHandler('env').scanLocalForPush(teamConfig, localConfig)) { + const content = await readFileSafe(item.sourcePath); + if (content !== null && !await hasGitModeChange(git, item.relativePath)) { + pendingEnvFiles.set(item.relativePath, content); + } + } const unsafeDirtyPaths = collectUnsafeDirtyPaths( await git.status(), pendingTeamConfig, modeChangedPaths, + new Set(pendingEnvFiles.keys()), ); if (unsafeDirtyPaths.length > 0) { pullSpin.fail( @@ -907,8 +924,16 @@ async function pushCore( process.exitCode = 1; return; } - await resetToCleanMaster(git, repoPath); - await pullRepo(repoPath); + try { + await resetToCleanMaster(git, repoPath); + await pullRepo(repoPath); + } finally { + // Unlike teamai.yaml, nothing later in the run holds these edits, so + // they go back even when the refresh fails after reset --hard. + for (const [relativePath, content] of pendingEnvFiles) { + await writeFile(path.join(repoPath, ...relativePath.split('/')), content); + } + } if (pendingTeamConfig !== null) { // Re-apply the TeamAI-owned config edit after refreshing the default branch. await writeFile(yamlPath, pendingTeamConfig); From 42c4821b254b96d32dbbb706619c1ff40848109c Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Mon, 28 Sep 2026 18:16:28 +0200 Subject: [PATCH 2/5] fix(push): keep staged and rolled-back env edits out of reach of reset (#881) Two review findings on the env exemption from the dirty-clone guard: - A staged env file that was edited again was exempt through its unstaged edit, so reset --hard dropped the staged state. Only a working-copy edit with no staged, conflicted, deleted or renamed entry is captured now; any index state keeps the path unsafe and stops the push. - The captured edits went back only after the initial refresh. A group that committed nothing (the pushGroup rollback, pushRepoBranch's no-change path, or the config-only push after reuse groups) reset the clone and lost them. They are restored after each such group, as teamai.yaml is, and dropped once a group's branch carries them through the env/ sweeper. --- src/__tests__/push-env.test.ts | 34 +++++++++++++++++++++++++++++++ src/push.ts | 37 ++++++++++++++++++++++++++++------ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts index e806d1b45..ca353981a 100644 --- a/src/__tests__/push-env.test.ts +++ b/src/__tests__/push-env.test.ts @@ -157,6 +157,40 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); }); + it('still refuses an env.yaml edit staged before a later edit, and keeps both', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + const envPath = path.join(teamRepo, 'env', 'env.yaml'); + await envAdd('TEAM_VAR', 'staged', {}); + await git.add('env/env.yaml'); + // Edited by hand: a second `env add` would realign the clone first. + fs.writeFileSync(envPath, fs.readFileSync(envPath, 'utf8').replace('value: staged', 'value: changed')); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toMatch(/Paths: env\/env\.yaml$/m); + expect(await pushBranches(remote)).toEqual([]); + expect(await git.show([':env/env.yaml'])).toContain('value: staged'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); + + it('keeps the env.yaml edit when the push rolls the clone back', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + await envAdd('TEAM_VAR', 'changed', {}); + // A local branch of the requested name makes the branch creation throw + // after the copy step, so pushGroup resets and cleans the clone. + await simpleGit(teamRepo).branch(['teamai/taken']); + + await push({ all: true, branch: 'teamai/taken' }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Push failed'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); + it('still refuses a deleted env.yaml', async () => { const { push } = await import('../push.js'); fs.rmSync(path.join(teamRepo, 'env', 'env.yaml')); diff --git a/src/push.ts b/src/push.ts index 30e9de668..347003368 100644 --- a/src/push.ts +++ b/src/push.ts @@ -868,6 +868,15 @@ async function pushCore( // origin/, so resetToCleanMaster/pullRepo (which assume a normal // clone on a branch) are neither needed nor safe — skip them. let pendingTeamConfig: string | null = initialPendingTeamConfig; + // The env edits captured before the refresh below. Their only copy is the + // clone's working tree, so every reset that can run before they are + // committed must be followed by this restore (#881). + const pendingEnvFiles = new Map(); + const restorePendingEnvFiles = async (): Promise => { + for (const [relativePath, content] of pendingEnvFiles) { + await writeFile(path.join(localConfig.repo.localPath, ...relativePath.split('/')), content); + } + }; // Set when the pull below failed: everything read from the clone after this // point is the previous pull's, manifests included. let teamRepoStale = false; @@ -902,16 +911,26 @@ async function pushCore( // `env add` edits env files in the clone and leaves the commit to push: // capture what the env scan will push, to restore it after the refresh // below as teamai.yaml is (#881). A mode change is not captured, so it - // stays dirty and stops the push. - const pendingEnvFiles = new Map(); + // stays dirty and stops the push. So does any index state: the snapshot + // holds only the working copy, and reset --hard would drop a staged, + // conflicted, deleted or renamed entry. + const status = await git.status(); + const indexedPaths = new Set(collectDirtyPaths({ + staged: status.staged, + created: status.created, + conflicted: status.conflicted, + deleted: status.deleted, + renamed: status.renamed, + })); for (const item of await getHandler('env').scanLocalForPush(teamConfig, localConfig)) { + if (indexedPaths.has(item.relativePath)) continue; const content = await readFileSafe(item.sourcePath); if (content !== null && !await hasGitModeChange(git, item.relativePath)) { pendingEnvFiles.set(item.relativePath, content); } } const unsafeDirtyPaths = collectUnsafeDirtyPaths( - await git.status(), + status, pendingTeamConfig, modeChangedPaths, new Set(pendingEnvFiles.keys()), @@ -930,9 +949,7 @@ async function pushCore( } finally { // Unlike teamai.yaml, nothing later in the run holds these edits, so // they go back even when the refresh fails after reset --hard. - for (const [relativePath, content] of pendingEnvFiles) { - await writeFile(path.join(repoPath, ...relativePath.split('/')), content); - } + await restorePendingEnvFiles(); } if (pendingTeamConfig !== null) { // Re-apply the TeamAI-owned config edit after refreshing the default branch. @@ -1658,6 +1675,12 @@ async function pushCore( if (pendingTeamConfig !== null && groupIndex < configGroupIndex) { await writeFile(path.join(localConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig); } + // A group that pushed a branch committed the env edits through the env/ + // sweeper, so they are no longer pending. Any other group may have reset + // and cleaned the clone on the way out (the rollback, or pushRepoBranch's + // no-change path), taking them with it. + if (outcome === 'pushed' || outcome === 'pr-failed') pendingEnvFiles.clear(); + else await restorePendingEnvFiles(); if (outcome === 'failed') { // The branch/PR for earlier groups is already on the remote, so their // records must survive this failure or the next run would duplicate them. @@ -1704,6 +1727,8 @@ async function pushCore( options, anyPrFailed ? undefined : result, ); + // Its no-change path resets the clone too, and it commits teamai.yaml only. + await restorePendingEnvFiles(); return; } From bf5098bc4b30f0a902a46ac69cfe97849c98621b Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Mon, 28 Sep 2026 18:55:21 +0200 Subject: [PATCH 3/5] fix(push): stage only the selected env files and keep edits a failed push committed (#881) Two more review findings on the env exemption: - pushGroup swept the whole env/ directory, so an env edit left out of the selection was published in whichever group pushed first. Each env item is already its own path in pushedFiles, so env/ is no longer swept, and a pushed group drops only its own files from the pending set. The deselected edits stay dirty in the clone. - A git push that failed after the local commit left the clone on that branch, where the restore matched HEAD and left nothing dirty; the next push found no change and switched away. After a failed group the clone now goes back to the default branch before the pending edits are restored, so the next push retries them. --- src/__tests__/push-env.test.ts | 50 +++++++++++++++++++++++++++++++++- src/push.ts | 29 ++++++++++++-------- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts index ca353981a..e52415fc6 100644 --- a/src/__tests__/push-env.test.ts +++ b/src/__tests__/push-env.test.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; import { simpleGit } from 'simple-git'; +import { askSelection } from '../utils/prompt.js'; // Regression for #881: `env add` edits env/env.yaml in a standalone team clone // without committing and leaves the commit to `push`. The #690 dirty-clone @@ -50,10 +51,11 @@ async function initTeamRepos(root: string): Promise<{ teamRepo: string; remote: const teamRepo = path.join(root, 'team-repo'); await simpleGit().init(['--bare', '--initial-branch=main', remote]); - fs.mkdirSync(path.join(seed, 'env'), { recursive: true }); + fs.mkdirSync(path.join(seed, 'env', 'team'), { recursive: true }); fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'version: 1\n'); fs.writeFileSync(path.join(seed, 'README.md'), '# team\n'); fs.writeFileSync(path.join(seed, 'env', 'env.yaml'), 'variables:\n - key: TEAM_VAR\n value: first\n'); + fs.writeFileSync(path.join(seed, 'env', 'team', 'env.yaml'), 'variables:\n - key: TEAM_ONLY\n value: first\n'); const seedGit = simpleGit(seed); await seedGit.init(); await seedGit.addConfig('user.email', 't@t.com'); @@ -191,6 +193,52 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); }); + it('pushes only the selected env file and keeps the deselected edit in the clone', async () => { + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + const rootEnv = path.join(teamRepo, 'env', 'env.yaml'); + const teamEnv = path.join(teamRepo, 'env', 'team', 'env.yaml'); + fs.writeFileSync(rootEnv, fs.readFileSync(rootEnv, 'utf8').replace('value: first', 'value: selected')); + fs.writeFileSync(teamEnv, fs.readFileSync(teamEnv, 'utf8').replace('value: first', 'value: deselected')); + // Entry files list the root first: 1. env.yaml, 2. team/env.yaml. + vi.mocked(askSelection).mockResolvedValueOnce([0]); + + await push({}); + + const [branch] = await pushBranches(remote); + expect(branch).toBeDefined(); + expect(await simpleGit(remote).show([`${branch}:env/env.yaml`])).toContain('value: selected'); + expect(await simpleGit(remote).show([`${branch}:env/team/env.yaml`])).toContain('value: first'); + expect(fs.readFileSync(teamEnv, 'utf8')).toContain('value: deselected'); + expect(await git.raw(['status', '--porcelain'])).toContain('env/team/env.yaml'); + }); + + it('keeps the env.yaml edit on the default branch when git push fails after the commit, and retries it', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + const hook = path.join(remote, 'hooks', 'pre-receive'); + fs.writeFileSync(hook, '#!/bin/sh\necho rejected >&2\nexit 1\n', { mode: 0o755 }); + await envAdd('TEAM_VAR', 'changed', {}); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Push failed'); + expect((await git.revparse(['--abbrev-ref', 'HEAD'])).trim()).toBe('main'); + expect(await git.raw(['status', '--porcelain'])).toContain('env/env.yaml'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + + fs.rmSync(hook); + process.exitCode = previousExitCode; + // A generated name could repeat the failed run's, which is still a local branch. + await push({ all: true, branch: 'teamai/retry' }); + + const [branch] = await pushBranches(remote); + expect(branch).toBeDefined(); + expect(await simpleGit(remote).show([`${branch}:env/env.yaml`])).toContain('value: changed'); + }); + it('still refuses a deleted env.yaml', async () => { const { push } = await import('../push.js'); fs.rmSync(path.join(teamRepo, 'env', 'env.yaml')); diff --git a/src/push.ts b/src/push.ts index 347003368..fa1e38776 100644 --- a/src/push.ts +++ b/src/push.ts @@ -595,11 +595,12 @@ async function pushGroup(args: { } // Create branch, commit, and push. - // Only include "sweeper" directories (rules/, env/) that actually - // exist — otherwise `git add 'rules/'` throws `pathspec did not match - // any files` and the whole push aborts (BUG #1). A team may not have - // rules/ or env/ yet. - const sweeperCandidates = ['rules/', 'env/', '.codebuddy-plugin/']; + // Only include "sweeper" directories (rules/) that actually exist — + // otherwise `git add 'rules/'` throws `pathspec did not match any files` + // and the whole push aborts (BUG #1). A team may not have rules/ yet. + // env/ is not swept: each env item is its own file in pushedFiles, and a + // sweep would publish the env edits the user left out of the selection (#881). + const sweeperCandidates = ['rules/', '.codebuddy-plugin/']; const existingSweepers = await filterExistingTopLevelPaths( localConfig.repo.localPath, sweeperCandidates, @@ -1675,12 +1676,18 @@ async function pushCore( if (pendingTeamConfig !== null && groupIndex < configGroupIndex) { await writeFile(path.join(localConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig); } - // A group that pushed a branch committed the env edits through the env/ - // sweeper, so they are no longer pending. Any other group may have reset - // and cleaned the clone on the way out (the rollback, or pushRepoBranch's - // no-change path), taking them with it. - if (outcome === 'pushed' || outcome === 'pr-failed') pendingEnvFiles.clear(); - else await restorePendingEnvFiles(); + // A group that pushed a branch carries its own env files, so those are no + // longer pending. Any other group may have reset and cleaned the clone on + // the way out (the rollback, or pushRepoBranch's no-change path), taking + // the edits with it. A failure can also leave the clone on the group's + // local branch, where an unpushed commit holds them: go back to the default + // branch first, where the next run finds them again. + if (outcome === 'pushed' || outcome === 'pr-failed') { + for (const item of group.items) pendingEnvFiles.delete(item.relativePath); + } else { + if (outcome === 'failed' && pendingEnvFiles.size > 0) await checkoutMaster(localConfig.repo.localPath); + await restorePendingEnvFiles(); + } if (outcome === 'failed') { // The branch/PR for earlier groups is already on the remote, so their // records must survive this failure or the next run would duplicate them. From f465062e859dc153319bff19926dc28b1721acc7 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Mon, 28 Sep 2026 19:17:59 +0200 Subject: [PATCH 4/5] fix(push): report env edits push cannot put back and keep a failed group's config edit (#881) From an audit of the env capture/restore path: - A restore write that failed was not reported. After the refresh it was caught as "Pull failed" and the push carried on without the edit; after a group it escaped push as a bare error before earlier groups' PR records were saved. The restore now returns the files it could not write, and push stops naming them and the next step, saving state first in the group loop. - A config group whose git push failed after the commit went back to the default branch for its env edits and left the teamai.yaml edit behind on the local branch. The captured config is re-applied there too while no earlier group has pushed it. A new env//env.yaml that the rollback's clean -fd removes already comes back: the writeFile helper recreates the directory. Tests now cover it for the rollback and for a rejected push. --- src/__tests__/push-env.test.ts | 109 +++++++++++++++++++++++++++++++++ src/push.ts | 52 ++++++++++++---- 2 files changed, 149 insertions(+), 12 deletions(-) diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts index e52415fc6..997a1684c 100644 --- a/src/__tests__/push-env.test.ts +++ b/src/__tests__/push-env.test.ts @@ -33,6 +33,25 @@ vi.mock('../config.js', async (importOriginal) => ({ saveStateForScope: vi.fn(() => Promise.resolve()), })); +// Path → writes still allowed before each further write fails, to stand for +// a disk that refuses the restore. +const failingWrites = new Map(); +vi.mock('../utils/fs.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + writeFile: (filePath: string, content: string) => { + const allowed = failingWrites.get(filePath); + if (allowed === undefined) return actual.writeFile(filePath, content); + if (allowed > 0) { + failingWrites.set(filePath, allowed - 1); + return actual.writeFile(filePath, content); + } + return Promise.reject(new Error(`EACCES: permission denied, open '${filePath}'`)); + }, + }; +}); + vi.mock('../read-only.js', () => ({ assertNotReadOnly: vi.fn() })); vi.mock('../utils/pre-push-sync.js', () => ({ syncTeamUpdatesToLocal: vi.fn() })); vi.mock('../utils/prompt.js', () => ({ @@ -73,6 +92,11 @@ async function initTeamRepos(root: string): Promise<{ teamRepo: string; remote: return { teamRepo, remote }; } +/** What log.error printed. */ +function errorOutput(): string { + return vi.mocked(console.error).mock.calls.map((args) => args.map(String).join(' ')).join('\n'); +} + /** What push printed to stderr, where the spinner reports the refusal. */ function stderrOutput(): string { return vi.mocked(process.stderr.write).mock.calls.map(([chunk]) => String(chunk)).join(''); @@ -92,6 +116,7 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 beforeEach(async () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-push-env-')); vi.clearAllMocks(); + failingWrites.clear(); mockCreatePullRequest.mockResolvedValue('https://example.test/pr/1'); ({ teamRepo, remote } = await initTeamRepos(tmpDir)); const localConfig = { @@ -239,6 +264,90 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(await simpleGit(remote).show([`${branch}:env/env.yaml`])).toContain('value: changed'); }); + it('restores a new env file env add --role created when the push rolls the clone back', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + const opsEnv = path.join(teamRepo, 'env', 'ops', 'env.yaml'); + await envAdd('OPS_VAR', 'ops-value', { role: 'ops' }); + expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); + await git.branch(['teamai/taken']); + + await push({ all: true, branch: 'teamai/taken' }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Push failed'); + expect(fs.readFileSync(opsEnv, 'utf8')).toContain('value: ops-value'); + expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); + }); + + it('restores a new env file env add --role created when git push fails after the commit', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + const opsEnv = path.join(teamRepo, 'env', 'ops', 'env.yaml'); + fs.writeFileSync(path.join(remote, 'hooks', 'pre-receive'), '#!/bin/sh\nexit 1\n', { mode: 0o755 }); + await envAdd('OPS_VAR', 'ops-value', { role: 'ops' }); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect((await git.revparse(['--abbrev-ref', 'HEAD'])).trim()).toBe('main'); + expect(fs.readFileSync(opsEnv, 'utf8')).toContain('value: ops-value'); + expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); + }); + + it('keeps the teamai.yaml edit on the default branch when git push fails after the commit', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const git = simpleGit(teamRepo); + fs.writeFileSync(path.join(remote, 'hooks', 'pre-receive'), '#!/bin/sh\nexit 1\n', { mode: 0o755 }); + await envAdd('TEAM_VAR', 'changed', {}); + // After env add, whose refresh would realign the clone and drop it. + fs.appendFileSync(path.join(teamRepo, 'teamai.yaml'), '# local edit\n'); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect((await git.revparse(['--abbrev-ref', 'HEAD'])).trim()).toBe('main'); + expect(fs.readFileSync(path.join(teamRepo, 'teamai.yaml'), 'utf8')).toContain('# local edit'); + expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + }); + + it('stops and names the env file when putting it back after the refresh fails', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + await envAdd('TEAM_VAR', 'changed', {}); + failingWrites.set(path.join(teamRepo, 'env', 'env.yaml'), 0); + + await push({ all: true }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).not.toContain('Pull failed'); + expect(errorOutput()).toMatch(/env\/env\.yaml \(EACCES: permission denied/); + expect(errorOutput()).toContain('teamai env add'); + expect(mockCreatePullRequest).not.toHaveBeenCalled(); + }); + + it('stops and names the env file when putting it back after a group fails', async () => { + const { envAdd } = await import('../env-commands.js'); + const { push } = await import('../push.js'); + const { saveStateForScope } = await import('../config.js'); + await envAdd('TEAM_VAR', 'changed', {}); + await simpleGit(teamRepo).branch(['teamai/taken']); + // The restore after the refresh succeeds; the one after the rollback fails. + failingWrites.set(path.join(teamRepo, 'env', 'env.yaml'), 1); + + await push({ all: true, branch: 'teamai/taken' }); + + expect(process.exitCode).toBe(1); + expect(stderrOutput()).toContain('Push failed'); + expect(errorOutput()).toMatch(/env\/env\.yaml \(EACCES: permission denied/); + expect(errorOutput()).toContain('teamai env add'); + // Earlier groups' PR records are saved before stopping, as for any failed group. + expect(vi.mocked(saveStateForScope)).toHaveBeenCalled(); + }); + it('still refuses a deleted env.yaml', async () => { const { push } = await import('../push.js'); fs.rmSync(path.join(teamRepo, 'env', 'env.yaml')); diff --git a/src/push.ts b/src/push.ts index fa1e38776..3e5765d6f 100644 --- a/src/push.ts +++ b/src/push.ts @@ -873,11 +873,27 @@ async function pushCore( // clone's working tree, so every reset that can run before they are // committed must be followed by this restore (#881). const pendingEnvFiles = new Map(); - const restorePendingEnvFiles = async (): Promise => { + /** Write the captured env edits back; returns each one it could not, with the reason. */ + const restorePendingEnvFiles = async (): Promise => { + const lost: string[] = []; for (const [relativePath, content] of pendingEnvFiles) { - await writeFile(path.join(localConfig.repo.localPath, ...relativePath.split('/')), content); + try { + await writeFile(path.join(localConfig.repo.localPath, ...relativePath.split('/')), content); + } catch (e) { + lost.push(`${relativePath} (${(e as Error).message})`); + } } + return lost; + }; + const reportLostEnvFiles = (lost: string[]): void => { + log.error( + `Could not put back ${lost.join(', ')} after resetting the team repo, so that env edit is no longer in ` + + 'the clone. Nothing more was pushed. Run `teamai env add` again for the variables it held, then push.', + ); + process.exitCode = 1; }; + // Env files the refresh below could not put back; the push stops on any. + let lostEnvFiles: string[] = []; // Set when the pull below failed: everything read from the clone after this // point is the previous pull's, manifests included. let teamRepoStale = false; @@ -950,7 +966,7 @@ async function pushCore( } finally { // Unlike teamai.yaml, nothing later in the run holds these edits, so // they go back even when the refresh fails after reset --hard. - await restorePendingEnvFiles(); + lostEnvFiles = await restorePendingEnvFiles(); } if (pendingTeamConfig !== null) { // Re-apply the TeamAI-owned config edit after refreshing the default branch. @@ -961,6 +977,10 @@ async function pushCore( teamRepoStale = true; pullSpin.warn(`Pull failed: ${(e as Error).message}`); } + if (lostEnvFiles.length > 0) { + reportLostEnvFiles(lostEnvFiles); + return; + } } // Validate the refreshed checkout, not the stale local clone. A pull can @@ -1669,29 +1689,36 @@ async function pushCore( includeTeamConfig: groupIndex === configGroupIndex, branch: options.branch, }); + // A failure can leave the clone on the group's local branch, where an + // unpushed commit holds the config and env edits: go back to the default + // branch first, where the next run finds them again. + const configPending = pendingTeamConfig !== null && groupIndex <= configGroupIndex; + if (outcome === 'failed' && (configPending || pendingEnvFiles.size > 0)) { + await checkoutMaster(localConfig.repo.localPath); + } // A preceding reuse group may take the metadata-only path in // pushRepoBranch(), which resets and cleans the clone. Re-apply the // captured config before the new explicit-branch group runs, or that - // cleanup would silently discard the user's edit (#800). - if (pendingTeamConfig !== null && groupIndex < configGroupIndex) { + // cleanup would silently discard the user's edit (#800). The same goes + // for the config group itself when it failed. + if (pendingTeamConfig !== null && configPending && (groupIndex < configGroupIndex || outcome === 'failed')) { await writeFile(path.join(localConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig); } // A group that pushed a branch carries its own env files, so those are no // longer pending. Any other group may have reset and cleaned the clone on // the way out (the rollback, or pushRepoBranch's no-change path), taking - // the edits with it. A failure can also leave the clone on the group's - // local branch, where an unpushed commit holds them: go back to the default - // branch first, where the next run finds them again. + // the edits with it. + let lost: string[] = []; if (outcome === 'pushed' || outcome === 'pr-failed') { for (const item of group.items) pendingEnvFiles.delete(item.relativePath); } else { - if (outcome === 'failed' && pendingEnvFiles.size > 0) await checkoutMaster(localConfig.repo.localPath); - await restorePendingEnvFiles(); + lost = await restorePendingEnvFiles(); } - if (outcome === 'failed') { + if (outcome === 'failed' || lost.length > 0) { // The branch/PR for earlier groups is already on the remote, so their // records must survive this failure or the next run would duplicate them. await saveStateForScope(pushState, localConfig); + if (lost.length > 0) reportLostEnvFiles(lost); process.exitCode = 1; return; } @@ -1735,7 +1762,8 @@ async function pushCore( anyPrFailed ? undefined : result, ); // Its no-change path resets the clone too, and it commits teamai.yaml only. - await restorePendingEnvFiles(); + const lost = await restorePendingEnvFiles(); + if (lost.length > 0) reportLostEnvFiles(lost); return; } From b3cb0a4febe89df97d2bf5f2e18b4b94e5622267 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Mon, 28 Sep 2026 19:47:41 +0200 Subject: [PATCH 5/5] fix(push): restore env edits byte for byte with their mode, after every group (#881) From the adversarial review: - The snapshot held decoded UTF-8 text and no mode. A new env//env.yaml set to 0600 came back 0644 after clean -fd (git records no mode for an untracked file, and reset --hard recreates a tracked one under the umask too), and a hand edit that is not UTF-8 came back re-encoded. It now keeps the bytes and the permission bits and restores both. - The restore ran only after a failed or no-change group. A reuse group retrying its missing PR takes pushRepoBranch's metadata-only reset path and still reports pushed, so a pending env edit was wiped with no restore. The restore now runs after every group, once a pushed group's own files are dropped from the pending set. - The pushItem and filterExistingTopLevelPaths comments no longer describe an env/ sweeper. Tests cover both modes, the non-UTF-8 bytes, the reuse retry, and the restores after a no-change group and after the config-only push. --- src/__tests__/push-env.test.ts | 102 +++++++++++++++++++++++++++++++-- src/push.ts | 43 +++++++++----- src/resources/env.ts | 6 +- src/utils/fs.ts | 4 +- 4 files changed, 130 insertions(+), 25 deletions(-) diff --git a/src/__tests__/push-env.test.ts b/src/__tests__/push-env.test.ts index 997a1684c..2dead29b1 100644 --- a/src/__tests__/push-env.test.ts +++ b/src/__tests__/push-env.test.ts @@ -14,6 +14,8 @@ import { askSelection } from '../utils/prompt.js'; const mockCreatePullRequest = vi.fn().mockResolvedValue('https://example.test/pr/1'); const mockAutoDetectInit = vi.fn(); const mockDetectProjectConfig = vi.fn(); +const freshState = (): unknown => ({ lastPush: null, pushedSkills: [], pushedRules: [], pushedEnvVars: [] }); +let storedState = freshState(); vi.mock('../providers/index.js', () => ({ getProvider: () => ({ @@ -27,10 +29,12 @@ vi.mock('../config.js', async (importOriginal) => ({ ...(await importOriginal()), autoDetectInit: (...args: unknown[]) => mockAutoDetectInit(...args), detectProjectConfig: (...args: unknown[]) => mockDetectProjectConfig(...args), - loadStateForScope: vi.fn(() => Promise.resolve({ - lastPush: null, pushedSkills: [], pushedRules: [], pushedEnvVars: [], - })), - saveStateForScope: vi.fn(() => Promise.resolve()), + // One state file per test, so a second push sees the first one's records. + loadStateForScope: vi.fn(() => Promise.resolve(structuredClone(storedState))), + saveStateForScope: vi.fn((state: unknown) => { + storedState = structuredClone(state); + return Promise.resolve(); + }), })); // Path → writes still allowed before each further write fails, to stand for @@ -117,6 +121,7 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-push-env-')); vi.clearAllMocks(); failingWrites.clear(); + storedState = freshState(); mockCreatePullRequest.mockResolvedValue('https://example.test/pr/1'); ({ teamRepo, remote } = await initTeamRepos(tmpDir)); const localConfig = { @@ -203,10 +208,13 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); }); - it('keeps the env.yaml edit when the push rolls the clone back', async () => { + it('keeps the env.yaml edit and its mode when the push rolls the clone back', async () => { const { envAdd } = await import('../env-commands.js'); const { push } = await import('../push.js'); + const envPath = path.join(teamRepo, 'env', 'env.yaml'); await envAdd('TEAM_VAR', 'changed', {}); + // Git sees no mode change in 0600, but reset --hard recreates the file 0644. + fs.chmodSync(envPath, 0o600); // A local branch of the requested name makes the branch creation throw // after the copy step, so pushGroup resets and cleans the clone. await simpleGit(teamRepo).branch(['teamai/taken']); @@ -215,7 +223,8 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(process.exitCode).toBe(1); expect(stderrOutput()).toContain('Push failed'); - expect(fs.readFileSync(path.join(teamRepo, 'env', 'env.yaml'), 'utf8')).toContain('value: changed'); + expect(fs.readFileSync(envPath, 'utf8')).toContain('value: changed'); + expect(fs.statSync(envPath).mode & 0o777).toBe(0o600); }); it('pushes only the selected env file and keeps the deselected edit in the clone', async () => { @@ -271,6 +280,8 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 const opsEnv = path.join(teamRepo, 'env', 'ops', 'env.yaml'); await envAdd('OPS_VAR', 'ops-value', { role: 'ops' }); expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); + // Git records no mode for an untracked file, so only the snapshot can keep it. + fs.chmodSync(opsEnv, 0o600); await git.branch(['teamai/taken']); await push({ all: true, branch: 'teamai/taken' }); @@ -278,6 +289,7 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(process.exitCode).toBe(1); expect(stderrOutput()).toContain('Push failed'); expect(fs.readFileSync(opsEnv, 'utf8')).toContain('value: ops-value'); + expect(fs.statSync(opsEnv).mode & 0o777).toBe(0o600); expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); }); @@ -288,12 +300,14 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 const opsEnv = path.join(teamRepo, 'env', 'ops', 'env.yaml'); fs.writeFileSync(path.join(remote, 'hooks', 'pre-receive'), '#!/bin/sh\nexit 1\n', { mode: 0o755 }); await envAdd('OPS_VAR', 'ops-value', { role: 'ops' }); + fs.chmodSync(opsEnv, 0o600); await push({ all: true }); expect(process.exitCode).toBe(1); expect((await git.revparse(['--abbrev-ref', 'HEAD'])).trim()).toBe('main'); expect(fs.readFileSync(opsEnv, 'utf8')).toContain('value: ops-value'); + expect(fs.statSync(opsEnv).mode & 0o777).toBe(0o600); expect(await git.raw(['status', '--porcelain', '--untracked-files=all'])).toContain('?? env/ops/env.yaml'); }); @@ -348,6 +362,82 @@ describe('push publishes the env files env add leaves in a standalone clone (#88 expect(vi.mocked(saveStateForScope)).toHaveBeenCalled(); }); + it('keeps the bytes of an env file that is not UTF-8', async () => { + const { push } = await import('../push.js'); + const envPath = path.join(teamRepo, 'env', 'env.yaml'); + // A hand edit saved as Latin-1: 0xE9 is "é" there and invalid UTF-8. + const latin1 = Buffer.from('variables:\n - key: TEAM_VAR\n value: caf\xe9\n', 'latin1'); + fs.writeFileSync(envPath, latin1); + await simpleGit(teamRepo).branch(['teamai/taken']); + + await push({ all: true, branch: 'teamai/taken' }); + + expect(process.exitCode).toBe(1); + expect(fs.readFileSync(envPath).equals(latin1)).toBe(true); + }); + + it('keeps a deselected env edit when a group with no change rolls the clone back', async () => { + const { push } = await import('../push.js'); + const rootEnv = path.join(teamRepo, 'env', 'env.yaml'); + fs.writeFileSync(rootEnv, fs.readFileSync(rootEnv, 'utf8').replace('value: first', 'value: deselected')); + // A blank line only: pushRepoBranch reads it as metadata, resets and cleans. + fs.appendFileSync(path.join(teamRepo, 'env', 'team', 'env.yaml'), '\n'); + vi.mocked(askSelection).mockResolvedValueOnce([1]); + + await push({}); + + expect(await pushBranches(remote)).toEqual([]); + expect(fs.readFileSync(rootEnv, 'utf8')).toContain('value: deselected'); + }); + + /** Push an env/team/env.yaml edit whose PR creation fails, leaving a reuse record with prUrl null. */ + async function pushWithoutPr(): Promise { + const { push } = await import('../push.js'); + const teamEnv = path.join(teamRepo, 'env', 'team', 'env.yaml'); + fs.writeFileSync(teamEnv, fs.readFileSync(teamEnv, 'utf8').replace('value: first', 'value: reviewed')); + mockCreatePullRequest.mockResolvedValue(null); + await push({ all: true }); + expect(await pushBranches(remote)).toHaveLength(1); + expect(fs.readFileSync(teamEnv, 'utf8')).toContain('value: first'); + process.exitCode = previousExitCode; + vi.mocked(process.stderr.write).mockClear(); + } + + it('keeps a deselected env edit when a reuse group that retries its PR rolls the clone back', async () => { + const { push } = await import('../push.js'); + await pushWithoutPr(); + const rootEnv = path.join(teamRepo, 'env', 'env.yaml'); + fs.writeFileSync(rootEnv, fs.readFileSync(rootEnv, 'utf8').replace('value: first', 'value: deselected')); + // The recorded file now differs from main by a blank line only: the reuse + // branch is rebuilt from main, reads that as metadata, resets and cleans + // the clone, then retries the missing PR. + fs.appendFileSync(path.join(teamRepo, 'env', 'team', 'env.yaml'), '\n'); + vi.mocked(askSelection).mockResolvedValueOnce([1]); + mockCreatePullRequest.mockClear(); + + await push({}); + + expect(mockCreatePullRequest).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(rootEnv, 'utf8')).toContain('value: deselected'); + }); + + it('keeps a deselected env edit when the config-only push after the reuse groups has no change', async () => { + const { push } = await import('../push.js'); + await pushWithoutPr(); + const rootEnv = path.join(teamRepo, 'env', 'env.yaml'); + const teamEnv = path.join(teamRepo, 'env', 'team', 'env.yaml'); + fs.writeFileSync(rootEnv, fs.readFileSync(rootEnv, 'utf8').replace('value: first', 'value: deselected')); + fs.writeFileSync(teamEnv, fs.readFileSync(teamEnv, 'utf8').replace('value: first', 'value: reviewed again')); + // A blank line only: pushTeamConfigOnly's pushRepoBranch resets and cleans. + fs.appendFileSync(path.join(teamRepo, 'teamai.yaml'), '\n'); + vi.mocked(askSelection).mockResolvedValueOnce([1]); + + await push({ branch: 'teamai/config' }); + + expect(stderrOutput()).toContain('No changes to push (config already up to date)'); + expect(fs.readFileSync(rootEnv, 'utf8')).toContain('value: deselected'); + }); + it('still refuses a deleted env.yaml', async () => { const { push } = await import('../push.js'); fs.rmSync(path.join(teamRepo, 'env', 'env.yaml')); diff --git a/src/push.ts b/src/push.ts index 3e5765d6f..c8b8f1fa3 100644 --- a/src/push.ts +++ b/src/push.ts @@ -1,3 +1,4 @@ +import { chmod, readFile, stat } from 'node:fs/promises'; import path from 'node:path'; import YAML from 'yaml'; import { autoDetectInit, loadStateForScope, saveStateForScope } from './config.js'; @@ -34,11 +35,11 @@ import { pathExists, pruneEmptyDirs, readFileSafe, writeFile } from './utils/fs. import { brokenTeamProfileFiles } from './models/profile.js'; /** - * Filter a list of repo-root-relative paths (e.g. "rules/", "env/") down to + * Filter a list of repo-root-relative paths (e.g. "rules/", ".codebuddy-plugin/") down to * those that actually exist on disk. `git add` throws `pathspec did not match * any files` when any argument doesn't exist, so we guard against that when * passing "sweeper" directories that may or may not be present in a given - * team repo (e.g. a pure-wiki team has no rules/ or env/). + * team repo (e.g. a pure-wiki team has no rules/). */ export async function filterExistingTopLevelPaths( repoPath: string, @@ -342,6 +343,16 @@ async function hasGitModeChange( } } +/** A file's bytes and permission bits, or null when it cannot be read. */ +async function readFileSnapshot(filePath: string): Promise<{ content: Buffer; mode: number } | null> { + try { + const [content, stats] = await Promise.all([readFile(filePath), stat(filePath)]); + return { content, mode: stats.mode & 0o777 }; + } catch { + return null; + } +} + function isTeamaiOwnedDirtyPath( filePath: string, pendingTeamConfig: string | null, @@ -872,13 +883,18 @@ async function pushCore( // The env edits captured before the refresh below. Their only copy is the // clone's working tree, so every reset that can run before they are // committed must be followed by this restore (#881). - const pendingEnvFiles = new Map(); + // Bytes and permission bits: a hand edit need not be UTF-8, and git records + // no mode for an untracked file (a new env//env.yaml), so clean -fd + // plus a plain write would recreate a 0600 file under the umask. + const pendingEnvFiles = new Map(); /** Write the captured env edits back; returns each one it could not, with the reason. */ const restorePendingEnvFiles = async (): Promise => { const lost: string[] = []; - for (const [relativePath, content] of pendingEnvFiles) { + for (const [relativePath, { content, mode }] of pendingEnvFiles) { + const target = path.join(localConfig.repo.localPath, ...relativePath.split('/')); try { - await writeFile(path.join(localConfig.repo.localPath, ...relativePath.split('/')), content); + await writeFile(target, content); + await chmod(target, mode); } catch (e) { lost.push(`${relativePath} (${(e as Error).message})`); } @@ -941,9 +957,9 @@ async function pushCore( })); for (const item of await getHandler('env').scanLocalForPush(teamConfig, localConfig)) { if (indexedPaths.has(item.relativePath)) continue; - const content = await readFileSafe(item.sourcePath); - if (content !== null && !await hasGitModeChange(git, item.relativePath)) { - pendingEnvFiles.set(item.relativePath, content); + const snapshot = await readFileSnapshot(item.sourcePath); + if (snapshot !== null && !await hasGitModeChange(git, item.relativePath)) { + pendingEnvFiles.set(item.relativePath, snapshot); } } const unsafeDirtyPaths = collectUnsafeDirtyPaths( @@ -1705,15 +1721,14 @@ async function pushCore( await writeFile(path.join(localConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig); } // A group that pushed a branch carries its own env files, so those are no - // longer pending. Any other group may have reset and cleaned the clone on - // the way out (the rollback, or pushRepoBranch's no-change path), taking - // the edits with it. - let lost: string[] = []; + // longer pending. Any group may have reset and cleaned the clone on the way + // out (the rollback, or pushRepoBranch's no-change path, which a reuse group + // retrying its PR also takes before it reports pushed), so the rest go back + // whatever the outcome; rewriting a file still in place changes nothing. if (outcome === 'pushed' || outcome === 'pr-failed') { for (const item of group.items) pendingEnvFiles.delete(item.relativePath); - } else { - lost = await restorePendingEnvFiles(); } + const lost = await restorePendingEnvFiles(); if (outcome === 'failed' || lost.length > 0) { // The branch/PR for earlier groups is already on the remote, so their // records must survive this failure or the next run would duplicate them. diff --git a/src/resources/env.ts b/src/resources/env.ts index 2b386e133..d35718136 100644 --- a/src/resources/env.ts +++ b/src/resources/env.ts @@ -274,13 +274,13 @@ export class EnvHandler extends ResourceHandler { } async pushItem(item: ResourceItem, _teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise { - // Non-self modes: env files already live in the repo dir; push.ts commits - // them via the env/ sweeper — nothing to copy. + // Non-self modes: env files already live in the repo dir; push.ts stages + // each selected one by its path — nothing to copy. // // Single-repo mode: the source is the ACTIVE tree's .teamai/env/ file, but // the commit happens in the knowledge worktree (localConfig.repo.localPath). // Copy the active copy into the worktree so the PR actually carries the change; - // otherwise the env/ sweeper would commit the stale baseline. (Guarded on the + // otherwise staging that path would commit the stale baseline. (Guarded on the // paths differing so non-self stays a no-op.) if (isSelfMode(localConfig)) { const dest = path.join(localConfig.repo.localPath, ...item.relativePath.split('/')); diff --git a/src/utils/fs.ts b/src/utils/fs.ts index ae094a7ca..290e0803a 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -52,9 +52,9 @@ export async function readFileIfExists(filePath: string): Promise } /** - * Write a file, creating parent dirs as needed. + * Write a file, creating parent dirs as needed. Bytes are written as they are. */ -export async function writeFile(filePath: string, content: string): Promise { +export async function writeFile(filePath: string, content: string | Uint8Array): Promise { const expanded = expandHome(filePath); await fse.ensureDir(path.dirname(expanded)); await fse.writeFile(expanded, content, 'utf-8');