fix(push): namespace new rules and agents from --role/--project (#649)

[SaulMoro]

Summary

teamai push --role <ns> and teamai push --project <id> only ever placed new skills. A new rule was written to rules/<name>.md and a new agent to agents/<name>.yaml. Neither path carries a namespace segment, so pull delivers both to every member of the team (src/pull.ts, slashIndex === -1 returns true, and the !agent.namespace || branch). --project also collapsed into the project's skills namespace, which is the wrong directory for a rule: a rule is scoped on the knowledge axis, and ProjectResourceNamespacesSchema allows a project to declare different namespaces for the two.

Each pushable type now resolves from its own axis, and push says where the resource went.

 push --project front-app
   skills → resources.skills     = fe-skills
+  rules  → resources.knowledge  = fe-know
+  agents → resources.agents     = fe-agents

 team-repo/
   skills/fe-skills/my-skill/
-  rules/my-rule.md            # no namespace segment, so every member receives it
+  rules/fe-know/my-rule.md
-  agents/vr.yaml              # same, through the !agent.namespace branch
+  agents/fe-agents/vr.yaml

Only a new resource that would otherwise land at the shared root is placed. A resource the scanner already namespaced keeps its path, which is what #654 fixed, and the destination an open PR recorded still wins so a force-push never moves a resource that is under review.

push --project <id> / --role <ns>
  step 3b  reuse the destination this PR recorded
  step 4   for each of skills, rules, agents
             items that are new AND still at the shared root
               --project  resolve that type's axis, fail and name the type if absent
               --role     use the value as the namespace
               neither    the role manifest's namespaces for that axis
             print the destination

One more change closes a hole the placement would have opened. RulesHandler.scanLocalForPush matched a local rule against the team repo by full path, so once a rule was placed under rules/<ns>/, the author's copy at the tool's rules root read as brand new on the next push and went back to the shared root. push now records where it placed each root-level rule in state.json (placedRules, rule name → team path), and the scan redirects a root-level local rule only when that record exists and its team file is still present. A basename match alone is never used: a namespaced team rule is pulled into a namespaced local directory, so another member's unrelated root rule with the same name has no record and stays new (review round 1).

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature causing existing behavior to change)
• Documentation only
• Refactor / internal cleanup

Test Plan

Run on 23717ec, the head of this PR, based on a52374a (current main), after npm run build of that commit.

• npm run build — success
• npx tsc --noEmit — clean
• npx vitest run — 277 files, 4056 passed, 1 skipped, 0 failed
• npm run test:e2e — the full e2e suite, every file: 45 files (42 passed, 3 skipped — the live-provider files that need credentials), 215 passed, 26 skipped, 0 failed, exit 0
• Real-CLI coverage for every agent and every provider — src/__tests__/push-namespace-e2e.test.ts, 37/37

The full e2e suite was run locally on this exact commit. Since #739, CI also runs it for this fork PR in the E2E (fork-safe, no credentials) job; E2E (GitHub provider, full surface) still skips here because its vars.TEAMAI_TEST_REPO_URL guard cannot be satisfied from a fork.

In round 21 one unrelated unit test, local-agent: emitBindingHint … emits hint only once per sessionId, failed once and then passed on every rerun. This PR does not touch src/local-agent.ts or its test. On 23717ec the full suite passed on the first run.

The three shell-profile failures reported in the previous round are gone. They were environmental: the detectShellProfile > Windows (win32) cases do not stub SHELL, and detectShellProfile reads SHELL before the platform branch, so a zsh terminal resolved them to .zshrc. The suite is green with the SHELL=/bin/bash that CI has, and none of those files is touched by this branch.

Real CLI, committed

The verification is no longer a one-off script. src/__tests__/push-namespace-e2e.test.ts drives the built dist/index.js against real git remotes, a fake gh binary and a fake GitLab API server, and asserts on the content of the branch that reached the remote — never on CLI output alone.

case	agents	provider	asserts
--project resolves each type from its own axis	claude, codex, codebuddy, opencode	git	rules/fe-know/my-rule.md, skills/fe-skills/my-skill/SKILL.md, agents/fe-agents/vr.yaml on the branch; no rules/my-rule.md, no agents/vr.yaml; state.placedRules recorded
creates a GitHub PR for the namespaced branch	claude, codex, codebuddy, opencode	github	gh pr create -R team/issue-649 invoked, PR URL reported, same namespaced paths on the branch
creates a GitLab MR for the namespaced branch	claude, codex, codebuddy, opencode	gitlab	POST /api/v4/projects/team%2Fissue-649/merge_requests, MR URL reported, same namespaced paths
edit of the root copy after the PR merges	claude	git	unchanged copy → No new or modified resources; edited copy → (modified) back to rules/fe-know/my-rule.md, never to the root
another member's root rule with the same basename	claude	git	listed (new), lands in that member's own namespace, the author's rules/fe-know/my-rule.md on the branch is byte-identical
teammate's newer namespaced rule	claude	git	the stale root copy is synced to the teammate's version, not pushed over it; main keeps the teammate's content
unreadable roles manifest	claude	git	exit 2, Cannot resolve where new rules should go, no branch on the remote
--dry-run	claude	git	both destinations printed, no branch pushed; exit 2 on a project axis that cannot answer
an agent published into a namespace this directory never activated	claude	git	the author's next edit is listed (modified) rather than skipped as "no active source", and reaches agents/fe-agents/vr.yaml on the branch
remove rules <bare name> for a rule published under a namespace	claude	git	resolves to fe-know/my-rule, drops the namespaced team file from the branch, and takes the author's root copy with it; the record stays until the removal merges, and the next pull drops it
remove agents vr, then again while the first removal is unmerged	claude	git	the retry still reports vr was published as fe-agents/vr; its branch drops only agents/fe-agents/vr.yaml and keeps agents/other-ns/vr.yaml; after merge + pull, no record
a placement merged that this clone never fetched (fetch URL unreachable, push URL working), then remove agents vr	claude	git	exit 1, could not be refreshed, Nothing was removed, no branch on the remote — the stale tree would have resolved vr to the bare stem and removed other-ns/vr too
a placed agent removed, pull while the removal is open, merge, pull, then the copy written back and push --all	claude	git	redelivered during review, as any pending removal is; gone after the post-merge pull although the only tombstone is fe-agents/vr; the push lists no [agents] vr and puts no vr agent on any branch
remove agents vr with a copy in an excluded tool (enabledAgents: [claude], .codex/agents/vr.toml), merged, pulled, then push --all	claude + excluded codex	git	the codex copy survives the removal, as designed, and the push lists no [agents] vr and puts no vr agent on any branch
the same removal with local edits on the author's copy	claude	git	still resolves to fe-know/my-rule; the local scan offering the bare name does not win
a roles manifest namespace that is not one path segment	claude	git	exit 2 naming foo/bar, no branch on the remote
an agent whose stem also exists in a namespace this directory never activates	claude	git	published to agents/fe-agents/vr.yaml; the other namespace's agent is untouched on the branch
a new rule whose resolved namespace already holds that name	claude	git	exit 2 naming rules/be-know/foo.md, no branch, the team rule intact on main
remove agents <bare stem> for an agent published under a namespace	claude	git	resolves to fe-agents/vr, drops only that file, leaves other-ns/vr intact, tombstones only the published name
pull --force, twice, for an agent published into a namespace never activated here	claude	git	delivered and still present after the second pull — delivery and revocation agree
pull --force for a rule this machine placed in an ACTIVE namespace	claude	git	the author's root copy carries the teammate's newer content; no second copy at .claude/rules/be-know/my-rule.md
a shared-root rule awaiting review, then push --role fe-know	claude	git	awaiting review at rules/my-rule.md + separate PR; two branches on the remote, the pending one still holds rules/my-rule.md at v1, the new one holds rules/fe-know/my-rule.md
the remote renames the project's knowledge namespace after this clone's last pull	claude	git	push --project front-app places at rules/fe-know-v2/my-rule.md, not the stale fe-know
origin unreachable, then push --project front-app	claude	git	exit 1, could not be refreshed, no branch on the remote
a placed rule merged, pulled (record written), then deleted from main by a teammate, then pull --force; then a teammate recreates the path and pull --force again	claude	git	state.placedRules no longer carries the record after the delete, and does not get it back after the recreate; the teammate's rule is delivered to be-know/
push, PR never merged, branch kept; then a teammate creates that very path	claude	git	no record after either pull; the teammate's rule is delivered to be-know/, the author's root copy is untouched
a placed rule merged and recorded, then a teammate adds rules/my-rule.md at the shared root	claude	git	pull warns now exists at the shared root, drops the record, root copy = shared rule, be-know/my-rule.md = the author's; the next push of the root copy goes to rules/my-rule.md and leaves rules/be-know/my-rule.md alone

Each agent authors the new agent in its own format, because push reverse-parses the local file before it can write the canonical .yaml: TOML for Codex, .md with YAML frontmatter for the rest.

Red before green. Reverting any one fix and rebuilding turns exactly its cases red and leaves the rest green:

round 2, reverting the three fixes in src/push.ts
  ×  syncs a teammate's newer namespaced rule instead of pushing the stale root copy over it
  ×  stops the push when the roles manifest exists but cannot be parsed
  ×  --dry-run reports the destinations and pushes nothing
  ×  --dry-run fails on a project axis the real push would refuse
  ✓  14 others

round 3, reverting each fix in place
  ×  lets the author keep editing an agent they published into an inactive namespace     (e2e)
  ×  removes a rule by the bare name it was published under a namespace with             (e2e)
  ×  accepts the namespace push recorded for an agent even when it is inactive           (unit)
  ×  keeps following the record when a shared-root rule with the same name appears later (unit)
  ✓  everything else

round 4, reverting each fix in place
  ×  removes the published rule even when the author's root copy has local edits    (e2e)
  ×  refuses a roles manifest namespace that is not a single path segment           (e2e)
  ×  does not record an agent it merely edited in an already-active namespace       (unit)
  ×  refuses a roles manifest whose namespace is not a single path segment          (unit)
  ×  keeps the placement of a group that pushed when a later group fails            (unit)
  ✓  everything else

round 5, reverting each fix in place
  ×  publishes an agent into the requested namespace despite the same stem elsewhere (e2e)
  ×  publishes into the requested namespace despite a stem in an inactive one        (unit)
  ×  edits the copy in the requested namespace rather than treating it as new        (unit)
  ×  refuses to publish a namespaced second copy beside a shared-root agent          (unit)
  ×  leaves an unrelated root rule that only shares the basename                     (unit)
  ×  ignores a record that points somewhere else                                     (unit)
  ✓  everything else

round 6, reverting each fix in place
  ×  refuses to place a new rule onto a team rule that already holds the name      (e2e)
  ×  refuses to place a new rule onto an existing team file                        (unit)
  ×  refuses to place a new skill onto an existing team skill                      (unit)
  ×  reports an existing manifest it cannot read, rather than a missing one        (unit)
  ×  tombstones the bare name too, so a copy the sweep skipped cannot come back    (unit)
  ✓  everything else

round 7, reverting each fix in place
  ×  resolves the bare agent name to the namespace push recorded for it            (unit)
  ×  refuses to place a new .md agent beside an existing .yaml of the same stem    (unit)
  ✓  everything else

round 8, reverting to the round-7 state
  ×  removes only the published agent, through the real remove command           (e2e)
  ×  removes only the named namespace, leaving the same stem elsewhere           (unit)
  ×  keeps the flattened local copy when no record proves it is this agent's     (unit)
  ✓  everything else

round 9, reverting each fix in place
  ×  fails on a project with no agents namespace even when the scan skipped the agent (unit)
  ×  requires the exact filename, not merely the resource name as a prefix           (unit)
  ✓  everything else

round 10, reverting each fix in place
  ×  follows the record for a root canonical source placed in a namespace          (unit)
  ×  removes the canonical source so the agent cannot republish itself             (unit)
  ×  refuses to push over a recorded agent that moved on since the last pull       (unit)
  ×  lets a modified namespaced agent through a project whose agents axis is empty (unit)
  ✓  everything else

round 11 — the guard was removed rather than deepened, so this round is covered
by new behaviour rather than by reverting a check:
  delivers an agent this machine published into an inactive namespace      (unit, pull filter)
  leaves the record alone when an active namespace claims that stem        (unit, pull filter)
  prefers an active source over the placement record                       (unit, scan)
  keeps the namespace an open PR recorded for a skill even under --role    (unit, superseded expectation)

self-review + round 12, reverting each fix in place
  ×  pull delivers a published agent from an inactive namespace and keeps it   (e2e)
  ×  does not revoke an agent this machine published into an inactive namespace (unit)
  ×  spares the author's root copy of a rule published into a namespace        (unit)
  ×  detects a pending namespace recorded only in the path                     (unit)
  ×  --dry-run reports the same destination the real push would use            (unit)

round 13, reverting each fix in place (a304672 tests against f0717d6's build)
  ×  pushes a selected rule when only the unselected new agent lacks a project agents namespace (unit)
  ×  reads the projects manifest only after the team clone has been pulled                    (unit)
  ×  treats a pending shared-root resource as conflicting with an explicit namespace          (unit)
  ×  delivers a rule this machine placed onto the author's root copy, not beside it           (unit)
  ×  pull updates the author's root copy of a placed rule instead of writing a second one     (e2e)
  ×  leaves a shared-root PR untouched when the next push names a namespace                   (e2e)
  ×  places by the projects manifest the pull just fetched, not the one from the last pull    (e2e)

round 14, reverting each fix in place (fe8b5b4 tests against a304672's build)
  ×  stops a --project push when the team clone could not be refreshed                       (unit)
  ×  drops a placement record whose PR was closed without merging, before scanning           (unit)
  ×  moves the placement record to the extension a renamed canonical agent now has           (unit)
  ×  follows a renamed canonical source to its new extension and retires the recorded file   (unit)
  ×  prunePlacementRecords, all 6 cases                                                      (unit, new module)
  ×  refuses a --project push when the team clone cannot be refreshed                        (e2e)
  ×  pull drops the placement record of a rule the team has since deleted                    (e2e)

round 15, reverting each fix in place (76a0902 tests against fe8b5b4's build)
  ×  reconcilePlacementRecords + isPlacement, all 12 cases                                    (unit, rewritten module)
  ×  turns a placement into a record only once its file is on the default branch              (unit)
  ×  records where a root-level rule / a new agent was placed (now on the pending entry)      (unit, 2)
  ×  keeps the placement of a group that pushed when a later group fails                      (unit)
  ×  moves the placement record to the extension a renamed canonical agent now has            (unit)
  ×  does not redirect a placed rule onto a root path a shared-root rule of the same name owns (unit)
  ×  spares the author's root copy while its placement is still awaiting review               (unit)
  ×  --project resolves each type from its own axis, all 4 agents (record timing)             (e2e)
  ×  never records a placement whose PR was closed without merging, even with the branch kept (e2e)
  ×  withdraws a placement record when a shared-root rule takes the name                      (e2e)

round 16, reverting the fix (b8df32f tests against 76a0902's build)
  ×  records a placement once: not again after the team deleted the file and someone recreated the path (unit)
  ×  pull drops the placement record of a rule the team has since deleted (extended with the recreate) (e2e)
  ✓  everything else

round 17, new tests against b8df32f's build, then each fix disabled in place and rebuilt
  ×  resolves a retried agent removal through the record while the first removal is unmerged     (e2e)
  ×  removes a rule by the bare name it was published under a namespace with (record kept)        (e2e)
  ×  does not republish a removed agent from the copy an excluded tool kept                       (e2e)
  ×  cleans the flattened copy of a removed namespaced agent on pull, and never republishes it    (e2e)
  ×  scanLocalForPush does not publish the copy an excluded tool still holds                     (unit, agents)
  ×  scanLocalForPush reads a namespaced tombstone as the flattened stem once no namespace has it (unit, agents)
  ×  does not read rules from a tool this member excluded                                         (unit, rules)
  ×  tombstones only the published name, even when the record makes the root copy ours          (unit, rules)
  ×  proves landing only by commits after the revision the push branch was built on             (unit)
  ×  drops a record whose file was deleted and recreated between two checks                     (unit)
  ×  spends a placement unrecorded when its path was deleted and recreated before the first check (unit)
  ×  not in single-repo mode, where the checkout is the member's own                            (unit, pull)
  ✓  keeps the flattened stem live while another namespace still has that agent   (guard, green either way)
  ✓  everything else

round 18, each fix disabled in place
  ×  clears the checkpoint with the last record, so a later re-placement at that path is recorded      (unit)
  ×  keeps a record made in this run although the checkpoint predates an earlier deletion of its path (unit)
  ×  in single-repo mode, through origin/<default> rather than the member's checkout                  (unit, pull)
  ×  retires the flattened stem when the surviving same-stem agent is not active here                  (unit, agents)
  ✓  reads the default branch through a ref when the checkout is somewhere else    (reconcile with a ref, real git)
  ✓  changes nothing when the ref cannot be resolved                               (guard)
  ✓  everything else

round 19, new tests against 4f059f9's code
  ×  refuses to remove when the team clone cannot be refreshed                  (e2e, against 4f059f9's remove.ts)
  ×  keeps updating the open PR of a resource the flag does not move            (unit)
  ✓  everything else

round 20, new tests against 31207f1's agents.ts / pre-push-sync.ts / push-namespaces.ts
  ×  compares a deployed agent with its active source, not with the requested namespace's file (unit)
  ×  edits the shared-root agent it was deployed from, never a namespaced second copy           (unit, rewritten)
  ×  holds a recorded agent that changed on the team since this machine last synced it          (unit)
  ×  syncs a placement that landed after the last pull from the version it was added with       (unit)
  ×  rejects a namespace segment that is not a safe directory name                              (unit)
  ✓  everything else

round 21, the new tests against 665cd4b's production code (the rebased round-20 head)
  ×  leaves an open placement PR alone once a shared-root file takes the name                    (unit)
  ×  removes nothing and exits 1 (placement records cannot be saved)                              (unit, new file)
  ×  still pushes an unrelated rule when the only skipped agent needs a destination the project lacks (unit)
  ×  spends a placement with no blob instead of recording it because the path exists             (unit)
  ×  still scans .teamai/rules when enabledAgents is set, as single-repo init always writes it   (unit)
  ×  leaves a same-named agent alone on a member who never had the removed agent's namespace     (unit)
  ×  drops every record whose file the team has deleted (now also retires the agent records)     (unit)
  ×  keeps two active same-stem agents ambiguous even when a flag names one of them              (unit)
  ×  refuses to place onto a requested namespace's agent that was never delivered here           (unit, rewritten)
  ×  holds a root canonical source that is an older version of the placed file                   (unit, real git)
  ×  does not push an old copy of a placed rule over a newer team version                        (unit, real git)
  ×  does not hold a recorded agent whose local copy already matches the team file               (unit)
  ×  sends an agent awaiting review as a placement back to its PR, despite a same stem elsewhere (unit)
  ×  names the choice and --role instead of prompting when there is no terminal                 (unit)
  ×  proves landing only by commits after the base (now also: reported once, mark spent)         (unit)
  ✓  everything else
  and one regression caught by the e2e run of the first draft of this round, fixed before commit:
  ×  never records a placement whose PR was closed without merging, even with the branch kept   (e2e)
     → the rules stale sweep keyed on `placed`; it now spares any pending namespaced placement

round 22, the new tests against 859ac58's production code
  ×  refuses a bare stem that names agents in several namespaces                       (unit, remove)
  ×  removes exactly the agent a namespaced name gives                                (unit, remove)
  ×  resolves a bare stem that only one namespace has to that namespaced agent        (unit, remove)
  ×  stops placing a new resource on a stale clone even when that clone has no roles manifest (unit)
  ×  stops the push when the reconciled placement records cannot be saved             (unit)
  ×  drops a record whose file was deleted and recreated (now also retires it)        (unit)
  ✓  everything else

Unit tests added or changed:

src/__tests__/push-namespaces.test.ts     19 cases   axis map, path building, project resolution, record resolver
src/__tests__/push-role.test.ts           42 cases   placement, collisions, placement records (incl. the recorded base, and a push that stops when they cannot be saved), pending-PR conflicts incl. a shadowed pending placement, axis failures and skipped agents not blocking, non-interactive choice, manifest freshness, stale clone (with or without a roles manifest), dry-run
src/__tests__/rules.test.ts               15 cases   recorded re-push, record precedence, stale sweep spares the author's copy (recorded, awaiting review, or a spent pending mark), delivery onto the author's copy, shared-root guard, excluded tools not scanned
src/__tests__/pre-push-sync.test.ts        5 cases   record redirect and precedence, no record, record with a gone file, placement landed after the last pull
src/__tests__/agents.test.ts              33 cases   record scope and source order (active, then record), collision in a named namespace, ambiguity under a flag, stale recorded agent held only when changed, single-repo old copy held, pending placement routed to its PR, revocation, removal scope, single-repo sources incl. rename, flattened tombstones scoped to where the agent was delivered, excluded tools not scanned
src/__tests__/placement-records.test.ts   21 cases   record lifecycle: landed (blob in base..HEAD), no blob spent, landed with other content reported, recorded once, deleted after landing, deleted and recreated between checks, checkpoint cleared with the last record, same-run records exempt, read through a ref, awaiting review, not a placement, deleted upstream (all of them), shadowed by the shared root; isPlacement
src/__tests__/pull-placement-reconcile.test.ts  2 cases   pull reconciles the clone itself, and a single-repo member through origin/<default>
src/__tests__/remove-command.test.ts       4 cases   remove stops when the placement records cannot be saved; `<ns>/<name>` removes one agent, an ambiguous bare stem is refused, a unique one is qualified
src/__tests__/self-mode-push-scan.test.ts  2 cases   .teamai/rules scanned despite enabledAgents; an old copy of a placed rule is not pushed
src/__tests__/roles.test.ts                2 cases   unreadable manifest vs missing manifest
src/__tests__/pull-agents-role-filter.test.ts  3 cases   recorded agents delivered, active stem wins, mismatched record ignored
src/__tests__/remove.test.ts               8 cases   publishedNameFor, removal scope, tombstones, unrelated basename left alone

Evidence

Before, from the issue:

<repo>/.claude/rules/my-rule.md
  -> teamai push --project front-app     flag applies to skills only
  -> rules/my-rule.md                    written with no first path segment
  -> teamai pull                         slashIndex === -1 returns true
  -> every member of the team

After, from the branch that push created:

agents/fe-agents/vr.yaml
rules/fe-know/my-rule.md
skills/fe-skills/my-skill/SKILL.md

The same case as a test, red before the change and green after:

push({ all: true, project: 'front-app' })
  rules  → rules/fe-know/my-rule.md      was rules/my-rule.md
  skills → skills/fe-skills/my-skill     unchanged
  agents → agents/fe-agents/vr.yaml      was agents/vr.yaml

Related Issues

Part of #649, the remainder left open after #653 and #654. No closing keyword, matching how those two were handled. Please close #649 only if you agree nothing else remains.

Notes for Reviewers

Behaviour change worth a decision. With no flag at all, a new rule or agent now resolves from the primary role's knowledge and agents namespaces, mirroring what new skills already did. The consequence is that an author whose role declares a knowledge namespace can no longer push a rule that reaches the whole team, because every new root-level rule is placed and --role has no value meaning "shared". Skills have the same limit today, so this is consistent, but it is new for rules. Say the word and I will add a "shared" entry to the prompt for rules and agents.

--project resolution is per type. The skills axis resolves against the scan rather than the selection, because the same override also relocates modified skills and the listing has to show where they go, but the failure is held until the selection proves a skill is going out. Deselect the skill and a rules-only push still goes through.

The project id is validated as soon as the manifest loads, so --project typo-id fails even on a push where nothing needs placing.

Review round 2. Three more changes, all from the last review. A roles manifest that EXISTS but cannot answer — unparseable, or missing the configured role — now stops the push instead of returning an empty namespace list, because that fallback sent a new rule or agent to the shared root and therefore to the whole team. loadRolesManifest throws a tagged RolesManifestNotFoundError so an ABSENT manifest still keeps the pre-manifest convention. syncTeamUpdatesToLocal follows the same placedRules record the scanner does, so a root-authored rule placed under rules/<ns>/ takes part in the three-way sync rather than being pushed over a teammate's newer version. And --dry-run now runs the recorded-destination and placement steps before it exits, so it reports where every new resource goes and fails on the same unresolvable project axis the real command refuses.

Review round 3. Three findings about the placement itself, each reproduced against the real CLI before being fixed. A placement record only ever meant "push put this here", and the two sides that read one disagreed about how much it was worth: placedResourcePath is now the single resolver, validating the record (inside the resource root, namespaced, no traversal, named after the resource), and both the push scanner and the pre-push sync go through it. The record also takes precedence over a shared-root file that appears later with the same basename, since mapping the author's copy onto somebody else's rule would push their content over it. Agents gained the analogue, placedAgents: AgentsHandler.scanLocalForPush only accepts a team source whose namespace is ACTIVE in this directory, so an agent published with --role/--project was skipped as "no active source" on the author's very next edit — they could create it and then never maintain it. And teamai remove rules <name> resolves the same record through a new publishedNameFor hook: the author's copy stays at the rules root, so the name they type is the bare one, and remove used to answer "not found" about a rule it had recorded publishing. It now reports which name it resolved to, deletes the namespaced team file, and takes the author's root copy with it — left behind, that copy re-publishes the rule on the next push.

Review round 4. Four findings, all about the placement record rather than the placement: what it grants, and when it is written. remove consulted it only after a bare-name match failed, but the local scan contributes the bare name whenever the author's own copy has edits — so teamai remove rules my-rule deleted that copy, reported success, and left rules/<ns>/my-rule.md published. The record is resolved first now. A record is written only for a resource push actually placed, meaning new and namespaced by this run: recording a modified agent turned a namespace that happened to be active at edit time into standing permission to keep editing that agent long after the role or project granting it was dropped. Records are persisted per group, right after that group reaches the remote, rather than after every group completes — a failing later group returned early and took the earlier group's mapping with it, so a resource that was pushed came back misclassified once its PR merged. And the roles manifest is held to the same rule as --role and the projects manifest, that a namespace is one path segment: foo/bar wrote an agent below the depth pull looks at, and read back as namespace foo for a rule.

Review round 5. AgentsHandler.scanLocalForPush picked which team file a local agent is an edit of by activity alone, and it runs before the destination is resolved — so an agents/other-ns/vr.yaml this directory never activates made push --project front-app report "no active source" and push nothing, although the layout allows the same stem in several namespaces and the flag had named a different one. The scan now takes that namespace, through a new optional ScanForPushOptions: with one named, sources in other namespaces are other agents, and an absent one means this agent is new there. A shared-root copy still blocks, and now says why — both would be active at once, which is the collision pull reports and skips. RulesHandler.removeItem swept the bare basename unconditionally, so teamai remove rules fe/foo deleted an unrelated personal .claude/rules/foo.md; the bare copy is only ours to delete when this machine's placement record says the two are the same rule. And teamai push --help described both flags as targeting skills.

Review round 6. Placement rewrote a new root-level resource to the resolved namespace without looking at what was already at that path. An unrelated local foo.md — which the scanner rightly calls new, since no record maps it anywhere — landed on rules/<ns>/foo.md and replaced somebody else's rule, silently, in a run they never reviewed. Push stops and names the file now, and the same guard covers the --role/--project skills override for new skills only, since a modified one is meant to land on its own directory. This is the hole flagged in the first round's comment and left as a follow-up; that was wrong for rules and agents, which could not collide before this PR placed them at all. loadRolesManifest read through readFileSafe, which answers null for every failure, so a manifest that exists but cannot be read arrived looking exactly like a missing one — and a missing one is the pre-manifest layout, which sends new rules and agents to the shared root. The two are told apart now, and an empty manifest reaches the parser and fails as invalid rather than as absent. And RulesHandler.removeItem tombstoned only the name it was given: removing through a placement record means the author's source is named <name> while the published file is <ns>/<name>, and the local sweep skips excluded tools, so a root copy could outlive the removal there and come back on the next push. Both names are tombstoned when the record vouches for the bare one. (Superseded in round 17: the bare tombstone swept other members' rules of that name, so only the published name is tombstoned, and excluded tools are no longer a push source.)

Review round 7. remove asks every handler for the published name, but only rules answered it. So teamai remove agents vr matched the bare stem, deleted every vr in every namespace — other people's agents included — and left the namespaced placedAgents record behind, since the record is keyed by a path the removal never named. AgentsHandler.publishedNameFor resolves it now, and the existing sweep already narrows a <ns>/<stem> to exactly one file, because the root directory is one of the directories it probes; the bare stem is tombstoned alongside the published one and swept from the tool directories, the same way rules are. The placement collision check tested the proposed path alone, but pull reads a legacy <stem>.md as the same agent as <stem>.yaml, so a new .md landing beside an existing .yaml passed the check and left two copies answering to one name; agents are checked under both canonical extensions.

Rebased onto 9d8ed1f, which matters here: CI runs as root, where chmod 0o000 is a no-op, and that commit establishes it.skipIf(process.getuid?.() === 0) for exactly that case. The manifest-readability test added in round 6 uses chmod, so it now carries the same gate and restores the mode in a finally — without it the test would have passed on CI while asserting nothing.

Review round 8. Round 7 added AgentsHandler.publishedNameFor, but remove only used its answer when allNames also carried that spelling — and scanTeamForPull reports an agent by its bare stem, never <ns>/<stem>. The resolution was therefore inert on the real command path: teamai remove agents vr fell back to the bare stem and deleted every vr in every namespace, exactly as before the fix. The cross-check is gone, since publishedNameFor has already proved the file is in the team repo — stronger evidence than membership in a list each type spells differently. The bare-stem tombstone from that round went with it: agents deploy FLATTENED, so both the push scan and the post-pull cleanup read a bare tombstone globally, and removing fe/vr suppressed and deleted be/vr the moment that namespace became active. Only the published name is tombstoned now, and the author's own flattened copy is swept only where this machine's record says the file just removed is where push put it — without that, the copy on disk may be another namespace's deployment. Rules can carry the bare spelling because they keep their namespace directory locally; agents cannot, and that distinction is now written where the tombstone is created.

The round-7 miss was a testing gap, not a reasoning one: both halves were unit-tested and the join between them never was. The new coverage drives teamai remove agents vr through the built CLI instead.

Review round 9. --project <id> resolved the agents destination before scanning and dropped the failure on the floor, so a project with no agents namespace looked identical to a run with no flag at all: the scan skipped the agent as "no active source", the item never reached placement, and the command exited 0 with "No new or modified resources" — on a flag it could not honour. The error is carried forward and raised as soon as the scan contains an agent. It deliberately does not wait for the selection the way the skills axis does: the item that would prove the axis is needed is exactly the one the scan removes, so deferring means never raising it. The cost is that a modified agent already in an active namespace is also stopped under a --project whose axis is empty; that is a clear message rather than a silent drop, and threading the unresolved destination into the scan is the alternative if the stricter reading is preferred.

placedResourcePath matched the recorded filename by prefix, so a record pointing at rules/<ns>/foo.backup.md was trusted whenever that file existed, and scanning, the pre-push sync and removal would all follow it onto somebody else's file. The filename must now be exactly the resource's own — <name>.md for a rule, <name>.yaml or a legacy <name>.md for an agent.

Review round 10. Four findings, all on the agent side of placement.

The single-repo canonical source in .teamai/agents/ is picked up directly, never reverse-parsed, and that branch ignored the placement record: a root vr.yaml placed at agents/fe/vr.yaml read as new on the next push, and the collision guard then refused the very agent this machine published. Removal missed the same directory, so the agent republished itself — which a bare-stem tombstone cannot prevent without suppressing that stem in every other namespace, since agents deploy flattened. That is the tension between this round and round 8: the tombstone format cannot express namespace-scoped removal for a flattened type, so the fix is to delete the source that would republish rather than to suppress it. A copy in an excluded tool can still survive, but isAgentExcluded gates every write in this codebase and that policy is not this PR's to override. (Round 17: that copy is still left alone, but the push scan no longer reads excluded tools, so it cannot republish.)

The project agents-axis error now counts only agents that actually need a destination, through an explicit needsDestination flag the scan sets when it finds no writable source. A modified agent already in a namespace is written in place, so an empty agents axis is none of its business; blocking it contradicted the rule that only new shared-root resources are placed.

And the record is no longer taken as licence to overwrite. It admits a namespace this directory never activates, which also means pull never refreshed a copy of it and syncTeamUpdatesToLocal does not cover agents — so if the canonical file moved on since the last pull, push says so and asks for a pull instead of writing a stale rendering over whoever changed it. This is a guard, not a synchronization: agents have no three-way pre-push sync at all, which predates this PR and affects active namespaces equally. The path this PR opened is closed; the general gap is left for its own issue rather than widening this one.

Review round 11. The staleness guard added in round 10 was defeated by the pull it recommended: pull advances lastPullRev without deploying an inactive namespace, so the next push saw an unchanged canonical and wrote the stale rendering anyway. It also never fired right after the first PR merged, when the file did not exist at lastPullRev and the comparison had nothing to compare against.

The guard is gone, and the cause with it. pull now delivers an agent whose placement record names it, so the local copy tracks the team file and the ordinary comparison is valid — the inactive case stops being special instead of acquiring its own machinery. This is a different route from the one the finding proposed (persist a per-resource baseline): the record already means "this author maintains this agent", and pull simply had never honoured that, which is what let the copy go stale. A stem an ACTIVE namespace already claims is left alone, since agents deploy flattened and the active one is what is deployed here; the scan follows the same order, treating the record as a fallback rather than an extra candidate. The guard, its two tests and the getFileContentAtRev import all went with it.

Worth weighing: this changes pull, not only push. A member who published an agent into a namespace they do not activate now receives that agent on pull. It is the same file they already have locally, so it is not new exposure, but it is a wider blast radius than the earlier rounds.

Pending-PR reuse matched on type and name alone, so an open PR holding a DIFFERENT resource of the same name captured a push that named another namespace and force-pushed into that review. Neither silent answer is safe — honouring the PR ignores the flag the user typed — so the flag decides, the open PR is left untouched, and the collision is reported. This supersedes the original #331/#654 rule that a PR's destination always won; that rule still holds whenever no destination is named, and its test was updated with the reasoning rather than removed.

Fallout fixed along the way: three doctor test files mocked ../config.js without loadStateForScope, which resolveDesiredAgents now needs — 19 failures that were missing mock surface rather than broken behaviour.

Rebase onto cd3e0e6. The branch conflicted with #700, which moved matchesRoles and the logger out of src/roles.ts into the new src/membership.ts. The two hunks were import lists only: roles.ts keeps main's imports plus the pathExists this branch needs for the unreadable-manifest check, and roles.test.ts drops the stale matchesRoles import that #700 had already removed from the module. No behaviour on either side changed; the placement code does not touch what #700 scopes (hooks, MCP, env).

Self-review, then round 12. Before the next review pass I audited the whole branch adversarially, asking of every namespace list "what else reads this?". That caught three defects, all introduced by round 11: pull delivered an agent through its placement record and cleanupInactiveNamespaces — which knew nothing about records — deleted it again in the same run, byte-equal to the render so the data-safety gate passed it through, leaving the round-11 fix inert and the file churning on every pull; --dry-run grouped against the unfiltered pending list, so it reported a destination the real push no longer used; and the partial-selection warning counted entries the run had already declined to reuse. Delivery and revocation now resolve through one exported selectAgentsForDirectory, the same "one resolver, two callers" treatment placedResourcePath already gives the push scanner and the pre-push sync.

The review round that followed reported those first two independently, against the commit that predated the audit. Two further findings were new, and both are worth naming because the audit missed them.

pullAllRules sweeps a local rule whose name is absent from the desired set. A rule published into a namespace keeps the author's copy at the rules ROOT under its bare name, while the desired set holds <ns>/<name> — or nothing at all when that namespace is not active here — so the sweep deleted the author's own file, local edits included. This is the same bug class as the agent one the audit did find; it was missed because the audit only asked the question of the code round 11 had touched, and rules had been placed since round 1. The placement record now marks that copy as the author's, and only while the team file it points at still exists.

Pending-PR conflict detection trusted PendingPushItem.namespace, but two of the three agent item constructors record a namespaced destination without setting that field, so those entries slipped past the check and a push naming another namespace could force-push into the PR under review. The audit checked that the consumer read the field correctly and never checked that every producer wrote it. The namespace is now derived from the recorded path when the field is absent, and the scan sets the field as well — it was also the only thing telling pendingNamespaceFor where to put the resource.

Docs updated in both languages: docs/usage-guide.md, docs/usage-guide.zh-CN.md, docs/designs/multi-project-management.md, and skills/teamai/references/contribute-member.md. The README*.md files do not describe push placement, so they are untouched.

Review round 13 (first review of the rebased branch). Four findings, none caused by the rebase.

The agents-axis check from round 9 fired before the listing for every agent that needed a destination, including a NEW one. A new agent is listed, so the user can deselect it — failing early blocked a rules-only --project push on an agent that was never going out. The early check is now limited to the case round 9 was actually about: an agent the scan itself skipped (needsDestination), which never reaches the listing, so deferring its error would mean never raising it. A new agent that stays selected fails in step 4 with the same message, which is the skills-axis order.

manifest/projects.yaml was read in push, before pushCore pulled the clone, so a project whose namespaces changed on the remote placed this run's resources by the previous pull's mapping. The read moved into pushCore after the pull; in self mode that is the fresh worktree. The --role/--project exclusivity check stays up front, since it needs no manifest.

The round-11 conflict check answered "no conflict" for a recorded path with no namespace segment, so an explicit --role/--project reused a shared-root PR's branch and rebuilt it with the namespaced path — the scope of a review the user did not name went from "everyone" to one namespace, silently. The shared root is a destination like any other now, and conflicts with any namespace the flag names. The e2e case for this exposed a one-second window in generateBranchName: two pushes in the same wall-clock second are given the same branch name and the second lands on the first for that reason alone, which is what made the case fail once in three runs. The test waits the second out; the window itself predates this PR and is left alone.

pull delivered a rule this machine had placed at <tool>/rules/<ns>/<name> beside the author's copy at the rules root, so a tool that loads rules recursively applied both, and they disagreed as soon as the team file moved on. The round-12 fix only spared the root copy from the sweep; it never asked where delivery should go. deliveryTargets now resolves the local name through the placement record — the root copy when this machine placed the rule there, the namespaced path otherwise — and pullItem removes the namespaced duplicate an earlier pull wrote (DeliveryTarget.supersedes). doctor reads the same targets, so it checks the copy pull actually writes. A record for a different namespace is not this rule's and changes nothing. The round-12 test that asserted "local edits preserved" now runs with that namespace inactive, which is the only case in which pull leaves the root copy alone; with it active, pull updates the copy the same way it updates every other delivered rule.

Review round 14. Three findings.

A failed refresh of the clone in pushCore was warned about and the run went on, so under --project every destination came from the previous pull's manifest/projects.yaml — round 13 moved the read after the pull, but a pull that failed leaves the same stale file in place. --project now stops with exit 1 and names --role <ns> as the way through. The same gate covers a new resource that would resolve from manifest/roles.yaml, since that manifest is read from the same clone; a team with no roles manifest at all resolves from nothing that can go stale and keeps its pre-manifest behaviour, so a flaky network does not start blocking pushes for teams the feature never touched. This is a behaviour change for the roles-manifest case that the finding did not ask for; I would rather have one rule for "where does a new resource go" than two.

In self mode the record redirected a root canonical agent to the recorded file, extension included, while pushItem derives the extension from the source. Rewriting vr.md as vr.yaml therefore wrote the .yaml, staged the .md, and pushed nothing. The destination keeps the record's directory and takes the source's extension; the retired file is carried on the item as supersedes, deleted by pushItem, staged by pushGroup (it is tracked, so staging its path stages the removal), and the record moves to the new path when the branch lands — otherwise the next scan finds nothing at the recorded .md and reads the agent as new.

Placement records had no lifecycle: written when a branch reached the remote, never removed. Closed unmerged, or deleted upstream later, a record pointed at nothing until another member created that path, at which point it came true again. prunePlacementRecords drops a record whose target is neither on the default branch nor awaiting review on a branch origin still has; push runs it after the pull and before the scan, pull after its refresh and before delivery, so nothing reads a record the current tree does not back. A record is kept when origin cannot be asked, the same rule prunePendingPushes follows, and push skips the cleanup entirely on a stale clone, since a file missing from an unrefreshed tree proves nothing. The residual window is between the PR closing and the author's next push or pull, which is the cadence everything else in the tool works at. (Superseded in round 15: the record is no longer written before the PR merges, so there is no window.)

Review round 15. Three findings, one of which changed the design.

The third — a closed PR whose branch was kept counts as "awaiting review" forever — has no local fix, because no provider here can say whether a PR is open, and adding that to six providers (the plain git one cannot answer at all) is the wrong size of change for this PR. So the record's timing moved instead. Push no longer writes placedRules/placedAgents at all; it marks the placement on the pending PR entry (PendingPushItem.placed) together with the git blob of the file it wrote (blob). The record is written by a reconcile pass — reconcilePlacementRecords, run by push, pull and remove after their pull and before anything reads the records — only when that blob is in the default branch's history for the path: the PR merged, whether by merge commit, squash or rebase. A PR closed unmerged therefore leaves no record, branch or no branch, and the path merely existing on main is not enough, which closes the "another member later creates that path" case the finding named: that e2e case pushes, never merges, has a teammate create the very path, and checks the teammate's rule is delivered to its namespace with the author's root copy untouched. Nothing changes while the PR is open: every record consumer already required the target to exist on main, so a record was inert during review anyway, and the pending entry is what routed the author's edits back to the PR (reuseRecordedDestinations, since round 3). The one consumer that did rely on the early record was the stale sweep in pullAllRules, which now also spares a root copy whose placement is on a pending entry. The remoteBranchExists check from round 14 is gone; remove gained the reconcile pass because it may be the first command to see a merge, and its e2e cases showed that.

The first: localNameFor redirected a recorded rule onto the bare root path without asking whether a shared-root rule of the same name was being delivered too, so both landed on the one file in loop order — and with the namespace inactive, the shared rule always won, and the next push followed the record and carried it over rules/<ns>/<name>.md. Delivery keeps the namespaced path whenever rules/<name>.md exists (the shared root owns the root path in every tool directory), and the reconcile pass withdraws the record with a warning, because the author's root copy follows the shared rule from then on and no longer stands for the namespaced one. The e2e case checks both halves, including that the next push of the root copy goes to rules/my-rule.md and leaves the namespaced file alone.

The second was a plain bug in round 14's prune: destructuring from the original map on every iteration put back what earlier iterations had removed, so only the last stale record actually went. The kept entries are rebuilt in one pass, and the test has three stale rules and two stale agents.

Docs: the two usage-guide bullets on records (both languages) now describe the landed-only timing and the shared-root withdrawal.

Review round 16. One finding, a hole in round 15's own design: recording a landed placement left the placed mark on the pending entry. Had the team then deleted the file — which drops the record — and another member recreated the path, the next reconcile recorded it again, because the path existed and the blob push had written was still in the default branch's history, so both landing checks passed. The mark and the blob are cleared when the record is written, so a placement is recorded exactly once; the pending entry itself still lives and dies by prunePendingPushes as before. The e2e case for the deleted rule now goes one step further and has a teammate recreate the path.

Review round 17. Three findings, all about records that ran ahead of the default branch.

remove dropped the placement record right after pushing its branch, while the deletion was still only on that branch. A retry during review, or after PR creation failed, could no longer resolve vr to fe-agents/vr, fell back to the bare stem, and AgentsHandler.removeItem('vr') deleted every same-stem agent in every namespace. remove no longer touches the records. The reconcile pass drops a record once its file is gone from the default branch, which is when the removal has actually landed, and a removal PR closed unmerged leaves the record in place. The rule-removal e2e case used to assert the record was gone straight after remove. That assertion encoded the bug, so it now checks the record survives the unmerged removal and is gone after merge and pull.

Keeping the record opened a window the old code did not have: the removal merges, another member publishes their own resource at the same path, and the record then claims it. The reconcile pass now stores the default-branch commit it last checked the records against (placementsCheckedAt). A record whose file was deleted in placementsCheckedAt..HEAD is dropped even if the path exists again. The same check covers the placement side: a placement whose path was deleted after base is spent without a record, because placement refuses an occupied path, so that deletion came after it landed.

The landing proof matched the pushed blob anywhere in the path's history, so identical content that sat at that path before the push counted as this PR having merged. Each pending entry now records base, the full default-branch commit its branch was built on (getHeadCommit just before pushRepoBranch), and blobInHistory searches base..HEAD only. A force-push update of an open PR rebuilds from the current default branch and records the new base. A PR-creation retry that pushes nothing keeps the entry's previous base and items, because the retry has no branch checked out to hash, and recomputing lost the blob.

The excluded-tool finding was one symptom of a wider gap, which a self-review of this round turned up with reproductions against the built CLI. Removing a namespaced agent tombstones only fe-agents/vr (round 8, so that be/vr survives). But agents deploy flattened: every member holds that agent as <agents>/vr, the tombstone never matched that copy, and the next push republished it as new, for the author and for every teammate with the namespace active. AgentsHandler.removedStems reads a namespaced tombstone as the flattened stem while no namespace still has an agent of that stem. Both the pull tombstone cleanup and the push scan use it, so the copy is cleaned and never pushed again. While another namespace still has the stem, the flattened copy may be that agent's, so round 8's rule holds. The consequence matches a bare tombstone on main: nobody can publish a new agent of that name until the tombstone line goes. (Refined in round 18: the test is what this directory receives, not whether the stem exists anywhere.)

The push scans for agents and rules now skip excluded tools, the same gate removeItem, pull and doctor already apply. The agents canonical-merge branch loses its now-redundant copy of that check. The cost is that a rule or agent authored only in a tool excluded on this machine is no longer pushed from it. That is consistent with uninstall --agent <tool>, after which teamai neither writes nor deletes there. With excluded copies no longer a source, rules no longer need round 6's bare tombstone. That tombstone was read by every member, so removing fe-know/my-rule also swept and suppressed any unrelated root my-rule, and a shared-root rules/my-rule.md never reached anyone. Only the published name is tombstoned now. The round-6 unit test asserted the bare tombstone, so it is rewritten to assert its absence.

Single-repo pull no longer reconciles placement records. Its refresh leaves the member's own checkout alone, a feature branch or an un-pulled main, so a record it dropped against that tree never came back. push and remove still reconcile, in a fresh origin/<default> worktree. (Superseded in round 18: pull reconciles again, through the ref.)

Known limitations, not addressed here:

• A placement PR that a reviewer amends before a squash merge puts a different blob on main, so the placement is never recorded. The author's next push treats the root copy as new. With --project or --role naming the same namespace, the collision guard stops it and names the file. Without a flag, it is placed again, and the destination is printed. Closing this needs the pending branch's tip fetched during pull, which is network work on every session start. (Round 21: still not recorded, but no longer silent. Reconcile says once that the file arrived with other content, and names it.)
• A rule author whose namespace is active, who uses JoyCode, OMP or Copilot, and who pulls while their removal PR is open, gets the root copy redelivered. Those tools' rule directories are swept only by tombstone, so that copy survives the merge and would be listed as new on the next push.

Docs, both languages: the records bullet in usage-guide says remove leaves the record until its deletion lands. The enabledAgents paragraph says push reads no rules or agents from an excluded tool. The agents section describes how a namespaced removal reaches flattened copies.

Review round 18. Three findings, all about round 17's own additions.

The checkpoint was advanced only while a record survived, and step 2 checked every record against it, including one step 1 had just made. Take a record at a path, then its removal merges: the record goes, and placementsCheckedAt stays behind. When the author later placed a resource at that path again, the new record was compared with the old checkpoint, the earlier deletion showed up, and the record was dropped on the spot. The checkpoint is now cleared with the last record. A record made in the current run is exempt from it, since step 1 has already checked that record against its own base.

Round 17 took the reconcile pass out of single-repo pull so it would not read the member's own checkout. As a result, a merged placement stayed unrecorded until the next push or remove, and delivery went on without the record in the meantime. reconcilePlacementRecords now takes an optional ref, and single-repo pull passes origin/<default>. Existence is read as <ref>:./<path> and history as ..<ref>, so the checked-out branch never enters into it. origin/<default> is whatever the business repo last fetched. That is always a real state of the default branch, possibly an old one, so at worst a placement is recorded late. A ref that cannot be resolved changes nothing: read as "every file gone", it would drop every record.

removedStems retired the flattened stem only when no namespace had it at all. With fe/vr removed and an unrelated be/vr still present, an fe member kept the removed agent. The later inactive-namespace pass compared that copy with be/vr, found it different, and preserved it as "modified". The question is now what this directory is meant to hold: the stem is retired unless the directory still receives an agent of that name, through the same selectAgentsForDirectory selection that pull delivers with (active namespaces plus records). A be member keeps their vr, and an fe member's copy is cleaned.

Review round 19. Three findings.

remove ignored a failed refresh and reconciled the stale clone as if it were the default branch. A placement merged since the last pull was then never recorded, so remove agents vr fell back to the bare stem and removed vr from every namespace, and the push URL could still carry that removal to the remote. remove now stops with exit 1, says the clone could not be refreshed, and removes nothing. Skipping only the reconcile, as push does, would not have been enough: the fallback to the bare stem is the harm, and it needs the fresh tree to be avoided. The e2e case makes the fetch URL unreachable while pushing still works, which is the case where the old code reached the remote.

Under --role/--project, an open PR counted as conflicting whenever its recorded namespace differed from the flag's. Only some items are actually moved by the flag: every selected skill, and a rule or agent only while it is new and at the shared root. A modified rule already in fe-know kept its path, but its open PR was treated as conflicting, so the edit went to a second PR on the same file. Conflicts are now judged on the scanned item: one the flag does not move keeps updating its open PR.

The Test Plan named 16db08e while the head was 4f059f9: the body was updated after that push, so the review ran on the previous one. This time the whole record above was produced on 31207f1 before it was pushed, and the body was updated before the push too.

Docs, both languages: the remove bullet says a failed refresh stops the command with nothing removed.

Review round 20. Three findings.

With --role/--project, the requested namespace always chose which team file a local agent was compared with. That was round 5's rule, and it went too far. A vr delivered from the active common namespace was compared with fe/vr, differed, and was pushed over it without any local edit. Candidates now follow delivery order: an active source first (the shared root always counts as active), then the one this machine's record names. Only when neither exists does the flag decide, and the agent is then new in that namespace. Round 5's case, a same-stem agent in an inactive namespace, is unchanged. Two of its unit tests claimed to test an inactive namespace but ran with no roles, where every namespace is active, so they now set up a projects manifest that makes it true. Round 5's shared-root refusal can no longer be reached, because the root copy is always active and is edited in place. Its test now asserts that edit.

A record becomes active as soon as the merge is seen, but lastPullRev may still predate the file. The pre-push sync then found no base version and skipped the rule. If a teammate edited it before the author's next pull, the stale root copy went up over their edit. A rule reached through its record now takes the version the file was added with (getFileContentWhenAdded) as its base, which is the content the author's copy started from. Agents have no pre-push sync, so a recorded agent whose team file has moved past that baseline (the lastPullRev version, else the added version) is held, with a message to run teamai pull and reapply. That is round 10's guard brought back narrowly, now that round 11 makes pull deliver recorded agents: after a pull the baseline moves and the guard lets the edit through. Active agents still have no pre-push sync, as on main.

placedResourcePath rejected . and .. segments but never checked the namespace segment itself. In rules/..\..\victim/foo.md the backslashes are separators on Windows, so path.join walked out of the resource root. The namespace must now pass isSafeNamespaceSegment, the same check push applies when it writes one.

Docs, both languages: the --role/--project agent bullet describes the source order and the pull-first hold.

Not verified end to end (round 18): there is no single-repo e2e fixture in this repo, so the ref path is covered by a real-git unit test (a feature branch cut before the merge, reconciled through main) and by the pull test that checks the ref passed.

Rebase onto a52374a. As requested, the branch is rebased onto current main (#711/#713, #718, #736, #739). Only one conflict came up: remove.test.ts's vi.mock('../config.js'), which main changed to spread importOriginal. Both changes are kept. git range-diff shows no other change to the 23 earlier commits beyond test-mock context. Main's isInteractive is now used by the namespace prompt (below), and the five push-related test files that mock utils/prompt.js gained it.

Review round 21. Four findings from the bot, and six more from an adversarial pre-review of the whole branch, all fixed in 859ac58.

From the bot:

• A rule or agent awaiting review in a namespace, whose name a shared-root file then takes, was scanned as modified at the shared-root path. The open PR, matched by type and name, would then be rebuilt with the author's copy over the shared file, and the namespaced change dropped. That copy is now left out of the push with a warning, and the open PR stays as it is. This is the rule reconcile already applies to a record: the shared root owns the name in every tool directory. The bot also suggested restoring the recorded path instead. I chose not to, because after pull the root copy holds the shared file's content, so either reading of what the author meant is a guess.
• remove swallowed a failed reconcile or state save and went on to resolve names from disk. A record that should have been written was then missing, and the bare stem removed the agent from every namespace. It now stops with exit 1 and removes nothing.
• Under --project with no agents axis, one agent skipped for want of a destination (a stale edited copy from a dropped role, say) failed the whole push before selection. It is now skipped like any other agent, with the axis message as a warning. The error remains when nothing else would go out, which is round 9's case.
• toPendingItems marked a placement even when hashObject failed, and reconcile then recorded it on the path existing. A placement is now marked only together with its blob, and a mark without one is spent rather than recorded. The reconcile unit tests that relied on existence-only landing now commit real git history.

From the pre-review:

• The rules scan's excluded-tool gate from round 17 also skipped the single-repo .teamai/rules source whenever enabledAgents was set. Single-repo init always sets it, so rules authored there stopped being pushed. That source is not a tool and is now exempt, via SELF_KNOWLEDGE_SCAN_KEY, which moved to types.ts.
• Round 18's removedStems retired a flattened stem on members who never had the removed agent's namespace. On their next pull their own agent of that name was deleted, and it could never be pushed again. The stem is now retired only where the agent could have been delivered: its namespace is active, nothing is filtered, or this machine placed it. After the removal merges, reconcile drops the author's record, so it keeps that record as retiredPlacedAgents and the author's own copy still counts.
• Round 20 let a flag choose between two active same-name agents. The untouched copy delivered from one could then be pushed over the other. Two active sources are ambiguous again, flag or not.
• --role <ns>, where <ns> already holds an agent of that name that was never delivered here, overwrote that agent. Rules refuse the same case, and agents now do too.
• In single-repo mode nothing refreshes the author's root copy under .teamai/ after placement, so after a teammate's edit (and even after teamai pull) that copy went back over it. A root copy equal to an older version of its placed file is now held as stale, for agents and rules alike (isPastVersionOf).
• P2s, fixed per the rule for this round:
  • The recorded-agent hold runs only when the copy actually differs, and tells you to set your edit aside before pulling.
  • An agent awaiting review as a placement goes back to its PR instead of being skipped as having no active source.
  • When a flag does not move an edit of a shared-root rule or agent, push says so.
  • Several candidate namespaces with no terminal now fail with the list and a --role hint, instead of throwing inside the prompt.
  • A placement that reached the default branch with other content is reported once.
  • hashObject got back its own JSDoc.

The first run of the full e2e suite on this round caught a regression of my own. Spending a mark made the rules stale sweep stop sparing the author's root copy while the PR was still pending, so that copy was deleted. The sweep now spares any pending namespaced placement of a root rule, whether or not it is still marked. The e2e case "never records a placement whose PR was closed without merging" covers this, and so does a new unit test.

Still open, as in round 17: the JoyCode/OMP/Copilot rule residue, and the absence of a pre-push sync for agents in active namespaces, which predates this PR. Not verified end to end: single-repo mode, which still has no e2e fixture. Its new paths are covered by real-git unit tests.

Review round 22. Four P1s from the bot, all fixed in 23717ec:

• push still swallowed a failed reconcile or save of the placement records. The pre-push sync and the scan read the records back from disk, so a record that should have been withdrawn, because a shared-root file now shadows it, could still redirect the author's copy onto the namespaced file. push now stops with exit 1 and pushes nothing, the same rule remove got in round 21.
• On a stale clone, a new resource placed without a flag was blocked only when the clone already had manifest/roles.yaml. This is round 14's exemption, and it is gone. A manifest's absence is also clone state (one added remotely would move new rules and agents off the shared root), and so are the skills namespaces detected from the tree. Every unflagged placement now stops on a stale clone and asks for --role <ns>. This is stricter than round 14 for teams without a manifest. A push whose own refresh fails will usually fail to push its branch anyway.
• remove offered namespaced agents only by their bare stem, because the team scan reports agents that way. A machine without the placement record could only type vr, which removes vr from every namespace. remove agents <ns>/<name> now names one agent. A bare name that only one namespace has resolves to that agent, so its tombstone is qualified too. A bare name found in several places is refused, with the qualified names listed, and nothing else in the same command is removed.
• The drop path for a record whose file was deleted and recreated did not write retiredPlacedAgents, unlike the path for a file that is simply gone. So the author's flattened copy of the removed agent went unrecognised. Both paths now retire the record.

push-skill-flag.test.ts needed its git mock completed, with isDedicatedRepoRoot, getDefaultBranch, getFileContentAtRev and getHeadCommit. Without them the refresh threw inside push and the clone counted as stale, which the stricter rule now correctly refuses to place from. No assertion changed.

Docs, both languages: the stale-clone bullet covers every unflagged placement and an unsaved record state, and the agents section explains how remove agents names one agent.

[github-actions]

• [P1 blocking] Avoid matching unrelated root rules by basename alone — src/resources/rules.ts:105 treats any root-level local foo.md as a modification of the sole active rules/<namespace>/foo.md. Namespaced rules are normally pulled into a namespaced local directory, so another user can legitimately have an unrelated root rule with the same basename. With --all or silent mode, this silently overwrites the existing team rule. Require durable provenance or otherwise distinguish the original pre-push copy before redirecting it.
• [P1 blocking] Complete the required pre-PR verification — The PR description checks “npx vitest run passes” while reporting three failures, omits npm run build, and explicitly says GitLab and GitHub were not exercised. The repository instructions require build plus real-CLI E2E coverage for git, gitlab, and github; update the test record with actual passing results for the required matrix.

[SaulMoro]

Both items addressed in d0ab295. PR body's Test Plan rewritten to match the actual record.

P1 — basename matching in RulesHandler.scanLocalForPush

Agreed: a namespaced team rule is pulled into a namespaced local directory, so a root-level local rule with the same basename is not evidence of anything. The basename index is gone, and with it the "more than one active namespace" warning.

The provenance is now durable and machine-local. When push places a root-level rule under rules/<ns>/, it records the destination in state.json:

 StateSchema
+  placedRules?: Record<ruleName, teamRelPath>   // { "my-rule": "rules/fe-know/my-rule.md" }

The scanner redirects a root-level local rule only when that record exists and the recorded team file is still present. Otherwise the rule is new, and step 4 places it (or warns that it stays at the shared root).

root-level local rule <name>.md, no rules/<name>.md in the team repo
  placedRules[name] recorded and file exists   → compare against it (modified / identical)
  record missing, or its file is gone          → new, at rules/<name>.md

Consequences:

• Another member's unrelated foo.md never matches rules/<ns>/foo.md: their machine has no record. Covered by keeps an unrelated root-level rule new when only its basename matches a namespaced team rule and by E2E step 7 below, where the team copy on the pushed branch is asserted unchanged.
• The author's own re-push still works after the PR merges, which is the case the basename match was trying to cover: No new or modified resources for an unchanged root copy, (modified) → rules/fe-know/my-rule.md for an edited one.
• A record pointing at a removed or renamed team file is ignored rather than inventing a destination.
• teamai remove drops the entry with the rule.

The field is optional, same convention as coAuthorManaged, so existing state.json files and hand-built State literals in tests keep parsing.

P1 — verification record

Full detail is in the PR body. Summary:

• npm run build ok, npx tsc --noEmit clean.
• npx vitest run: 3761 passed, 3 failed. The 3 are detectShellProfile > Windows (win32) cases that do not stub SHELL; on a zsh terminal they resolve to .zshrc. With SHELL=/bin/bash the file is 25/25. None of those files are in this branch.
• Real CLI, built dist, 7 cells: claude / codex / codebuddy / opencode on provider: git; claude on gitlab; claude on github against a local remote; claude on github against a real private fixture repo (SaulMoro/teamai-e2e-pr698, PRs sync: v0.16.3 from internal #1 merged, docs: make English the default README #2 and docs: fix README blockquote spacing #3 created by the CLI). All assertions pass in every cell, including the reviewer scenario:

member-b writes an unrelated root-level .claude/rules/my-rule.md, push --all
  [rules] my-rule (new)
  ⚠ [rules] my-rule → rules/my-rule.md (shared with everyone: no namespace resolved)
  branch: rules/fe-know/my-rule.md unchanged, rules/my-rule.md is the new file

On GitLab: I have no GitLab instance or token, so that cell runs against a local remote and its PR step stops at the provider with Unrecognized GitLab repo format. I do not think this leaves a gap for this PR. The provider is consulted in one place, createPrWithFallback, which runs after placement is done and the branch is already on the remote, and git diff origin/main --stat -- src/providers src/utils/git.ts src/pull.ts is empty. The gitlab cell does show the GitLab provider being selected and reached after all placement assertions passed, and the real github cell exercises the same createPrWithFallback path end to end. If a maintainer has a GitLab fixture available, the script is a single bash e2e-649.sh claude gitlab <url> and I am happy to attach its output.

One thing I noticed and left alone, since it predates this PR and applies to skills equally: a member who explicitly pushes with --project front-app a new root rule whose name already exists in rules/fe-know/ will overwrite it, because placement resolves to that path and the item is listed as new. The destination is printed, so it is not silent, but a "destination already exists, edit the namespaced copy instead" guard in step 4 would close it. Say the word and I will add it here or as a follow-up.

[github-actions]

Findings

• [P1 blocking] src/push.ts:83 swallows every roles-manifest error and returns no namespace for rules/agents. A missing or malformed manifest—or stale primaryRole—therefore makes --all publish new rules/agents at the shared root, exposing them to everyone. Manifest-resolution failures must stop the push rather than widen scope.
• [P1 blocking] src/resources/rules.ts:95 maps root-authored rules to namespaced files only after pre-push synchronization. syncTeamUpdatesToLocal() still searches rules/<name>.md, so a teammate’s newer rules/<ns>/<name>.md is not synced; the stale root copy is then classified as locally modified and can overwrite the teammate’s update. The placement mapping must participate in the three-way synchronization.
• [P1 blocking] The PR’s test record does not satisfy the repository’s required provider coverage. It explicitly says GitLab and GitHub were not exercised, while AGENTS.md requires real-CLI verification for git, gitlab, and github before PR.
• [P1 blocking] The behavior documentation remains contradictory: docs/usage-guide.md:1445 still says “A new agent lands at the root,” and docs/usage-guide.zh-CN.md:1404 says the same. This conflicts with the new --role/--project placement behavior and violates the bilingual-doc synchronization requirement.
• [P2 non-blocking] src/push.ts:857 exits for --dry-run before rule/agent namespace resolution. Consequently, dry-run neither displays their actual destinations nor reports missing/ambiguous project axes, so it can claim a push is viable when the real command would fail.

[SaulMoro]

All five findings addressed in 5229bec. The branch is rebased onto origin/main (9ce8a0e). PR body's Test Plan rewritten to match the new record.

P1 — src/push.ts:83 swallowed roles-manifest errors

Agreed, and it was the same widening this PR exists to close. The fallback returned an empty list for rules and agents, which resolved to the shared root — every member of the team — on a manifest that happened to be malformed or a primaryRole that had gone stale.

The two cases are not the same thing, so they are no longer caught together. loadRolesManifest now throws a tagged error for the one that is legitimate:

 loadRolesManifest(repoPath)
-  throw new Error(`Roles manifest not found: ${manifestPath}`)
+  throw new RolesManifestNotFoundError(manifestPath)

primaryRole is set
  manifest ABSENT      → pre-manifest layout: role id doubles as the skills namespace,
                         a new rule/agent stays shared and push says so out loud
  manifest UNREADABLE  → stop (exit 2), name the parse error
  role NOT in manifest → stop (exit 2), same message

A team that never had manifest/roles.yaml keeps working exactly as before; a team whose manifest is broken now hears about it instead of publishing to everyone. The message names the three ways out: fix the manifest, teamai roles set <role>, or --role <ns> for this push. The same treatment covers the no-role branch, where a failure to list the team repo's skills namespaces used to resolve to the shared root as well.

P1 — placement did not take part in the pre-push synchronization

Correct, and this was the more dangerous of the two: the scanner followed placedRules and syncTeamUpdatesToLocal did not, so a teammate's newer rules/<ns>/<name>.md was never synced down and the author's stale root copy was classified as a local modification.

syncRulesToLocal now applies the same redirect the scanner does:

local <name>.<ext> at the tool's rules root
  rules/<name>.md exists            → compare against it (unchanged)
  no rules/<name>.md, record exists
    and the recorded file is there  → compare against rules/<ns>/<name>.md
  otherwise                         → skip, as before

Both sides now read the same file, so the three-way check decides correctly: local matches the old rev of the placed file → the teammate's version is copied down, and the next scan finds nothing to push. The recorded path is required to start with rules/ and to contain no .. before it is joined, since it comes from state.json on disk.

Covered by three unit cases in src/__tests__/pre-push-sync.test.ts (redirect, no record, record whose file is gone) and by the real-CLI case below.

P1 — provider coverage in the test record

Fixed properly rather than re-argued. The verification is now a committed test, src/__tests__/push-namespace-e2e.test.ts, which drives the built dist/index.js against real git remotes, a fake gh binary and a fake GitLab API server, and asserts on the branch content that reached the remote:

case	agents	provider
--project resolves each type from its own axis	claude, codex, codebuddy, opencode	git
creates a GitHub PR for the namespaced branch	claude, codex, codebuddy, opencode	github
creates a GitLab MR for the namespaced branch	claude, codex, codebuddy, opencode	gitlab
edit of the root copy after the PR merges	claude	git
another member's root rule with the same basename	claude	git
teammate's newer namespaced rule	claude	git
unreadable roles manifest	claude	git
--dry-run, and --dry-run on an axis that cannot answer	claude	git

18/18. The GitLab cell now exercises createPrWithFallback end to end against a real HTTP server speaking the GitLab MR API, so the gap I flagged last round — no GitLab instance available to me — is closed without one.

Reverting the three fixes of this round in src/push.ts and rebuilding turns exactly their cases red and leaves the other fourteen green:

×  syncs a teammate's newer namespaced rule instead of pushing the stale root copy over it
×  stops the push when the roles manifest exists but cannot be parsed
×  --dry-run reports the destinations and pushes nothing
×  --dry-run fails on a project axis the real push would refuse
✓  14 others

Full record on 5229bec: npm run build success · npx tsc --noEmit clean · npx vitest run 269 files, 3808 passed, 1 skipped, 0 failed · npm run test:e2e 43 files (40 passed, 3 skipped — the live-provider cells that need real tokens), 190 passed, exit 0.

On the three shell-profile failures I reported last round: they are gone, and they were environmental as described. The detectShellProfile > Windows (win32) cases do not stub SHELL, and detectShellProfile reads SHELL before the platform branch, so a zsh terminal resolved them to .zshrc. With the SHELL=/bin/bash that CI has, the suite is fully green.

P1 — contradictory documentation

docs/usage-guide.md:1445 and docs/usage-guide.zh-CN.md:1404 both said a new agent lands at the root. Rewritten in both languages to describe the actual behaviour, with a link to the push section:

- A new agent lands at the root.
+ A new agent is placed the way a new skill is: `--role <ns>` or `--project <id>`
+ (that project's `agents` namespace) names the directory, and with neither flag it
+ resolves from the primary role's `agents` namespaces. It only stays at the shared
+ root — where every member receives it — when no namespace resolves, and push warns
+ when that happens.

Both guides also gained a bullet for each behaviour change from this round: the manifest that stops the push, and --dry-run resolving the same destinations. I re-grepped for lands at the root / 落在根目录 and there is nothing else left.

P2 — --dry-run exited before namespace resolution

Fixed. The two steps that decide a destination are now functions, reuseRecordedDestinations and placeNewResources, and the dry run calls both in the same order a real push does before it exits:

push --dry-run
  step 1   list the scan
  step 2   reuse the destination an open PR recorded
           place the new root-level resources, printing each destination
           stop with exit 2 on an axis that cannot answer
           "Dry run — no changes made"

Everything is treated as selected, which is what a dry run means, so the destinations it prints and the failures it reports are the ones the real command would produce. The namespace prompt is reached only where a real push would reach it — two or more candidates and not --silent — so --dry-run --silent stays non-interactive.

One thing still open, unchanged from last round

A member who pushes with --project front-app a new root rule whose name already exists in rules/fe-know/ overwrites it, because placement resolves to that path and the item is listed as new. It predates this PR and applies to skills equally, and the destination is printed so it is not silent. A "destination already exists, edit the namespaced copy instead" guard in step 4 would close it — here or as a follow-up, your call.

[github-actions]

• [P1 blocking] src/remove.ts:199 deletes the placedRules entry without using it to remove the actual team file. After pushing root rule my-rule to rules/fe-know/my-rule.md, teamai remove rules my-rule still asks RulesHandler.removeItem to delete rules/my-rule.md; it removes the local copy/tombstones it but leaves the namespaced rule published. Resolve the recorded destination before removal.
• [P1 blocking] src/push.ts:352 namespaces new agents but provides no persistent mapping analogous to placedRules. On a later push --role <ns> or push --project <id> where that namespace is not already active locally, AgentsHandler.scanLocalForPush filters the namespaced canonical file out as inactive and skips the local edit, so the explicit destination override cannot update the agent it originally created.
• [P1 blocking] src/resources/rules.ts:92 only consults placedRules when no shared-root rule with the same basename exists. If another contributor later adds rules/my-rule.md, the author's root-local copy is silently remapped from its recorded rules/fe-know/my-rule.md to the unrelated shared rule and may overwrite it. A valid placement record should take precedence, or the collision should be rejected explicitly.
• The PR description includes a detailed test plan and real-CLI E2E records, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/remove.ts:104 checks bare names before calling publishedNameFor. If a root-authored, namespaced rule has local edits, scanLocalForPush contributes the bare name, so teamai remove rules my-rule removes only local/root paths while leaving rules/<ns>/my-rule.md published. Resolve placement records before accepting the bare match.
• [P1 blocking] src/push.ts:1139 records every namespaced agent, including existing agents merely edited while their namespace is active. After the user changes roles/projects, AgentsHandler treats that stale record as permission to keep modifying the now-inactive agent. Record only agents that were newly placed from the shared root.
• [P1 blocking] src/push.ts:98 returns role-manifest namespaces without validating them as single safe path segments. Unlike project namespaces and explicit --role, values such as foo/bar are accepted; agents are then written below an unsupported depth and are not discovered by pull, while rules are classified under namespace foo rather than foo/bar.
• [P1 blocking] src/push.ts:1107 saves state immediately when a later push group fails, but placedRules/placedAgents are populated only after every group completes at src/push.ts:1129. A successfully pushed earlier group therefore loses its placement mapping, causing incorrect reclassification after its PR merges. Persist placement records for each successful group before a possible early return.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/resources/agents.ts:155 — Explicit --role/--project cannot create an agent when the same stem exists in an inactive namespace. The scan runs before destination resolution and returns “no active source,” so agents/backend/reviewer.yaml prevents publishing a new agents/frontend/reviewer.yaml, even though multiple namespaces may legitimately contain the same stem. The requested destination must participate in candidate selection or reclassification.
• [P1 blocking] src/resources/rules.ts:315 — Removing an explicitly namespaced rule also deletes an unrelated root-local rule with the same basename. For example, teamai remove rules fe/foo unconditionally adds foo to localNames and removes .claude/rules/foo.md. Only remove the bare local copy when the name was translated through this machine’s placement record.
• [P2 non-blocking] src/index.ts:94 — CLI help still says --role and --project target “skills,” although this PR changes them to place rules and agents too. Update both option descriptions so teamai push --help reflects the new behavior.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so it satisfies the review requirement for testing evidence.

[github-actions]

• [P1 blocking] src/push.ts:396 blindly rewrites every root-level “new” rule to the resolved namespace without checking whether that destination already exists. For example, with an existing rules/fe/foo.md and an unrelated local root foo.md (which the scanner intentionally classifies as new), a default role resolving to fe or --role fe overwrites the existing team rule. Refuse the collision or resolve it during scanning, as the agent path does.
• [P1 blocking] src/roles.ts:111 uses readFileSafe, which returns null for every read error, not only ENOENT. Consequently, an existing but permission-denied or otherwise unreadable manifest/roles.yaml is treated as absent; namespaceCandidates then permits new rules/agents at the shared root, leaking them team-wide. Distinguish a genuinely missing file from other read failures.
• [P2 non-blocking] src/resources/rules.ts:327 tombstones only the published name such as fe/my-rule, although the placement record is removed and the author’s source is named my-rule. Any surviving root copy—for example in an excluded tool directory skipped by removal—will not match that tombstone and can be republished on the next push. Record the bare tombstone too when removing through a placement record.

The PR description contains a detailed test plan and real-CLI/e2e verification record, so it satisfies the review requirement.

[github-actions]

Findings

• [P1 blocking] Resolve placed agents before removal — src/remove.ts:110 calls publishedNameFor, but AgentsHandler does not implement it. After publishing vr as agents/fe/vr.yaml, teamai remove agents vr falls back to the bare name; removeItem then deletes every vr across all namespaces, and src/remove.ts:243 fails to clear the namespaced placedAgents record. Add agent-specific resolution and remove only the recorded published path.
• [P1 blocking] Detect alternate agent extensions before placement — src/push.ts:401 checks only the exact proposed path. In self mode, a new root reviewer.md targeted at a namespace already containing reviewer.yaml passes this check and creates both files with the same agent stem, producing an ambiguous pull. Agent collision checks must test both supported canonical extensions.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so it satisfies the testing-documentation requirement.

[github-actions]

Findings

• [P1 blocking] src/remove.ts:111 — publishedNameFor() returns a namespaced agent name such as fe-agents/vr, but AgentsHandler.scanTeamForPull() exposes only the bare name vr. Therefore allNames.has(published) is always false for agents. The fallback accepts vr, and removeItem('vr') deletes every vr across all namespaces. Build allNames with namespace-qualified agent identities or otherwise accept the validated published path directly.
• [P1 blocking] src/resources/agents.ts:509 — Removing one namespaced agent writes a bare-stem tombstone as well. Agents are flattened locally and both push scanning and post-pull cleanup interpret that bare tombstone globally, so removing fe/vr also suppresses or deletes be/vr when that namespace later becomes active. Placement ownership needs separate local cleanup metadata rather than a shared bare tombstone.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so no testing-description finding is needed.

[github-actions]

• [P1 blocking] Preserve project-axis resolution failures during agent scanning — src/push.ts:867 discards an unsuccessful resolveProjectNamespace() result. If the project lacks an agents namespace and the local agent’s stem already exists only in an inactive namespace, scanLocalForPush() treats this as an ordinary no-destination scan, skips it as “no active source,” and placeNewResources() never receives the item. The command then exits successfully with “No new or modified resources” instead of the required exit-2 project-axis error. Carry the resolution error forward or distinguish an explicitly requested but unresolved destination.

• [P2 non-blocking] Validate the recorded filename exactly — src/push-namespaces.ts:161 accepts any filename beginning with ${name}., so a record such as foo: rules/ns/foo.backup.md is trusted. If that file exists, scanning, pre-push sync, and removal can redirect foo to an unrelated resource and overwrite or remove it. Require exactly ${name}.md for rules and ${name}.yaml/${name}.md for agents.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/resources/agents.ts:155 — Placement records are applied only to reverse-parsed tool files, not directItems. In self mode, a root .teamai/agents/vr.yaml placed at agents/fe/vr.yaml is therefore classified as new on the next push; the collision check then rejects it instead of updating the published agent.
• [P1 blocking] src/resources/agents.ts:167 — Accepting an inactive namespace through placedAgents without three-way pre-push synchronization can overwrite teammates’ changes. If the team agent changed after the last pull, the stale local rendering differs from the current canonical file and is interpreted as a local edit, causing the next push to revert the newer team version.
• [P1 blocking] src/push.ts:891 — A project with no agents namespace rejects every scanned agent, including modifications that already have an existing namespace and require no placement. This contradicts the stated “only new shared-root resources are placed” behavior and breaks valid modified-agent pushes for projects where the optional agents axis is empty.
• [P1 blocking] src/resources/agents.ts:511 — Namespaced removal writes only a tombstone such as fe/vr, while agent scanning checks tombstones using the bare stem vr. A surviving flattened copy—particularly in an excluded tool, or the self-mode .teamai/agents source—will be detected as new and can republish the removed agent.

Testing

• The PR description includes a detailed test plan and real-CLI/e2e records, so no testing-description violation.

[github-actions]

• [P1 blocking] Prevent stale inactive-agent overwrites. src/resources/agents.ts:245 only blocks when the file existed at lastPullRev. Immediately after the agent’s first PR merges, that revision commonly predates the file, so atLastPull === null allows a stale local copy to overwrite a teammate’s update. Also, the suggested teamai pull does not deploy inactive namespaces but does advance lastPullRev, allowing the next push through. Persist a per-resource baseline or perform a real three-way comparison without advancing past an unsynced inactive agent.

• [P1 blocking] Include namespace in pending-PR identity. src/push.ts:338 reuses a pending destination obtained through a type + name lookup. If agents/fe/vr has an open PR and the user runs push --role be for a distinct vr, the old PR is matched and its fe namespace overrides the requested be destination. This can force-push unrelated content into the wrong PR. Pending matching must distinguish namespace/path.

• [P1 blocking] Re-run and document validation for the reviewed head. The Test Plan explicitly says it ran on d0ee08e, but the reviewed head is 0403785. The later commit changes production behavior in src/push.ts and src/resources/agents.ts; no build, unit, or real-CLI result is documented for those changes. Per repository review rules, the final head needs an actual test plan and end-to-end verification record.

[github-actions]

Findings

• [P1 blocking] src/pull.ts:296 — A recorded agent from an inactive namespace is added to the pull set, but the later cleanupInactiveNamespaces() call still receives only active namespaces. It therefore sees the freshly deployed agent as inactive, finds its contents unchanged, and immediately deletes it. Exclude recovered placement-record agents from cleanup or include them in the effective namespace set.
• [P1 blocking] src/push.ts:431 — Placing a root-authored rule into a namespace that the author does not activate makes the next pull delete the author’s root copy. resolveDesiredRules() excludes the namespaced team rule, while pullAllRules() compares the remaining root file name (foo) against desired names (namespace/foo) and removes it as stale, potentially discarding local edits. Pull/cleanup needs to honor placedRules, as the push scan and pre-push sync now do.
• [P1 blocking] src/push.ts:1179 — Dry-run applies every pending PR’s recorded destination before the explicit-destination conflict filtering performed by the real push at line 1225. Consequently, push --dry-run --role new-ns can report the old pending namespace while the real command creates a separate PR targeting new-ns, defeating dry-run’s stated parity.
• [P1 blocking] src/push.ts:1220 — Conflict detection trusts PendingPushItem.namespace, but normal scans of modified namespaced agents create items without a namespace field (src/resources/agents.ts:339). Such pending entries bypass this check; a later push specifying another namespace can reuse and force-push the original PR branch, replacing the change under review. Derive the recorded namespace from relativePath or ensure agent scan items always carry it.

The PR description includes a detailed test plan and real-CLI/E2E records, so no testing-description finding is needed.

[github-actions]

Findings

• [P1 blocking] src/push.ts:898 validates the project’s agents axis against fullScan before interactive selection. A project without an agents namespace cannot push only a rule/skill whenever any unrelated new agent is present locally—the user never gets the chance to deselect it. Defer this validation until selectedItems, matching the skills-axis behavior.
• [P1 blocking] src/resources/rules.ts:415 preserves the author’s root-level rule while pullItem also installs the namespaced copy. After publishing rules/fe/foo.md, the author retains both <tool>/rules/foo.* and <tool>/rules/fe/foo.*; tools that recursively load rules apply duplicate, potentially conflicting instructions. The placement record should cause pull to update the authoring copy rather than deploy a second copy.
• [P1 blocking] src/push.ts:1157 treats a pending shared-root resource as non-conflicting with an explicit --role/--project because missing recordedNamespace returns false. The pending branch is then reused and force-rebuilt with the newly namespaced path, silently changing the scope of an existing review instead of leaving it untouched and creating a separate PR. A shared-root recorded path must conflict with any explicitly requested namespace.
• [P1 blocking] src/push.ts:645 loads projects.yaml before pushCore updates the team clone. If the remote changed a project’s knowledge or agents namespace, new rules/agents are placed using the stale mapping after the repository has been pulled. Reload and validate the manifest after the pull, inside the locked/worktree context.

The PR description includes a detailed test plan and real-CLI E2E records, so there is no testing-documentation finding.

[github-actions]

Findings

• [P1 blocking] src/push.ts:815 — A failed pullRepo() is only warned about, after which --project resolves destinations from the stale local projects.yaml. If the remote changed a project’s namespace, the command can publish rules or agents to the old namespace—and therefore the wrong audience. Abort project-targeted placement when the refresh fails rather than continuing with stale routing data.
• [P1 blocking] src/resources/agents.ts:152 — The placement redirect preserves the recorded extension in relativePath, but pushItem() writes using the current source extension. Renaming a self-mode canonical agent from .md to .yaml or vice versa writes one path while pushGroup() stages the other, so the change is silently omitted and can leave duplicate canonical files. Update relativePath to the actual output extension and remove the superseded file.
• [P1 blocking] src/push.ts:311 — Placement records are persisted once a branch is pushed but are never cleared when that PR is closed/unmerged or the resource is deleted upstream. If another contributor later creates the same path, the stale record becomes valid again; pull/scan then treats that unrelated rule or agent as the author’s resource and may overwrite either copy. Placement records need lifecycle cleanup when their pending branch disappears or their target vanishes.

The PR description includes a detailed test plan and real-CLI/e2e verification record, so the testing-documentation requirement is satisfied.

[github-actions]

• [P1 blocking] src/resources/rules.ts:269 — localNameFor() redirects a recorded namespaced rule onto the bare root path without checking whether a distinct shared-root rule with the same basename is also being delivered. If rules/fe/foo.md is recorded and another contributor later adds rules/foo.md, pull writes both resources to the same local foo.md in iteration-dependent order. With fe inactive, the shared rule always overwrites the author’s local source; the next push then follows the record and can overwrite rules/fe/foo.md with the unrelated shared rule.

• [P1 blocking] src/utils/pending-push.ts:189 — Removing multiple stale records from the same map is incorrect. Each iteration destructures from the original records object and assigns that result to state[field], so a later deletion restores entries removed earlier. For example, with two stale placedRules, only the last iterated record is actually removed, leaving stale mappings that can later attach to unrelated same-named resources.

• [P1 blocking] src/utils/pending-push.ts:172 — A placement record is considered “awaiting review” solely because its remote branch still exists. Closing a PR without merging commonly leaves its branch intact, so the record is retained indefinitely despite the PR description explicitly claiming this case is handled. If that path is later created on the default branch by someone else, the stale record can redirect the author’s local resource onto it.

• The PR description includes a detailed test plan and real-CLI/e2e record, so it satisfies the testing-documentation requirement.

[github-actions]

• [P1 blocking] Retire landed pending placements after creating the record — src/utils/pending-push.ts:229. The reconciliation creates placedRules/placedAgents but leaves the placed pending item intact. If the resource is later deleted, step 2 drops its record; if another member subsequently recreates the same path, pathExists succeeds and blobInHistory remains true because the original blob existed historically. The stale pending item therefore recreates the record and causes the unrelated replacement to be treated as this author’s resource. Remove or mark the pending placement as consumed once it becomes a record.

• Testing assessment: The PR description includes a detailed test plan and real-CLI/e2e records across all required agents and providers, so it satisfies the repository’s testing-documentation requirement. Tests were not executed, per the review instructions.

[github-actions]

Findings

• [P1 blocking] src/remove.ts:260 drops placedAgents immediately after pushing the deletion branch, before the deletion reaches the default branch. A retry while the removal PR is still open—or after PR creation fails—can no longer resolve bare vr to fe-agents/vr; AgentsHandler.removeItem('vr') then deletes every same-stem agent across all namespaces. Keep the placement record until reconciliation observes the file removed from the default branch.
• [P1 blocking] src/resources/agents.ts:603 records only the namespaced tombstone (for example fe-agents/vr), while intentionally leaving copies in excluded tools. scanLocalForPush checks tombstones using the flattened stem (vr) and still scans excluded tool directories, so the surviving copy is detected as new and republishes the removed agent. Either exclude disabled tools from push scanning or persist suppression that matches the flattened source without suppressing unrelated namespaces.
• [P1 blocking] src/utils/pending-push.ts:235 considers a placement merged when its blob appears anywhere in the path’s history. If that same content existed at the path before this PR, the placement PR is closed unmerged, and another member later recreates the path, the historical blob check falsely assigns their resource to this author. Subsequent pull/push/remove operations may redirect or overwrite the unrelated resource. The proof must be bounded to changes after the pending push’s base revision, not the path’s entire history.

The PR description contains a detailed test plan and real-CLI/e2e verification record, so no testing-description blocker is needed.

[SaulMoro]

@jeff-r2026 I've rebased onto a52374a (current main), and the head is now 859ac58.

Rebase. One conflict: remove.test.ts's vi.mock('../config.js'), which main changed to spread importOriginal. I kept both main's change and the PR's. git range-diff shows only test-mock context changes to the earlier commits. Main's isInteractive is now also used by push's namespace prompt.

Round 21. The bot's four P1s from 04:06 are fixed, plus six more from our own adversarial pass over the branch:

Finding	Fix
push.ts:314: a pending namespaced placement was rebuilt at the shared-root path once a same-named root file appeared	That local copy is left out of the push, with a warning. The open PR is not touched.
remove.ts:83: a failed reconcile or save was swallowed before resolving names	remove stops with exit 1 and removes nothing
push.ts:951: an agent skipped for want of an agents namespace aborted the whole --project push	It is skipped like other agents. The error only fires when nothing else would be pushed.
pending-push.ts:259: a placement without a blob was recorded because its path existed	A placement is marked only when it has a blob. One without is spent, not recorded.
.teamai/rules was never scanned in single-repo mode once enabledAgents was set	The synthetic scan source is exempt from the excluded-tool gate
Removing fe/vr deleted a same-named personal agent on members who never had fe	The stem is retired only where that agent could have been delivered
Two active agents with the same name plus --role could push one over the other	Two active sources stay ambiguous, flag or not
--role <ns> overwrote an agent that already existed in <ns>	That is now a collision, as it already is for rules
In single-repo mode, a stale root copy could overwrite a teammate's edit	A copy equal to an older team version is held, not pushed
P2s	Hold message and when it applies; pending placement routed back to its PR; warning when a flag doesn't move an edit; non-interactive --role hint; placement reported once if it arrives with different content; JSDoc

Verification on 859ac58:

• npm run build and tsc are clean.
• Unit: 277 files, 4051 passed.
• Full npm run test:e2e: 215 passed, 26 skipped (live-provider files).
• push-namespace-e2e: 37/37.
• Every new test fails against the previous head's code.

Still open:

• Agents in active namespaces have no pre-push sync. That was already true on main.
• Single-repo mode has no e2e fixture. Its new paths are covered by real-git unit tests.

The PR description has the full record.

[github-actions]

Findings

• [P1 blocking] src/push.ts:863 — Placement reconciliation/save failures are still swallowed before scanning. If a shared-root rule now shadows a recorded namespaced rule and saveStateForScope fails, subsequent sync and scan reload the stale record and can push the shared-root content over the unrelated namespaced rule. Abort the push when reconciled placement state cannot be persisted.
• [P1 blocking] src/push.ts:375 — A failed refresh only blocks unflagged placement when the stale clone already contains manifest/roles.yaml. If the manifest was added remotely since the last successful pull, new rules/agents still fall back to the shared root; without a primary role, skills can also use namespaces detected from the stale directory tree. Any unflagged placement whose destination depends on repository state must stop when refresh fails.
• [P1 blocking] src/resources/agents.ts:524 — Namespaced agents are still exposed to remove only by their bare stem. On a machine without the placement record, remove agents fe/vr is rejected because allNames contains only vr, while remove agents vr deletes every same-stem agent across namespaces. Preserve the namespace in removal discovery so one namespaced agent can be removed safely.
• [P1 blocking] src/utils/pending-push.ts:333 — The deleted-and-recreated record path drops placedAgents without adding retiredPlacedAgents, unlike the currently-missing-file path above. For an author who placed an agent in an inactive namespace, the namespaced tombstone then cannot identify or clean the flattened local copy, allowing the removed agent to survive and later be republished into another namespace.

The four findings from the previous review are resolved in the current diff. The PR description includes a sufficient test plan and real-CLI/E2E record for exact head 859ac58, so no testing-description finding is needed. I reviewed only the requested diff and did not execute PR code or tests.

[SaulMoro]

The four P1s from the 06:11 review are fixed in 23717ec, which sits on top of a52374a with no rebase this time.

Finding	Fix
push.ts:863: a failed reconcile or save was swallowed before the scan	push stops with exit 1 and pushes nothing, as remove already does
push.ts:375: a stale clone only blocked placement when manifest/roles.yaml already existed	Every placement without --role stops on a stale clone. A missing manifest and the detected skills namespaces are clone state too.
agents.ts:524: remove knew namespaced agents only by their bare stem	remove agents <ns>/<name> removes one agent. A bare name held by one namespace resolves to that agent. A bare name held by several is refused and the qualified names are listed.
pending-push.ts:333: the deleted-and-recreated drop did not retire the record	It now writes retiredPlacedAgents, like the missing-file path

This changes behaviour for teams without a roles manifest: if a push can't refresh the clone, a new resource now needs --role <ns>. The PR description records this.

Verification on 23717ec:

• npm run build and tsc are clean.
• Unit: 277 files, 4056 passed.
• Full npm run test:e2e: 215 passed, 26 skipped.
• push-namespace-e2e: 37/37.
• All six new tests fail against 859ac58's code.