Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 202 additions & 0 deletions src/__tests__/push-role-skill-e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,182 @@ describe('push branch and dirty-clone e2e (issue #663)', () => {
}
}, 60_000);

it('routes config changes to the explicit new branch when an existing PR is also updated', async () => {
const fixture = makePushFixture('git', 'https://git.example.test/team/issue-663-mixed.git', 'claude');
try {
const teamRepo = path.join(fixture.projectRoot, '.teamai', 'team-repo');
git(['config', 'core.autocrlf', 'false'], teamRepo);
git(['checkout', '--', '.'], teamRepo);
const first = await pullModifyAndPush(fixture, 'claude');
expect(first.output).toContain('Pushed branch teamai/push/issue-331-git/');
const existingBranch = git(
['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/push/issue-331-git/'],
fixture.remote,
);
expect(existingBranch).toMatch(/^teamai\/push\/issue-331-git\//);

const statePath = path.join(fixture.projectRoot, '.teamai', 'state.json');
const state = JSON.parse(fs.readFileSync(statePath, 'utf8')) as {
pendingPushes: Array<{ branch: string; prUrl: string | null }>;
};
const pending = state.pendingPushes.find((entry) => entry.branch === existingBranch);
expect(pending).toBeDefined();
if (!pending) return;
pending.prUrl = 'https://github.com/team/issue-663-mixed/pull/663';
fs.writeFileSync(statePath, JSON.stringify(state, null, 2) + '\n');

fs.writeFileSync(
path.join(fixture.projectRoot, '.claude', 'skills', 'beta-proof', 'SKILL.md'),
'---\nname: beta-proof\ndescription: modified again\n---\n\n# Modified again\n',
);
const newSkillDir = path.join(fixture.projectRoot, '.claude', 'skills', 'gamma-proof');
fs.mkdirSync(newSkillDir, { recursive: true });
fs.writeFileSync(
path.join(newSkillDir, 'SKILL.md'),
'---\nname: gamma-proof\ndescription: new\n---\n\n# New skill\n',
);
fs.appendFileSync(path.join(teamRepo, 'teamai.yaml'), '\npublicSkills: []\n');

const second = await runCLI(
['push', '--all', '--branch', 'feature/explicit-mixed'],
fixture.projectRoot,
fixture.home,
);
expect(second.code, second.output).not.toBe(0);
expect(second.output)
.toContain('Existing PR updated: https://github.com/team/issue-663-mixed/pull/663');
expect(second.output).toContain('Pushed branch feature/explicit-mixed');
expect(git(['show', `${existingBranch}:teamai.yaml`], fixture.remote))
.not.toContain('publicSkills: []');
expect(git(['show', 'feature/explicit-mixed:teamai.yaml'], fixture.remote))
.toContain('publicSkills: []');
expect(git(['show', 'feature/explicit-mixed:skills/backend/gamma-proof/SKILL.md'], fixture.remote))
.toContain('# New skill');
} finally {
fs.rmSync(fixture.sandbox, { recursive: true, force: true });
}
}, 60_000);

it('preserves config after a metadata-only existing-PR group before the explicit new branch', async () => {
const fixture = makePushFixture('git', 'https://git.example.test/team/issue-800-metadata.git', 'claude');
try {
const seed = path.join(fixture.sandbox, 'seed');
fs.mkdirSync(path.join(seed, 'rules', 'backend'), { recursive: true });
fs.writeFileSync(
path.join(seed, 'rules', 'backend', 'beta-rule.md'),
'---\ntitle: beta-rule\n---\n\n# Original rule\n',
);
const seedConfigPath = path.join(seed, 'teamai.yaml');
fs.writeFileSync(
seedConfigPath,
fs.readFileSync(seedConfigPath, 'utf8').replace(
' skills: .claude/skills',
' skills: .claude/skills\n rules: .claude/rules',
),
);
git(['add', 'teamai.yaml', 'rules/backend/beta-rule.md'], seed);
git(['commit', '-q', '-m', 'add rule fixture'], seed);
git(['push', '-q', fixture.remote, 'main'], seed);

const pulled = await runCLI(['pull'], fixture.projectRoot, fixture.home);
expect(pulled.code, pulled.output).toBe(0);
const rulePath = path.join(fixture.projectRoot, '.claude', 'rules', 'backend', 'beta-rule.md');
expect(fs.existsSync(rulePath)).toBe(true);
fs.writeFileSync(rulePath, '---\ntitle: beta-rule\n---\n\n# Modified rule\n');

const first = await runCLI(['push', '--all'], fixture.projectRoot, fixture.home);
expect(first.code, first.output).not.toBe(0);
expect(first.output).toContain('Pushed branch teamai/push/issue-331-git/');
const existingBranch = git(
['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/push/issue-331-git/'],
fixture.remote,
);
expect(existingBranch).toMatch(/^teamai\/push\/issue-331-git\//);

const statePath = path.join(fixture.projectRoot, '.teamai', 'state.json');
const state = JSON.parse(fs.readFileSync(statePath, 'utf8')) as {
pendingPushes: Array<{ branch: string; prUrl: string | null }>;
};
const pending = state.pendingPushes.find((entry) => entry.branch === existingBranch);
expect(pending).toBeDefined();
if (!pending) return;
pending.prUrl = 'https://github.com/team/issue-800-metadata/pull/800';
fs.writeFileSync(statePath, JSON.stringify(state, null, 2) + '\n');

fs.writeFileSync(
rulePath,
'---\ntitle: beta-rule\nlastUpdated: 2026-09-24T00:00:00.000Z\n---\n\n# Original rule\n',
);
const newSkillDir = path.join(fixture.projectRoot, '.claude', 'skills', 'gamma-proof');
fs.mkdirSync(newSkillDir, { recursive: true });
fs.writeFileSync(
path.join(newSkillDir, 'SKILL.md'),
'---\nname: gamma-proof\ndescription: new\n---\n\n# New skill\n',
);
fs.appendFileSync(path.join(fixture.projectRoot, '.teamai', 'team-repo', 'teamai.yaml'), '\npublicSkills: []\n');

const second = await runCLI(
['push', '--all', '--branch', 'feature/explicit-metadata'],
fixture.projectRoot,
fixture.home,
);
expect(second.code, second.output).not.toBe(0);
expect(second.output).toContain('Pushed branch feature/explicit-metadata');
expect(git(['show', 'feature/explicit-metadata:teamai.yaml'], fixture.remote))
.toContain('publicSkills: []');
expect(git(['show', `${existingBranch}:teamai.yaml`], fixture.remote))
.not.toContain('publicSkills: []');
} finally {
fs.rmSync(fixture.sandbox, { recursive: true, force: true });
}
}, 60_000);

it('keeps config off an existing PR when --branch has no new resource group', async () => {
const fixture = makePushFixture('git', 'https://git.example.test/team/issue-800-config-only.git', 'claude');
try {
const first = await pullModifyAndPush(fixture, 'claude');
expect(first.output).toContain('Pushed branch teamai/push/issue-331-git/');
const existingBranch = git(
['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/push/issue-331-git/'],
fixture.remote,
);
expect(existingBranch).toMatch(/^teamai\/push\/issue-331-git\//);

const statePath = path.join(fixture.projectRoot, '.teamai', 'state.json');
const state = JSON.parse(fs.readFileSync(statePath, 'utf8')) as {
pendingPushes: Array<{ branch: string; prUrl: string | null }>;
};
const pending = state.pendingPushes.find((entry) => entry.branch === existingBranch);
expect(pending).toBeDefined();
if (!pending) return;
pending.prUrl = 'https://github.com/team/issue-800-config-only/pull/800';
fs.writeFileSync(statePath, JSON.stringify(state, null, 2) + '\n');

fs.writeFileSync(
path.join(fixture.projectRoot, '.claude', 'skills', 'beta-proof', 'SKILL.md'),
'---\nname: beta-proof\ndescription: modified again\n---\n\n# Modified again\n',
);
const teamRepo = path.join(fixture.projectRoot, '.teamai', 'team-repo');
fs.appendFileSync(path.join(teamRepo, 'teamai.yaml'), '\npublicSkills: []\n');

const second = await runCLI(
['push', '--all', '--branch', 'feature/explicit-config-only'],
fixture.projectRoot,
fixture.home,
);
expect(second.code, second.output).not.toBe(0);
expect(second.output)
.toContain('Existing PR updated: https://github.com/team/issue-800-config-only/pull/800');
expect(second.output).toContain('Pushed branch feature/explicit-config-only');
expect(git(['show', `${existingBranch}:teamai.yaml`], fixture.remote))
.not.toContain('publicSkills: []');
expect(git(['show', 'feature/explicit-config-only:teamai.yaml'], fixture.remote))
.toContain('publicSkills: []');
} finally {
fs.rmSync(fixture.sandbox, { recursive: true, force: true });
}
}, 60_000);

it('refuses a mode-only teamai.yaml change before reset --hard', async () => {
const fixture = makePushFixture('git', 'https://git.example.test/team/issue-663-dirty.git', 'claude');
try {
Expand All @@ -491,4 +667,30 @@ describe('push branch and dirty-clone e2e (issue #663)', () => {
fs.rmSync(fixture.sandbox, { recursive: true, force: true });
}
}, 60_000);

it('refuses a content-and-mode teamai.yaml change before reset --hard', async () => {
const fixture = makePushFixture('git', 'https://git.example.test/team/issue-663-dirty-content.git', 'claude');
try {
const teamRepo = path.join(fixture.projectRoot, '.teamai', 'team-repo');
git(['config', 'core.autocrlf', 'false'], teamRepo);
git(['checkout', '--', 'teamai.yaml'], teamRepo);
const pulled = await runCLI(['pull'], fixture.projectRoot, fixture.home);
expect(pulled.code, pulled.output).toBe(0);
fs.appendFileSync(path.join(teamRepo, 'teamai.yaml'), '\npublicSkills: []\n');
git(['config', 'core.fileMode', 'true'], teamRepo);
git(['update-index', '--chmod=+x', 'teamai.yaml'], teamRepo);
expect(git(['status', '--short'], teamRepo)).toContain('teamai.yaml');

const result = await runCLI(['push', '--all'], fixture.projectRoot, fixture.home);
expect(result.code, result.output).not.toBe(0);
expect(result.output).toContain('Cannot push: the team repo has uncommitted changes');
expect(result.output).toContain('teamai.yaml');
expect(git(
['for-each-ref', '--format=%(refname:short)', 'refs/heads/teamai/push/'],
fixture.remote,
)).toBe('');
} finally {
fs.rmSync(fixture.sandbox, { recursive: true, force: true });
}
}, 60_000);
});
5 changes: 4 additions & 1 deletion src/__tests__/push-role.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ describe('team repo dirty-path guard', () => {
renamed: [],
};

it('allows teamai.yaml only when its content was captured', () => {
it('allows teamai.yaml only when its content was captured and its mode is unchanged', () => {
expect(collectUnsafeDirtyPaths({ ...cleanStatus, modified: ['teamai.yaml'] }, 'edited')).toEqual([]);
expect(collectUnsafeDirtyPaths({ ...cleanStatus, modified: ['teamai.yaml'] }, 'edited', new Set(['teamai.yaml'])))
.toEqual(['teamai.yaml']);
expect(collectUnsafeDirtyPaths({ ...cleanStatus, modified: ['teamai.yaml'] }, null)).toEqual(['teamai.yaml']);
});

Expand Down Expand Up @@ -81,6 +83,7 @@ const mockGitStatus = vi.fn().mockResolvedValue({
});
const mockCreateGit = vi.fn().mockReturnValue({
status: mockGitStatus,
raw: vi.fn().mockResolvedValue(''),
merge: mockMerge,
stash: mockStash,
});
Expand Down
97 changes: 79 additions & 18 deletions src/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,22 +317,50 @@ function collectDirtyPaths(status: PushRepoStatus): string[] {
return [...paths].sort();
}

function isTeamaiOwnedDirtyPath(filePath: string, pendingTeamConfig: string | null): boolean {
async function hasGitModeChange(
git: { raw?: (args: string[]) => Promise<string> },
filePath: string,
): Promise<boolean> {
// The content snapshot is enough only when the file mode is unchanged. Check
// both the index and worktree diffs because a chmod can be staged, unstaged,
// or both alongside a content edit (#690 review).
if (typeof git.raw !== 'function') return false;
try {
const [worktreeDiff, indexDiff] = await Promise.all([
git.raw(['diff', '--summary', '--', filePath]),
git.raw(['diff', '--cached', '--summary', '--', filePath]),
]);
return [worktreeDiff, indexDiff].some((diff) => /mode change \d+ => \d+/.test(diff));
} catch {
// If Git cannot prove that metadata is unchanged, stop before reset rather
// than risk discarding a mode change that was not captured.
return true;
}
}

function isTeamaiOwnedDirtyPath(
filePath: string,
pendingTeamConfig: string | null,
modeChangedPaths: ReadonlySet<string>,
): boolean {
const normalized = filePath.replaceAll('\\', '/');
// The sync lock is disposable TeamAI state. teamai.yaml is different: it is
// safe to restore only when its working-tree content was captured above.
// Deletion and mode-only changes leave pendingTeamConfig null and must stop
// before reset --hard, or the user's change is silently lost (#690 review).
// 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).
if (normalized === '.teamai/.sync-lock') return true;
return normalized === 'teamai.yaml' && pendingTeamConfig !== null;
return normalized === 'teamai.yaml'
&& pendingTeamConfig !== null
&& !modeChangedPaths.has(normalized);
}

export function collectUnsafeDirtyPaths(
status: PushRepoStatus,
pendingTeamConfig: string | null,
modeChangedPaths: ReadonlySet<string> = new Set(),
): string[] {
return collectDirtyPaths(status)
.filter((filePath) => !isTeamaiOwnedDirtyPath(filePath, pendingTeamConfig));
.filter((filePath) => !isTeamaiOwnedDirtyPath(filePath, pendingTeamConfig, modeChangedPaths));
}

/**
Expand Down Expand Up @@ -859,7 +887,15 @@ async function pushCore(
pendingTeamConfig = workingContent;
}
}
const unsafeDirtyPaths = collectUnsafeDirtyPaths(await git.status(), pendingTeamConfig);
const modeChangedPaths = new Set<string>();
if (pendingTeamConfig !== null && await hasGitModeChange(git, 'teamai.yaml')) {
modeChangedPaths.add('teamai.yaml');
}
const unsafeDirtyPaths = collectUnsafeDirtyPaths(
await git.status(),
pendingTeamConfig,
modeChangedPaths,
);
if (unsafeDirtyPaths.length > 0) {
pullSpin.fail(
'Cannot push: the team repo has uncommitted changes. Commit or stash them first. '
Expand Down Expand Up @@ -1475,12 +1511,6 @@ 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, reusablePending);
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;
}
reuseRecordedDestinations(groups);
// The conflicting entries dropped above are deliberately not reused, so they
// are not "partly selected" either — warning about them would contradict the
Expand All @@ -1498,21 +1528,38 @@ async function pushCore(
})) return;

// ── Step 5: Push each group — one branch/PR per group ──────────────
// Config edits ride along with the first group so they land in a single PR.
let configRider = pendingTeamConfig !== null;
// Config edits ride along with the first group normally. With --branch, an
// existing-PR group must not receive the config because the explicit branch
// is intended for the new group. If there is no new group, use groups.length
// as a sentinel and push the config separately after all reuse groups finish.
const newGroupIndex = options.branch
? groups.findIndex((group) => !group.reuse)
: -1;
const configGroupIndex = pendingTeamConfig === null
? -1
: options.branch
? (newGroupIndex >= 0 ? newGroupIndex : groups.length)
: 0;
// Track the outcome across groups: a run counts as completed only if at least
// one group actually pushed AND no group's PR creation failed (#702 follow-up).
let anyPushed = false;
let anyPrFailed = false;
for (const group of groups) {
for (const [groupIndex, group] of groups.entries()) {
const outcome = await pushGroup({
group,
teamConfig,
localConfig,
pushState,
includeTeamConfig: configRider,
includeTeamConfig: groupIndex === configGroupIndex,
branch: options.branch,
});
// 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) {
await writeFile(path.join(localConfig.repo.localPath, 'teamai.yaml'), pendingTeamConfig);
}
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.
Expand All @@ -1526,7 +1573,6 @@ async function pushCore(
// (`reconcilePlacementRecords`).
if (outcome === 'pushed') anyPushed = true;
if (outcome === 'pr-failed') anyPrFailed = true;
configRider = false;
}

// Update state (pushState already carries the pendingPushes records above)
Expand All @@ -1548,6 +1594,21 @@ async function pushCore(
}
}
await saveStateForScope(state, localConfig);

// When every selected resource reuses an existing PR, --branch still names a
// real destination for the pending config edit. The reuse groups have already
// been saved above, so now push teamai.yaml alone on that explicit branch.
// Do not report completion if an earlier reuse PR creation failed.
if (pendingTeamConfig !== null && options.branch && newGroupIndex < 0) {
await pushTeamConfigOnly(
localConfig,
teamConfig,
options,
anyPrFailed ? undefined : result,
);
return;
}

// A real push completed only when a group actually pushed and no PR creation
// failed. Not set on dry-run/cancel (return earlier), a no-change run (every
// group 'nochange' → anyPushed stays false), or a PR-creation failure
Expand Down
Loading