fix(push): stop reverting a teammate's update from HOME or a stale .teamai copy (#823) - #835
Merged
Merged
Conversation
…eamai copy (Tencent#823) Item 4, user scope: push compared HOME's rules and skills with the shared lastPullRev only, because the per-checkout push bases of Tencent#819 were keyed for project scope alone. After a push synced HOME's unedited copy to a teammate's R2, the next push compared it with R1 and offered it back over the teammate's R3. A user-scope pull now records HOME under checkoutKey(HOME) in the user state.json, and push reads and extends it like a project checkout's. The user-scope fast path still reads the shared fields, and an install with no record yet keeps comparing with lastPullRev without the unrecorded-checkout refusal: HOME is the scope's only checkout, so that revision is its own. Item 19, inherited user scope: a project pull with inheritUserScope rewrites HOME's skills, rules and agents under lastInheritedPullRev without moving the user scope's push bases, so the next user-scope push offered a teammate's newer update back the same way. That pull now adds its revision to HOME's pushBaseRevs, creating the record from lastPullRev if there is none, and leaves the record's rev, lastPullRev and the fast paths alone. Item 10, single-repo: the active tree's .teamai/rules and .teamai/skills are push sources, and on a branch behind the default branch they hold older team versions nobody edited, which push listed as modified. The isPastVersionOf guard that held only placed rules now covers every .teamai/rules copy, and .teamai/skills gets the same guard: a skill is skipped with a warning when every team file whose copy differs is an older version of it. A team file missing locally is a teammate's addition when the member's branch never added it, and the member's deletion otherwise; member-only files are ignored, as the equality check already ignores them. The checkout-base resolution that push and the agents scan each repeated (key, record, checkoutBaseRevs, lastPullRev fallback) is now one exported helper, resolveCheckoutBases, next to checkoutBaseRevs in pull.ts; pull uses the same key for its record.
SaulMoro
force-pushed
the
fix-823-push-bases
branch
from
September 25, 2026 19:26
5279418 to
6c46ee4
Compare
The PR description includes sufficient unit, E2E, and representative real-CLI verification. |
…stall (Tencent#823) An upgraded user-scope install with no HOME record synced HOME from R1 to R2 on its first push but saved no base, because push recorded one only when the bases came from a record. A teammate's R3 then made the next push compare the R2 copy with lastPullRev R1 and offer it back. Push now creates HOME's record from lastPullRev (userScopeRecord, shared with the inherited pull) and adds the revision its sync reached. An unrecorded project checkout still records nothing, since its fallback base may be another checkout's. skill-data: contribute-member explains the stale .teamai copy warning and how to publish an edit of such a copy.
The two earlier findings are resolved in the current diff: first-push HOME record creation was added, and the affected |
…pull's delivered base (Tencent#823)
|
No new findings.
|
…laced root rule at every base (Tencent#823)
|
No new findings.
|
jeff-r2026
self-requested a review
September 26, 2026 11:24
jeff-r2026
approved these changes
Sep 26, 2026
This was referenced Sep 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Push no longer offers a teammate's update back as a local edit from a user-scope install (HOME), or from a single-repo branch behind the default branch.
pullrecords HOME inlastPullByWorkspace, and push reads and extends it the way it does for a project checkout (fix(push): keep a teammate's update when pushing from a stale worktree (#812) #819). The user-scope fast path still reads the shared fields. An install with no record yet keepslastPullRevand is not refused: HOME is the scope's only checkout. Its first push creates the record fromlastPullRevand adds the revision its sync reached.pullwithinheritUserScopemoves HOME's skills, rules and agents, so it adds its revision to HOME's push bases in the same record, creating the record fromlastPullRevif there is none. It leaves the record'srev,lastPullRevand both fast paths alone.lastPullRevandlastInheritedPullRev, and the record created from them keepslastInheritedPullRevas a push base. Nothing records which of the two pulls ran last, and each delivered every rule, skill and agent push compares, so both are candidates and a copy matching either counts as unedited. Picking one would re-open the bug whenever the other ran last.rev,lastPullRev = nulland the retry exactly as before.isPastVersionOfguard now covers every.teamai/rulescopy, and.teamai/skillsgets the same guard. A genuine edit is still listed, because its blob is not in the default branch's history. A team file missing from the.teamai/skillscopy is a teammate's addition when the member's branch never added it; otherwise it is the member's deletion, and still listed.{ rev, targets }, dropping the bases push compared those copies with. It now keeps them aspushBaseRevs, read before the marker moves, for HOME and for a recorded project checkout (items 4 and 19; the same revert, through a held pull).isDeliveredRenderinrules.ts) compared a copy only with the current render and the one atlastPullRev. It now accepts a render at any of the checkout's bases, so an unedited copy an inherited pull delivered, or a project checkout whose record differs from the sharedlastPullRev, is withdrawn instead of kept with a "differs from what teamai delivered" warning.resolveCheckoutBases(localConfig, state): Promise<CheckoutBases>(exported frompull.ts, besidecheckoutBaseRevs) replaces the copies of this logic inpush.tsandagents.ts; project scope behaves as before.Type of Change
Test Plan
npx tsc --noEmitpassesnpx vitest runpasses (326 files, 5045 passed, 1 skipped)push-sync-followups-823.test.ts, 1 unit case inpull-sync-truth.test.ts, 1 unit case inpull-namespace-override.test.tsAblations (each reverted alone, e2e rerun): the user-scope record key, the rules guard, the skills guard, the added-file check (both directions), the project-only refusal, the user-scope fast path, the inherited pull's base, the record it creates when there is none, and the record an upgraded install's first push creates each turn their test red. Creating that record with an empty
revinstead oflastPullRevstays green: an inherited pull delivers every rule and skill push compares, so keepinglastPullRevonly makes the bases a superset of what push read before.Review cycle 2: the three new cases are red on
ad0ce83c(thepull.tsfix swapped out) and green after. Fullnpx vitest runpasses; e2epush-sync-followups-823,push-stale-worktree-812,pull-new-worktree-807,roles-tags-pull,pull-missing-team-config,import-mr-publish-823(6 files, 49 passed).Review cycle 3: the held-skill e2e (both scopes) and the inherited-pull row of the replaced-root-rule unit test are red on
32bffbdband green after.npx tsc --noEmit, fullnpx vitest runandnpm run buildpass; e2epush-sync-followups-823,push-stale-worktree-812,roles-tags-pull,project-scoped-delivery,namespaced-entries,pull-new-worktree-807(6 files, 46 passed).Earlier e2e run with
--retry 0:push-sync-followups-823,push-stale-worktree-812,pull-new-worktree-807,self-mode-worktrees-808,role-scoped-agents,namespaced-entries,project-agent-cold-start,scope-isolation-issue85,deleted-worktree-scope-810,multi-project,scope-inheritance-e2e,e2e(12 files, 107 passed, 22 skipped by their own guards).Real CLI:
teamai --dry-run push, origin/main vs this branchThrowaway sandboxes with local bare remotes; the config files are written by hand, as the e2e harness does.
Item 10: single-repo project, a teammate lands a new
.teamai/rules/team-rule.mdonmain, the member fetches and does not merge.Item 4: user scope, pull R1, teammate R2,
--dry-run push(syncs HOME to R2), teammate R3,--dry-run pushagain.Item 19: user scope pull R1; a project with
inheritUserScope: truepulls R2; teammate R3; user-scope--dry-run push.Item 4, upgraded install (review fix): user scope, pull R1, drop the user-scope record, teammate R2,
--dry-run push, teammate R3,--dry-run pushagain.Review cycle 2, upgraded install: user scope pull R1; a project with
inheritUserScope: truepulls R2; drop the HOME record (an older CLI ran that pull); teammate R3; user-scope--dry-run push.Review cycle 2, docs mirror failing: user scope with a team doc, pull R1; a file blocks
~/.teamai/docs; teammate R2;pull; teammate R3;--dry-run push.Review cycle 3, held skill: user scope with role
devover namespacesalphaandbeta, pull S1; a teammate updatesalpha/team-skilland addsbeta/team-skill;pull; the teammate removesbetaand publishes S3;--dry-run push.Related Issues
Part of #823 (items 4, 10 and 19)
Notes for Reviewers
Door: two-way. Blast radius: push.
lastPullRev, as today..teamai/rule or skill to an older team version is held with a warning, the trade-off already accepted for placed rules ([bug] Namespace resolution is inconsistent across resource types: push ignores --role/--project for rules and agents, and root-level claudemd never ships #649).codex-review-on-assign.ymlprompt, read-only Claude subagent) and/code-review(standards, spec): no blocking finding. Fixed here: a stale skill lacking a file a teammate added was still listed, and pushing it would have deleted that file.lastPullRevpulls; an inherited pull now writes push bases into it too, but never itsrev, which only a full user-scope pull sets. A pull whose docs mirror or submodule update fails does the same: it adds a base and leavesrevand the shared marker for the retry. A project checkout no pull had recorded gets a record with an emptyrevin that case, since its copies are now at a known revision..teamai/copy warning and its recovery inskill-data/core/references/contribute-member.md(P2);docs/designs/data-directory-layout.mdcovers the new record. Cycle 2: fixed the upgraded install ignoringlastInheritedPullRev(P1) and a failed docs mirror dropping the delivered base (P1, submodule failures had the same gap and share the fix). Scanned the other base andlastPullRevwrites inpull.tsandpush.ts: push already records its base when the sync stops partway; it records none when the team repo refresh failed, by design, since the tree may not match HEAD. Cycle 3: the two adjacent gaps first logged as follow-ups are fixed here, as they are the same revert class: a pull that holds skills (or agents) on a namespace collision keeps the older bases, and the replaced-root-rule cleanup reads every base. A project checkout no pull has recorded keeps no held bases, matching push's rule for that case (its fallbacklastPullRevmay be another checkout's). The held-agents path shares the flag and has no separate test..teamai/agents(unchanged), and extra providers and agents, which are left to CI.