Skip to content

fix(pull): keep a skill, rule or agent you edited instead of overwriting it (#822) - #865

Open
SaulMoro wants to merge 3 commits into
Tencent:mainfrom
SaulMoro:fix/822-pull-keeps-local-edits
Open

SaulMoro wants to merge 3 commits into
Tencent:mainfrom
SaulMoro:fix/822-pull-keeps-local-edits

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Every full teamai pull copied skills, rules and agents over the local copy, so an edit the member had not pushed yet was lost without a word. Now pull keeps a copy the member changed since teamai delivered it, and names it with the next step.

Pull records what it wrote. Each checkout record (lastPullByWorkspace[<checkout>], and HOME's for the user scope) gains an optional delivered map: the sha256 of the bytes teamai last wrote at each skill, rule and agent path. Pull and the pre-push sync update it through the existing atomic state save. A copy counts as edited only when it has a record and no longer matches it. Comparing written bytes, rather than re-rendering the team file at an older revision, keeps the check correct across a renderer change such as #856's Cursor model field, for skills delivered through a submodule, and for a SKILL.md whose frontmatter pull repairs.

Copy on disk Pull (full sync)
matches the record, or equals the team version writes the team version, as before
changed, team version unchanged keeps it: Kept <path>: you changed it since teamai delivered it. Share it with `teamai push`, or delete it and run `teamai pull --force` to get the team version back.
changed, team version changed too keeps it and warns: Kept <path>: you changed it, and the team version (<rel>) has changed since. Merge the team change into your copy and share it with `teamai push`, or delete your copy and run `teamai pull --force` to take the team version.
deleted by the member writes the team version
no record yet (first full pull on this version, a new worktree's first pull, a path teamai never wrote) writes, as before, and records, so protection starts there
  • Per destination. Each tool's copy is judged on its own, so an edited Claude rule is kept while the Cursor .mdc of the same rule updates. A skill directory is one unit: a change to any of its team files keeps the whole skill. Files only the member added are not recorded and do not count, and neither does CONTRIBUTORS, matching what push compares.
  • Removals. Tombstone cleanup and the rules sweep of a rule no longer delivered keep an edited copy: Kept <path>: the team removed <name>, but you changed this copy. Delete it when you no longer need it.
  • --force keeps edits. It is the repair step doctor and several errors print, so it must not become a way to lose work. The steps say pull --force because the pull that kept a copy has recorded the team revision, so a plain pull after it would print Already synced and restore nothing.
  • --dry-run prints Would keep <path>: you changed it since teamai delivered it. and writes nothing.
  • Push. A kept copy is listed as modified, as it is today when a member pushes an edit before pulling. When the team version has moved since, push now warns first: The team changed <rel> since teamai delivered <path>; pushing replaces that change unless you merged it. Merge the team version first, or delete your copy and run `teamai pull --force`. The SessionStart pull is silent, so without this the member could miss pull's warning. It is a warning, not a hold, because the member may have merged the change already. The agent push hold's message now gives the same steps (it used to say pull replaces the copy).
  • feat: one namespace model for every resource type (#707) #816 leftover warning. Once a record exists, a file at a path another team version of the skill has is removed when it is still what teamai wrote there, and a file teamai has no record of is the member's own and stays without a warning.
  • doctor no longer fails a rules or agents check, or points at pull --force, for a kept copy. Next to another problem it lists the copy as changed by you (kept by pull).
  • Worktrees. A forced full sync elsewhere (roles set, projects set, skill exclude) resets the other checkouts' records and now keeps their delivered, so their next pull still protects their edits.

Not changed: a same-name copy teamai never delivered to that path is still overwritten. teamai remove's rules refresh and local-agent installs still write without this check and record nothing. Step 3b and the inactive-namespace cleanups still compare with the team source. An older CLI that saves state drops delivered; the next pull on this version is then back in the no-record case. All of this is in the usage guide and in docs/designs/multi-project-management.md.

Part of #822 (item 5).

Evidence

  • Tests before and after. Each new test fails without its change and passes with it:

    • delivered-copies.test.ts (new): the classification, including a fast-check property that pull keeps a copy only with a recorded file whose bytes are neither the record nor the team version.
    • e2e/pull-keeps-edits-822.test.ts (new, 10 tests, against the built CLI with a skill directory, a rule delivered to Claude .md and Cursor .mdc, and an agent rendered for Claude and Cursor):
      • unedited copies updated;
      • edited with the team unchanged: kept with the info line on --force, and restored after deletion;
      • edited with the team changed: kept with the warning, while the Cursor renders update;
      • --dry-run: names the kept copies, and state.json and the files are unchanged;
      • tombstones: kept when edited, removed when untouched;
      • a rule deleted without a tombstone: kept when edited;
      • a worktree record kept through a forced full sync;
      • upgrade with no record: overwritten, then protected;
      • push warns after a silent pull kept a copy, and after the team amended a pushed copy, but not while the team is unchanged.
    • pre-push-sync.test.ts (rule and skill writes recorded), pull-namespace-override.test.ts (feat: one namespace model for every resource type (#707) #816 case), and doctor-rules-delivery.test.ts (kept-copy label).
    • Before the change, 5 of the 6 original e2e scenarios failed, for example expected 'echo one\n' to be 'echo mine\n'.
  • npm run lint: 0 warnings. npx tsc --noEmit: clean. npm run build: OK.

  • npx vitest run --maxWorkers=4 on Node 22: 5176 passed, 1 skipped. 15 tests in git-kind-reports.test.ts and git-kind-learnings.test.ts hit the 15 s timeout under the parallel run; both files pass on their own (31/31).

  • E2E (--config vitest.e2e.config.ts), 79 passed: pull-keeps-edits-822, push-sync-followups-823, push-stale-worktree-812, pull-new-worktree-807, doctor-delivery-cli, role-scoped-agents, project-scoped-delivery, import-mr-publish-823. e2e.test.ts, multi-project, scope-isolation-issue85, self-mode-worktrees-808 and roles-tags-pull also pass.

  • Real CLI, Claude, git provider, local bare remote. Pull, edit the delivered rule and skill, a teammate changes both, pull again.

    Before (a8ab8e00), the edits are gone:

    $ teamai pull
    ✔ [project] Synced 1 skills (all updated)
    ✔ [project] Synced 1 rule(s)
    $ cat .claude/rules/style.md
    Use two spaces. Wrap at 100.
    $ cat .claude/skills/review/SKILL.md (body)
    Check the tests and the changelog.
    

    After (this branch):

    $ teamai pull
    ✔ [project] Synced 1 skills (all updated)
    ⚠ [project] Kept <sandbox>/project/.claude/skills/review: you changed it, and the team version (skills/review) has changed since. Merge the team change into your copy and share it with `teamai push`, or delete your copy and run `teamai pull --force` to take the team version.
    ✔ [project] Synced 1 rule(s)
    ⚠ [project] Kept <sandbox>/project/.claude/rules/style.md: you changed it, and the team version (rules/style.md) has changed since. Merge the team change into your copy and share it with `teamai push`, or delete your copy and run `teamai pull --force` to take the team version.
    $ cat .claude/rules/style.md
    Use two spaces. My team prefers tabs in Makefiles.
    $ cat .claude/skills/review/SKILL.md (body)
    Check the tests. Also check the docs.
    $ teamai pull --dry-run --force
    ℹ [project] [dry-run] Would pull 1 skills
    ℹ [project] [dry-run] Would keep <sandbox>/project/.claude/skills/review: you changed it since teamai delivered it.
    ℹ [project] [dry-run] Would sync 1 rule(s)
    ℹ [project] [dry-run] Would keep <sandbox>/project/.claude/rules/style.md: you changed it since teamai delivered it.
    $ teamai push --dry-run
    ⚠ [skills] The team changed skills/review since teamai delivered <sandbox>/project/.claude/skills/review; pushing replaces that change unless you merged it. Merge the team version first, or delete your copy and run `teamai pull --force`.
    ⚠ [rules] The team changed rules/style.md since teamai delivered <sandbox>/project/.claude/rules/style.md; pushing replaces that change unless you merged it. Merge the team version first, or delete your copy and run `teamai pull --force`.
    Found 2 resource(s) to push:
        1. [skills] review (modified)
        2. [rules] style (modified)
    ℹ Dry run — no changes made
    $ rm .claude/rules/style.md && teamai pull --force    # skill lines omitted
    ✔ [project] Synced 1 rule(s)
    $ cat .claude/rules/style.md
    Use two spaces. Wrap at 100.
    

Notes

Review notes

  • After a member pushes copy P, a reviewer may amend the PR, or a teammate may change the file before any full pull has run. The next pull then keeps P, with the "team version has changed since" warning, and push warns too. It does not update P silently, because telling a merged push from a pending one needs the team history of each file. The member is told the step (delete, pull --force), and nothing is lost.

…ing it (Tencent#822)

Pull records the sha256 of the bytes it writes at each skill, rule and
agent destination in the checkout's record (`delivered`), and the
pre-push sync records its writes too. On a full sync, a copy that no
longer matches the record is kept and named, with a warning when the
team version moved since; push repeats that warning. Tombstone cleanup
and the rules sweep keep an edited copy the same way. `--force` keeps
edits, `--dry-run` prints `Would keep`, and `doctor` does not fail on a
kept copy. Without a record (first pull on this version, a new
worktree) pull overwrites as before.
Resolve pre-push-sync.ts against Tencent#857: the Copilot/Cursor render path
records the copy it writes in delivered, so the next pull updates it.
@github-actions

Copy link
Copy Markdown

No findings.

The PR description documents sufficient testing, including a representative real-CLI end-to-end verification. Per instruction, I reviewed the diff only and did not run code.

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