Skip to content

fix(push): stop offering a teammate's update back as a local edit (#823) - #827

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
SaulMoro:fix-823-push-sync-followups
Sep 25, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
SaulMoro:fix-823-push-sync-followups

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Push could still list a copy the member never edited as modified, ready to send a teammate's update back as the old version. This fixes three of the paths #823 lists.

where push decides "did the member edit this copy?"
├── pre-push sync (git show)   single-repo mode     ── 2. path never resolved
├── placed agents              base                 ── 3. shared lastPullRev
└── pre-push sync skill copy   fails partway        ── 5. two revisions mixed
 syncTeamUpdatesToLocal(bases)                        # pre-push-sync.ts
-  git show <rev>:rules/x.md                          # single-repo: resolved from repo root, never found
+  git show <rev>:./rules/x.md                        # resolved from <wt>/.teamai
   skill at a base
-    copyDir(team, local)                             # overwrites in place
+    stage = copy(local) + copyDir(team)              # hidden sibling
+    rename local -> prev, stage -> local, rm prev    # a failure leaves the old copy whole

 AgentsHandler.scanLocalForPush                       # agents.ts
-  recordedAgentMovedOn(lastPullRev)
+  recordedAgentMovedOn(checkoutBaseRevs(record) or [lastPullRev])
+    held if the team file differs from its version at any base,
+    or from the version it was added with when a base predates it

Before / After (real CLI, teamai --dry-run push):

Scenario Before (origin/main f558b94) After
Item 2: single-repo, teammate updated .teamai/rules/team-rule.md, no teamai pull [rules] team-rule (modified) synced to the teammate's version, nothing to push
Item 3: placed agent, stale worktree, only the main checkout pulled the rewrite [agents] vr (modified) Skipped vr: ... changed on the team since this checkout last synced it

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Causes:

  • Item 2. Single-repo push runs with repo.localPath = <knowledge-wt>/.teamai, and git show resolves paths from the repo root, so no base was ever found.
  • Item 3. The shared lastPullRev is moved by a pull in another checkout (the [bug] push from a stale worktree diffs against another checkout's revision and can revert a teammate's update #812 revert, for agents). Push bases mark where rules and skills were synced, not agents, so any differing base counts. Push records the team HEAD before the scan, so the added-with version is also compared when a base predates the placement.
  • Item 5. A partial in-place copy mixed two revisions, which match no base. The stage starts from the local copy, so files only the member has survive as before.

Test Plan

  • npx tsc --noEmit passes
  • npx vitest run passes: 316 files, 4964 passed, 1 skipped
  • Added/updated tests for the change

New tests, all red on origin/main (f558b94) or under ablation:

e2e push-sync-followups-823
  item 2  teammate lands R2, branch fast-forwarded, no teamai pull -> synced, not listed   red on main
  item 2  genuine local edit -> still listed
  item 3  worktree B at R1, only main pulled R2 -> held in B                               red on main
  item 3  pull, place, merge, teammate edits, no pull -> held        guards the added-with fallback
unit    copyDir writes one file, throws ENOSPC -> skill whole, nothing beside it          red on main
unit    member-only files kept; read-only skill left whole, no leftover

Ablations, repeated after the rebase onto 21cb76a: each change was reverted alone and its test failed at the expected assertion. That covers the ./ prefix, agents.ts, the added-with fallback, the staged copy and the cleanup chmod. Three assertions in pre-push-sync.test.ts pinned the buggy argument rules/..., and now say ./rules/....

Build/Result: npm run build ok. E2E run (--retry 0): the new file, plus push-stale-worktree-812, push-namespace-e2e, role-scoped-agents, namespaced-entries, project-agent-cold-start, e2e, multi-project and copilot-agents: 9 files, 84 passed, 22 skipped (the skipped ones need a remote). The full e2e suite was not run.

Real CLI, before (origin/main f558b94) and after (this branch)
=== item 2, single-repo mode (before) ===
  .claude/rules/team-rule.md after pull: Version one.
  teammate pushed R2; member's branch fast-forwarded with git, no teamai pull
  $ teamai --dry-run push
    Found 1 resource(s) to push:
        1. [rules] team-rule (modified)
  .claude/rules/team-rule.md after push: Version one.

=== item 3, placed agent in a stale worktree (before) ===
    ✔ Pushed branch teamai/push/ci-823/...        (push --project front-app, then merged)
  wt-b agent after pull: You review.
  teammate rewrote vr; only the main checkout pulled it
  $ (wt-b) teamai --dry-run push
    Found 1 resource(s) to push:
        1. [agents] vr (modified)
           to:   agents/fe-agents/vr.yaml

=== item 2, single-repo mode (after) ===
  $ teamai --dry-run push
    ℹ No new or modified resources to push
  .claude/rules/team-rule.md after push: Version two, from a teammate.

=== item 3, placed agent in a stale worktree (after) ===
  $ (wt-b) teamai --dry-run push
    ⚠ [agents] Skipped vr: Agent "vr" (agents/fe-agents/vr.yaml) changed on the team since this checkout last synced it, ...
    ℹ No new or modified resources to push

Provider git (local bare remote), agent Claude. Other providers and agents are left to CI.

Related Issues

Part of #823 (items 2, 3 and 5; item 4 follows once #814 is merged)

Notes for Reviewers

Reproduced with the real CLI: items 2 and 3 (above).

From reading the code, covered by unit tests only: item 5. The partial-copy failure is forced by a mocked copyDir, plus a real read-only skill.

@jeff-r2026 jeff-r2026 self-assigned this Sep 25, 2026
@github-actions

Copy link
Copy Markdown

No findings.

The PR description documents sufficient testing for this runtime change, including representative real-CLI verification. Per instruction, I reviewed only the specified diff and did not run or build the PR code.

…ncent#823)

Three more ways push could list a copy the member never edited as modified,
ready to send a teammate's change back as the old version.

Single-repo mode (item 2). Push runs against a knowledge worktree whose team
root is <wt>/.teamai, a subdirectory of the git repo. The pre-push sync read
each base version with `git show <rev>:rules/x.md`, which git resolves from
the repo root, so it never found one, and every rule or skill a teammate
updated read as a local edit. The three reads now pass `./<path>`, which git
resolves from the working directory, as getFileContentWhenAdded and the agent
guard already did.

Placed agents (item 3). An agent placed with --role/--project is held when it
changed on the team since this machine's copy was current, and "current"
meant the version at the shared lastPullRev, which a pull in another checkout
moves past a copy a stale worktree still holds (the Tencent#812 revert, for agents).
The guard now reads this checkout's bases through checkoutBaseRevs, and falls
back to the shared lastPullRev for a checkout with no entry, as the pre-push
sync does. Push bases record where the sync moved rules and skills, not
agents, so the copy stays at the revision pull delivered: the guard holds an
agent that differs from its version at any base. Push records the team HEAD
as a base before the scan, and the file there is always the current one, so
the version the agent was added with is compared too whenever a base
predates it; otherwise a placement that landed after the last pull would go
back over a teammate's later edit. The hold message now says "this checkout".

Skill copy (item 5). The sync overwrote a local skill in place, so a copy
that failed partway left files from two revisions, matching no base, and the
next push listed the skill as modified. The update is now built in a hidden
sibling (the local copy, then the team version over it, so files only the
member has survive as before) and renamed into place; a failure leaves the
previous version whole. The stage carries the local modes, so cleanup makes
a read-only stage writable before removing it, and warns with the path if a
leftover cannot be removed; if the previous version cannot be renamed back,
the error names where it is.

Item 4 (user-scope push base) follows once Tencent#814 is merged.
@SaulMoro
SaulMoro force-pushed the fix-823-push-sync-followups branch from 0302ab4 to 51c4b75 Compare September 25, 2026 10:12
@github-actions

Copy link
Copy Markdown

No findings.

The PR description documents sufficient testing for this runtime change, including representative real-CLI verification. I reviewed only the specified diff and did not run, build, or install the PR code.

@jeff-r2026
jeff-r2026 self-requested a review September 25, 2026 12:15
@jeff-r2026
jeff-r2026 merged commit e79db17 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.

2 participants