Skip to content

fix(push): keep config changes on explicit branch - #820

Merged
jeff-r2026 merged 3 commits into
Tencent:mainfrom
dvd233:codex/fix/push-issue-800
Sep 25, 2026
Merged

jeff-r2026 merged 3 commits into
Tencent:mainfrom
dvd233:codex/fix/push-issue-800

Conversation

@dvd233

@dvd233 dvd233 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #800.
Follow-up to #690.

Summary

When teamai push --branch <name> updates an existing open-PR group and creates a new resource group in the same invocation, the pending teamai.yaml change must follow the explicit new branch. It must not silently modify the existing review.

This follow-up also makes the dirty-clone guard conservative for teamai.yaml metadata changes, preserves captured config across preceding reuse-group cleanup, and removes an unreachable grouping guard identified during the #690 review.

Changes

  • Route captured teamai.yaml changes to the first newly-created group when --branch is supplied; keep the existing PR group on its recorded branch.
  • Treat a captured teamai.yaml edit as restorable only when neither the worktree nor index diff reports a file-mode change.
  • Re-apply captured config after every preceding reuse group before the explicit new-branch group runs, including when metadata-only cleanup resets the clone.
  • Remove the unreachable multiple-new-group guard because planPushGroups() currently creates at most one new group.
  • Add real CLI coverage for mixed existing-PR/new-branch routing, metadata-only reuse cleanup, and content-plus-mode dirty-clone protection.

Test Plan

  • npm run typecheck
  • npm run build
  • npx vitest run src/__tests__/push-role.test.ts src/__tests__/push-pending-pr.test.ts --reporter=dot — 108 passed
  • npx vitest run --config vitest.e2e.config.ts src/__tests__/push-role-skill-e2e.test.ts --reporter=verbose — 21 passed
  • git diff --check

The E2E suite covers Claude, Codex, CodeBuddy, and OpenCode across generic Git, GitLab, and GitHub provider flows, including explicit resource/config branches, existing PR reuse, mixed routing, metadata-only reuse cleanup, and dirty-clone protection.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] Preserve the pending config across earlier reuse groups — src/push.ts:1534. With --branch, teamai.yaml remains unstaged while reuse groups run first. If one contains only metadata changes, pushRepoBranch() takes its metadata-only path and executes reset --hard, silently discarding the config before the new group runs. For example, an existing-PR rule changing only lastUpdated, plus a new resource and edited teamai.yaml, produces an explicit branch without the config edit. Reapply/preserve the captured config after every preceding group cleanup.

Testing

  • The PR description includes a sufficient real-CLI E2E record for this runtime change. Tests were not run during review, as requested.

@dvd233

dvd233 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the P1 in commit afb4893.

The issue was that a preceding existing-PR group could take the metadata-only path in pushRepoBranch(), reset and clean the clone, and discard the captured teamai.yaml before the explicit new-branch group ran.

The fix restores the captured config after every preceding reuse-group push, including metadata-only cleanup and handled failure paths, before the new explicit-branch group runs.

Added a real CLI regression test covering:

  • an existing PR with a rule change containing only lastUpdated metadata;
  • a new resource pushed with --branch;
  • an edited teamai.yaml in the same invocation.

The test verifies that the config lands on the explicit new branch and not on the existing PR branch.

Verification:

  • typecheck passed
  • build passed
  • focused unit tests: 108 passed
  • real CLI Push E2E: 21 passed

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] Handle --branch when every selected resource belongs to an existing PR — src/push.ts:1537. findIndex() returns -1, but Math.max(..., 0) assigns the config to the first reuse group. For example, editing teamai.yaml and a resource already under review, then running teamai push --branch feature/config, silently commits the config to the existing PR instead of the explicitly requested branch. Create a separate new config push or otherwise ensure the reuse group never receives it.

Prior Finding

  • Resolved: the captured config is now reapplied after preceding reuse-group cleanup.

Testing

  • The PR description includes sufficient real-CLI E2E coverage for this runtime change.
  • Tests were not run during review, as requested.

@dvd233

dvd233 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the follow-up finding in commit b3162b2.

When --branch is used and every selected resource belongs to an existing PR, teamai.yaml is now pushed separately to the explicitly named branch after the reuse groups finish. It is no longer assigned to the first existing-PR group.

Added a real-CLI regression test that updates a resource on an existing PR while changing teamai.yaml; it verifies the config is absent from the existing PR branch and present on the explicit branch.

Verification:

  • npm run typecheck — passed
  • npm run build — passed
  • npx vitest run --config vitest.e2e.config.ts src/__tests__/push-role-skill-e2e.test.ts — 22 passed

@github-actions

Copy link
Copy Markdown

Findings

  • [P2 non-blocking] Preserve the completed result when the separate config push is a no-op — src/push.ts:1609. If the local teamai.yaml edit was independently merged upstream before this run, pendingTeamConfig remains non-null but pushTeamConfigOnly() finds nothing to commit. After an existing-PR resource group was successfully pushed, the unconditional return skips anyPushed, leaving result.completed false and suppressing the push webhook.

Prior Findings

  • Resolved: config is reapplied after preceding reuse-group cleanup.
  • Resolved: when every selected resource reuses an existing PR, config is pushed separately to the explicit branch.

Testing

  • The PR description includes sufficient real-CLI E2E coverage for this runtime change.
  • Tests were not run during review, as requested.

@jeff-r2026
jeff-r2026 self-requested a review September 25, 2026 04:02
@jeff-r2026
jeff-r2026 merged commit e733377 into Tencent:main Sep 25, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] teamai push --branch: team-config edits attach to an existing PR group instead of the new branch

2 participants