Skip to content

feat(push): support all namespaced resource types - #1234

Merged
SaulMoro merged 30 commits into
Tencent:mainfrom
legend91019:feat/push-all-namespaced-resources-1101
Oct 23, 2026
Merged

SaulMoro merged 30 commits into
Tencent:mainfrom
legend91019:feat/push-all-namespaced-resources-1101

Conversation

@legend91019

@legend91019 legend91019 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • make every pull-distributed resource type pushable with project and role namespaces
  • add generic repository-backed push handlers and workflow namespace resolution
  • extend branch review to scan all supported resource types
  • harden routing so nested docs/wiki paths are only treated as namespaced when the namespace is declared
  • apply deterministic conflict resolution to legacy workflows as well as manifest-based workflows
  • keep a teammate's change to a team file out of a push made from a checkout behind the default branch

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Test Plan

  • npm run build / npm run typecheck / npm run lint
  • npx vitest run src/__tests__/push-role.test.ts — 94/94
  • npx vitest run src/__tests__/push-all-resources-namespaces.test.ts src/__tests__/push-namespaces.test.ts src/__tests__/workflow-cache-namespaces.test.ts src/__tests__/review-branch.test.ts — 48/48
  • npx vitest run src/__tests__/commands-reference.test.ts — 1/1 (snapshot regenerated)
  • npx vitest run src/__tests__/workflow-records.test.ts — 43/43

Built-CLI record at 6c4bb84 (per AGENTS.md "Testing before a PR"). Self mode: a .teamai/ checkout of a local bare team repo, provider: git, manifest/roles.yaml declares ops for docs, env and prompts.

  1. teamai push --all --role ops with a new root doc, a root env and a nested prompt:
    • docs/guide.md → docs/ops/guide.md
    • env/env.yaml → env/ops/env.yaml
    • prompts/topic/note.md → prompts/ops/topic/note.md
    • exit 1 is PR creation on a generic git host; the branch is pushed with those three additions
  2. Merge the branch on the bare repo, fast-forward, teamai pull → exit 0. state.placements holds { path, blob } for all three.
  3. Edit the root copy of the placed doc only, teamai push --all --role ops:
    Found 1 resource(s) — [docs] guide.md (modified) to: docs/ops/guide.md, and the branch modifies only that file (env and the prompt stay unlisted).
  4. teamai pull → exit 0, teamai doctor → exit 0 (All checks passed!). The record's blob is refreshed to what that push delivered.
  5. Teammate fixes docs/ops/guide.md and docs/README.md on main; the author pushes an unrelated new doc:
    • current with main: only the new doc is listed, and the branch keeps both of the teammate's fixes.
    • behind main (the teammate's commit not pulled): Left out .teamai/docs/ops/guide.md, .teamai/docs/README.md: changed on origin/main since this checkout branched from it, and not edited here. git pull origin main brings the change in. Found 1 resource(s) — [docs] handbook.md (new), placed to docs/ops/handbook.md; the branch adds only that file, and both files keep the teammate's line.
    • behind main and editing one of them too: exit 2, the file is named, nothing is pushed.
  6. teamai push --all --role rogue (a namespace no role or project declares): exit 2, nothing pushed, no new branch.

npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/self-mode-push-behind.test.ts — 8/8 on the built CLI at this head, covering the three outcomes above plus a shallow clone, unrelated history, a namespaced rule destination, a tool copy and a file a format change would delete.

The E2E suites push-active-namespaces, contribute-namespace and all three E2E (fork-safe, no credentials) shards are green in CI on this head. Representative assertions from push-active-namespaces:

  • push --all --skill <path> --role svc-a → lands at skills/svc-a/foo/SKILL.md on the push branch (line 649, 683)
  • --project platform maps to the namespace platform declares in manifest/projects.yaml
  • Ambiguous delivered origins are skipped with copy it under a new name and push that (line 448–455)

Review Fixes

Commit 93809cd — addressing @SaulMoro's review

P1

  • Binary doc corruption: pushRepoResourceItem uses fs.copyFile (byte copy) instead of UTF-8 text I/O
  • Prompt namespaces independent of workflow layout: resolves namespaced prompt directories independently of workflow namespaced candidates; nested prompt paths resolve through namespaces
  • Env placement: envPushItem items carry status, so placement routes env/env.yaml → env/<ns>/env.yaml
  • Role learnings: resolveRoleResourceNamespaces includes learnings
  • Wiki source: handler points at teamwiki/evidence/code/
  • E2E warning pollution: pre-refresh scan filters LOCAL_TOOL_TYPES

P2 — rootSegmentsFor() for wiki multi-segment root; review-branch nested identity + handler-provided names; manifest-free undeclared-namespace skip; --role undeclared warning; status counts; pushRepoResourceItem signature; snapshot tightening; docs + help text

Commit d77e8f0 — re-review findings

  • Snapshot key mismatch (P1): track prePlacementPath so the snapshot delete finds the entry keyed by the root path after placement rewrites item.relativePath
  • Manifest-error namespace fallback (P1): a manifest that cannot be read now resolves to an empty namespace set (conservative) instead of treating every on-disk namespace as active
  • commands-reference snapshot regenerated for the updated --role/--project help text

Later review rounds (through 7e6dd4d)

  • Reuse-path snapshot key (P1): reuseRecordedDestinations saves prePlacementPath before rewriting relativePath
  • Env namespace copy (P1): EnvHandler.pushItem copies whenever source ≠ destination, and retires a clone-local original once it has been placed
  • Manifest-free prompt namespaces (P1): without a manifest, prompt namespaces are discovered from disk
  • Nested resource names (P1): parseRelativePath keeps each path segment valid on its own
  • Rule names (P1): RulesHandler.scanTeamForPull extracts name and namespace separately
  • Review identity (P1/P2): hooks are reviewed by entry id, wiki is scanned at teamwiki/evidence/code, docs/wiki keep their bundle-relative path, and nested prompts are shared paths
  • Placement records (P1): a placed resource is remembered by the path the author still edits, compared against the blob the push delivered, and the record expires when its path is deleted since the last check
  • Explicit prompt namespaces (P2): a prompt_file whose first segment is already an active namespace resolves as written
  • Scorer samples (P2): .md names, since a scorer is scorers/<name>.md

Maintainer follow-up c15069a..1debe94 (on top of this branch)

Placement records store the delivered blob; --role must name a declared namespace (exit 2 otherwise); scorers resolve inactive namespaces like prompts; contribute uses the member's role learnings and accepts an empty --namespace for the shared root; a pre-refresh scan error reaches the pull-failure path, and a conflict message says to merge the edit into the placed file.

Maintainer follow-up 2c0e940..6c4bb84 (on top of this branch)

A single-repo push compares the member's .teamai/ with a freshly fetched origin/<default>, so on a checkout behind that branch every team file a teammate changed reads as the member's edit and was pushed over the teammate's commit. Main already behaved this way for env/env.yaml; this PR widened it to docs, prompts and the other repo-backed types, so it is fixed here rather than left in.

6c4bb84 compares each push item sourced from .teamai/ — at its source, its destination, and any file a format change would delete — against what origin/<default> changed since the merge-base:

The member's copy Result
matches the merge-base (only main changed it) left out, with Left out <file>: changed on origin/main since this checkout branched from it, and not edited here.
differs from both (both changed it) exit 2, names the file, nothing pushed
a format change would delete a file main changed exit 2, names the file to carry the change into
no merge-base (shallow clone, unrelated history) exit 2, suggests git fetch --unshallow origin or a merge

The stop covers only files in the final selection, so push --skill other is not blocked by an unrelated env.yaml. 2c0e940 adds self-mode-push-behind, which fails before 6c4bb84; push-sync-followups-823 changed with the fix: editing a stale .teamai/rules copy a teammate updated on main now stops with exit 2 instead of pushing over that update.

Out of scope, both pre-existing on main: skills, whose source is a directory (they keep #823's stale-copy check, but an edit on both sides still overwrites), and a tool-directory copy (.claude/rules) edited on a stale checkout, which needs the delivery records.

Related Issues

Closes #1101

Notes

This keeps legacy teams compatible while bringing their workflow conflict arbitration onto the same deterministic path used by manifest-based teams.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/push.ts:662 — Placement records the original path as supersedes even when it was untracked. For a new workflows/deploy.yaml pushed into ops, the handler deletes the original, then staging includes that nonexistent, untracked path. Git rejects the pathspec and the push fails. Only stage superseded paths that Git tracks.

  • [P1 blocking] src/resources/repo-resource.ts:110 — Relocation copies every resource through text I/O. A new binary document such as docs/diagram.png, placed into a namespace, is decoded and rewritten as text before the original is deleted, corrupting the image. Copy raw bytes instead.

  • [P2 non-blocking] src/review-branch.ts:108 — Nested docs/wiki resources are identified only by basename. With co-active namespaces, docs/a/api/guide.md and docs/b/ui/guide.md are incorrectly reported as conflicting definitions of guide, despite their distinct bundle-relative paths. Preserve the path below the namespace when identifying these resources.

Testing documentation meets the stated review criterion: the body records a built, real-CLI run. The deferred full E2E suite is not blocking. No earlier findings were supplied.

@SaulMoro SaulMoro left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on. The workflow side is in good shape: namespace resolution, conflict withdrawal from the cache, and cross-file checks on the resolved set all hold up. But the push path regresses existing behavior and several issue requirements are not met yet, so requesting changes.

Reviewed head fd2c98b2 against origin/main 8787405e. Every P1 below was reproduced with the built CLI or the repo's own tests.

Regressions

[P1 blocking] Placing a new resource into a namespace aborts at git add. src/push.ts:660-662 sets item.supersedes to the original root path on every placement, and pushGroup stages it (src/push.ts:710-711). That path never existed in the clone, so git fails:

  • new skill, unflagged namespace selection: fatal: pathspec 'skills/foo' did not match any files
  • new docs/guide.md with --project billing: fatal: pathspec 'docs/guide.md' did not match any files
  • new learning with --role dev, new workflow with --project: same error.

Only set and stage supersedes for a tracked file the handler actually removed.

[P1 blocking] The pre-refresh snapshot now injects local tool files into the clone. src/push.ts:1110-1148 snapshots every placeable type's sourcePath under its relativePath. For rules and agents the source is the tool's file outside the clone, so restore writes it into the clone before the real scan. Result: a new rule reports "No new or modified resources to push"; a Claude agent .md render lands at agents/<name>.yaml and is then skipped as invalid YAML; an unrelated skill push sweeps the injected rule to the shared root through the rules/ sweeper. The same scenarios reach remote push correctly on main. Snapshot only items whose resolved sourcePath equals the clone destination.

[P1 blocking] Existing suites go red. push-active-namespaces.test.ts + push-sync-followups-823.test.ts: 28 of 109 fail on this head, 109/109 pass on main. Unit: 3 failures in push-role.test.ts, including a broken roles manifest that now throws Invalid roles manifest YAML instead of stopping the push. CI "Lint & Test" is failing.

[P1 blocking] Binary docs are corrupted. pushRepoResourceItem copies through readFileSafe/writeFile (UTF-8 text). A PNG under .teamai/docs/<ns>/ in self mode is pushed with replacement characters (89504e47... becomes efbfbd504e47...). Use a byte copy.

Issue #1101 and its comments

[P1 blocking] Env still bypasses placement. EnvHandler items carry no status, and placement requires status === 'new' (src/push.ts:597). A new env/env.yaml with --project (project declares env: [ops]) is committed at env/env.yaml. The issue lists env/<ns>/env.yaml explicitly.

[P1 blocking] Learnings with --role (comment from 2026-10-19). src/roles.ts:212-215 still hard-codes role learnings to []. Without --role, a new learning falls back to the shared root ("no namespace resolved"). With --role ops it is written to learnings/ops/, but pull for that role never activates ops, so teammates never receive it. That breaks "missing mappings fail" and "pull and push use the same rules".

[P1 blocking] Wiki scans the wrong source. The handler reads wiki/, but the wiki source that resources.wiki scopes is teamwiki/evidence/code/<slug>/ (src/manifest-schema.ts:112-117, read by recall.ts). Editing teamwiki/evidence/code/ops/overview.md and pushing fails with "the team repo has uncommitted changes".

[P2 non-blocking] Scorers. Push writes scorers/<ns>/, but nothing in src/ reads scorers/ on pull, on this branch or on main. Either drop scorers from the push set or add the pull side. Don't ship a one-way path.

[P2 non-blocking] Prompt namespaces in manifest-free teams. With no manifest, workflows treat every namespace as active, but activePrompts stays null, so prompts/<ns>/ is never considered (src/workflow/cache.ts:227-234, 522-547). The !leaf.includes('/') guard also skips namespaced overrides for nested prompt paths.

Other

  • [P2 non-blocking] --role <ns> is accepted for the new types without checking it is declared for that axis: --role rogue previews workflows/rogue/deploy.yaml, which pull ignores.
  • [P2 non-blocking] review --branch flags valid pull sources as undeclared namespaces, for example claudemd/legacy/ in a manifest-free team and nested prompts/topic/deep/note.md (src/review-branch.ts:99-109).
  • [P2 non-blocking] teamai status now lists the new types but always shows 0 for them (src/status.ts:95-130).
  • [P2 non-blocking] The pre-refresh scans swallow every error as [] (src/push.ts:1112-1115); before this PR, env scan errors were reported. scanRepoResourceForPush also treats every file as new if git ls-files fails.
  • [P2 non-blocking] pushRepoResourceItem(item, localConfigOrTeamConfig, maybeLocalConfig?) with as LocalConfig: every caller passes localConfig. Take LocalConfig directly.
  • [P2 non-blocking] Docs are stale: docs/guide/distribution.md (+ zh-CN) still says docs/hooks/MCP/models are committed manually; skill-data/core/references/contribute-member.md:98 and --role/--project help still name only skills/rules/agents. After changing help, regenerate commands.md.
  • Test record: the real-CLI run is teamai review --branch main --json, which does not exercise push --project/--role. Please add one real push run per changed path, e.g. a new docs file with --project and a new learning with --role, and show where they land on the remote branch.

Suggested fix for the push regressions

The first two P1s and the e2e failures come from one assumption: that an item's sourcePath is the file at its relativePath in the clone. That holds for the clone-backed types, not for skills/rules/agents, whose source is the tool's own file. And a newly placed item has no tracked original to retire. This change, applied on top of fd2c98b2, is enough for the push e2e suites:

+const LOCAL_TOOL_TYPES: ReadonlySet<PlaceableType> = new Set(['skills', 'rules', 'agents']);
@@ placeNewResources
-          const originalRelPath = item.relativePath;
           item.namespace = destination.namespace;
           item.relativePath = placedAt;
-          if (originalRelPath !== placedAt) {
-            item.supersedes = originalRelPath;
-          }
@@ pushCore, pre-refresh snapshot
       const repoScan = Promise.all(
-        PLACEABLE_TYPES.map(async (t) => {
+        PLACEABLE_TYPES.filter((t) => !LOCAL_TOOL_TYPES.has(t)).map(async (t) => {
@@
       for (const item of scannedGroups.flat()) {
         if (indexedPaths.has(item.relativePath)) continue;
+        if (path.resolve(item.sourcePath) !== path.resolve(repoPath, item.relativePath)) continue;
         const snapshot = await readFileSnapshot(item.sourcePath);
  • Placement only handles status === 'new' items, whose root path is never tracked, so there is nothing to stage as removed. With supersedes unset, the new ResourceItem.supersedes field in types.ts has no writer and can go.
  • skills/rules/agents were never in the pre-refresh scan before this PR. Scanning them there without the user's flags also prints spurious warnings, e.g. "Skipped b-skill".

With this diff, push-active-namespaces, push-sync-followups-823 and copilot-pre-push-sync pass 110/110 (28 failing before). tsc --noEmit is clean.

It does not fix the 3 push-role.test.ts failures, which have two more causes:

  • push runs validateWorkflows → syncWorkflowDefinitions before the pull, and syncWorkflowDefinitions now calls resolveResourceNamespaces (src/workflow/cache.ts:227). That reads the manifests from the stale clone, and a broken manifest throws there. This breaks "reads the projects manifest only after the team clone has been pulled" and "reports a projects manifest that cannot be loaded for --project instead of throwing".
  • The new resolveResourceNamespaces(localConfig) call in pushCore for docs/wiki is uncaught, so a broken roles manifest throws instead of stopping with exit 2 ("stops the push when the roles manifest exists but cannot be read"). Report it the way the other manifest readers in pushCore do.

The remaining findings above (env, role learnings, wiki source, binary copy) are separate and still need their own changes.

Three CI failures rooted in the same family of pre-pull / path-confusion
bugs introduced with the namespace placement work:

1. syncWorkflowDefinitions unconditionally resolved resource namespaces
   (reading projects.yaml and roles.yaml) from validateWorkflows, which
   runs BEFORE pushCore refreshes the clone. A stale manifest mapped new
   resources to the wrong namespace (Tencent#649 review). Now the manifest is
   only read when namespaced workflow candidates exist, wrapped in
   try/catch so an unreadable manifest degrades to on-disk namespaces.

2. resolveResourceNamespaces at the docs/wiki annotation site had no
   error handling: a manifest that exists but cannot be read produced an
   unhandled rejection instead of the graceful exitCode=2 stop every
   other manifest-error path uses.

3. The pre-pull snapshot capture was widened from env-only to every
   placeable type, but restorePendingEnvFiles writes to
   clone/<relativePath>. For project-local sources (tool-format agents,
   rules, skills) that wrote tool-format bytes over the clone's canonical
   files, so the next scan reported "cannot read or parse canonical agent
   YAML" and dropped rules entirely. Now only sources inside the team
   repo clone are captured — the ones resetToCleanMaster can destroy.
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/push.ts:662 — Still unresolved: placement sets supersedes for an untracked original. Pushing a new workflows/deploy.yaml into ops deletes that original, then includes its nonexistent, untracked path in staging, causing Git to reject the pathspec. Only stage superseded paths that Git tracks.
  • [P1 blocking] src/resources/repo-resource.ts:110 — Still unresolved: relocation reads and writes text. Placing a binary document such as docs/diagram.png into a namespace corrupts its bytes and deletes the original. Copy raw bytes instead.
  • [P2 non-blocking] src/review-branch.ts:108 — Still unresolved: nested docs/wiki identities discard intermediate directories. Co-active docs/a/api/guide.md and docs/b/ui/guide.md incorrectly conflict as guide. Preserve their bundle-relative paths.
  • [P1 blocking] src/workflow/cache.ts:230 — Prompt namespaces are resolved only when a namespaced workflow exists. With only workflows/deploy.yaml, active prompts namespace ops, and prompt_file: prompts/deploy.md stored at prompts/ops/deploy.md, the namespace lookup is skipped and the workflow is rejected as having an unreadable prompt. Resolve prompt namespaces independently of workflow layout.
  • [P2 non-blocking] src/review-branch.ts:120 — MCP scanning returns one item per server, but parseRelativePath replaces every server’s identity with the filename stem mcp. Co-active mcp/a/mcp.yaml and mcp/b/mcp.yaml therefore report duplicate definitions even when their server names are disjoint. Preserve server names for MCP conflict checks.

Review Status
None of the three earlier findings is resolved. Testing documentation meets the stated criterion: the body records a built, real-CLI run. The deferred full E2E suite is not blocking. No PR code was run, built, or installed.

@legend91019
legend91019 force-pushed the feat/push-all-namespaced-resources-1101 branch from 688fee6 to b48ef1f Compare October 21, 2026 09:05
@github-actions

Copy link
Copy Markdown

Review blocked: the exact requested diff is unavailable.

  • git diff 2aa8a8a...b48ef1f fails with no merge base; the checkout is shallow and the base commit’s parent is missing.
  • GitHub access also failed. A two-dot comparison includes unrelated changes, so it cannot substitute for the requested PR diff.
  • I cannot reliably confirm current findings or mark earlier findings resolved.
  • The supplied PR description meets the testing-documentation criterion: it records a built, real-CLI smoke test. The deferred full E2E suite is not blocking.

Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed.

@legend91019

Copy link
Copy Markdown
Contributor Author

CI fixes pushed (3 bugs: manifest timing, unhandled rejection, capture/restore path confusion). Triggering CI.

@github-actions

Copy link
Copy Markdown

Review blocked: the exact requested diff is unavailable.

  • The three-dot diff fails with no merge base; the checkout is shallow, and the base commit’s ancestry is unavailable.
  • GitHub retrieval also failed. A two-dot comparison cannot safely substitute for the requested PR diff.
  • I cannot confirm current findings or mark earlier findings resolved.
  • The supplied PR description meets the testing-documentation criterion: it records a built, real-CLI smoke test. The deferred full E2E suite is not blocking.

Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed.

@legend91019
legend91019 force-pushed the feat/push-all-namespaced-resources-1101 branch from 91b6c57 to 7961eb0 Compare October 21, 2026 09:17
@github-actions

Copy link
Copy Markdown

Review blocked: the exact requested diff is unavailable.

  • git diff 2aa8a8a...7961eb0 fails with no merge base; the checkout is shallow.
  • GitHub retrieval also failed. A two-dot comparison cannot safely substitute for the requested diff.
  • I cannot confirm current findings or mark earlier findings resolved.
  • The supplied PR description meets the testing-documentation criterion: it records a build and a real-CLI smoke test. The deferred full E2E suite is not blocking.

Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed.

@github-actions

Copy link
Copy Markdown

Review blocked: the exact requested diff is unavailable.

  • git diff 2aa8a8a...514e6c0 fails with no merge base.
  • GitHub retrieval also failed. A two-dot comparison cannot safely substitute for the requested diff.
  • I cannot confirm current findings or mark earlier findings resolved.
  • The supplied PR description meets the testing-documentation criterion: it records a build and a real-CLI smoke test. The deferred full E2E suite is not blocking.

Please provide the exact three-dot diff or a checkout with sufficient history. No PR code was run, built, or installed.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/push.ts:662 — Placement still records an untracked original as supersedes. Pushing a new workflows/deploy.yaml into ops deletes the original, then stages its nonexistent, untracked path, causing Git to reject the pathspec and abort the push. Only stage superseded paths that Git tracks.
  • [P1 blocking] src/resources/repo-resource.ts:110 — Relocation still copies through text I/O. Placing a binary document such as docs/diagram.png into a namespace corrupts its bytes, then deletes the original. Copy raw bytes instead.
  • [P1 blocking] src/workflow/cache.ts:230 — Prompt namespace resolution still depends on a namespaced workflow existing. With only workflows/deploy.yaml, active prompts namespace ops, and prompt_file: prompts/deploy.md stored at prompts/ops/deploy.md, resolution is skipped and the workflow is rejected as having an unreadable prompt. Resolve prompt namespaces independently.
  • [P2 non-blocking] src/review-branch.ts:108 — Nested docs/wiki identities still discard intermediate directories. Co-active docs/a/api/guide.md and docs/b/ui/guide.md incorrectly conflict as guide, despite distinct bundle-relative paths. Preserve the path below the namespace.
  • [P2 non-blocking] src/review-branch.ts:139 — MCP scanning supplies server names, but parseRelativePath replaces each with the filename stem mcp. Co-active mcp/a/mcp.yaml and mcp/b/mcp.yaml therefore report duplicate definitions even when their server names are disjoint. Preserve server identities.

Review Status
None of the five earlier code findings is resolved in the current diff. The exact requested three-dot diff is now available and was reviewed.

Testing documentation meets the criterion: the body records a build and a real-CLI smoke test. The deferred full E2E suite is not blocking. No PR code was run, built, or installed.

placeNewResources marked the pre-placement root path as supersedes when
moving a new resource into a namespace. pushRepoBranch then staged that
path with git add, but the root file was never in the clone, so git
aborted with 'pathspec did not match any files'. supersedes is for
pushItem retiring a tracked file during a format rename (agent .md to
.yaml); a placement from the shared root to a namespace is not a rename.
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/resources/repo-resource.ts:110 — Relocation still uses text I/O. Placing a binary document such as docs/diagram.png into a namespace corrupts its bytes, then deletes the original. Copy raw bytes instead.
  • [P1 blocking] src/workflow/cache.ts:233 — Prompt namespace resolution still requires a namespaced workflow. With only workflows/deploy.yaml, active prompts namespace ops, and prompt_file: prompts/deploy.md stored at prompts/ops/deploy.md, lookup is skipped and the workflow is rejected. Resolve prompt namespaces independently.
  • [P2 non-blocking] src/review-branch.ts:108 — Nested docs/wiki identities still discard intermediate directories. Co-active docs/a/api/guide.md and docs/b/ui/guide.md incorrectly conflict as guide. Preserve their bundle-relative paths.
  • [P2 non-blocking] src/review-branch.ts:139 — MCP server identities are still replaced with the filename stem mcp. Co-active mcp/a/mcp.yaml and mcp/b/mcp.yaml incorrectly report duplicates even when their server names are disjoint. Preserve the scanner’s server names.

Review Status
The earlier untracked-path staging finding is resolved; the four findings above remain unresolved. The exact requested three-dot diff was reviewed.

Testing documentation meets the criterion: the body records a build and a real-CLI smoke test. The deferred full E2E suite is not blocking. No PR code was run, built, or installed.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

@legend91019 @jeff-r2026 This re-reviews 91209e5 against our review at fd2c98b. We built main and the head and ran teamai push --all on macOS against a local git team repo. Findings marked reproduced come from that run. The rest are source traces.

Fixed since the last review. New resources placed into a namespace no longer abort at git add (R1, and the CI supersedes finding).

Blocking.

  • [P1 blocking] src/resources/repo-resource.ts:107-111. Relocation copies through a UTF-8 text read, so binary files are corrupted. A 70-byte PNG pushed under docs/ reached the branch as 94 bytes. Reproduced.
  • [P1 blocking] src/push.ts:1161,2008. The snapshot is keyed by the root path but deleted by the placed path, so the root copy is restored after the push. Once the placed file merges, every later push --all exits 2 with "already exists", and an unrelated new skill in the same run is not pushed. Reproduced.
  • [P1 blocking] src/workflow/cache.ts:232-243. Prompt namespaces now resolve only when a namespaced workflow exists. A root workflows/review.yaml with prompts: [ops] declared ignores prompts/ops/review.md. It fails without a root prompt and silently uses the shared one otherwise. The manifest-free and nested path cases also remain. Traced.
  • [P1 blocking] src/workflow/cache.ts:235-247. The new catch turns a manifest error into "every namespace active". A malformed manifest/roles.yaml makes workflows/svc-b/deploy.yaml cached and runnable for a svc-a member, written before push stops. Traced.
  • [P1 blocking] src/resources/env.ts:288-289. Env items carry no status, so placeNewResources (src/push.ts:596-599) skips them. A new env/env.yaml for a project declaring env: [ops] stays at the shared root. Traced.
  • [P1 blocking] src/roles.ts:212-227. Roles still resolve learnings to []. --role ops writes learnings/ops/, which no role-only teammate receives. Traced.
  • [P1 blocking] src/resources/index.ts:23. The wiki handler scans wiki/, but resources.wiki scopes teamwiki/evidence/code/<slug>/ (src/manifest-schema.ts:112-117). An edit there is never pushed. Traced.
  • [P1 blocking] Testing record. The only built-CLI run is teamai review --branch main --json. Nothing runs push, which is this PR's main behavior. Rule: AGENTS.md "Testing before a PR". One representative built-CLI push of a newly supported type is enough. Record the project or role mapping, the command, and the file path and content on the pushed branch.

Non-blocking.

  • [P2 non-blocking] E2E src/__tests__/e2e/push-active-namespaces.test.ts:977 is red in CI shard 1/3 and passes on main. The pre-refresh scan (src/push.ts:1116-1124) runs the skills handler without the selected path, so it prints "Skipped b-skill". Exclude skill, rule and agent scans from the snapshot.
  • [P2 non-blocking] Scorers are pushed to scorers/<ns>/ but nothing reads them on pull (src/resources/repo-resource.ts:161-167).
  • [P2 non-blocking] --role rogue writes workflows/rogue/ without checking declared namespaces (src/push.ts:241-243).
  • [P2 non-blocking] review --branch fails valid sources such as manifest-free claudemd/legacy/ or nested prompt paths (src/review-branch.ts:99-109,232-242).
  • [P2 non-blocking] teamai status shows 0 for the new types (src/status.ts:103-135).
  • [P2 non-blocking] Pre-refresh scan errors become [], and a failed git ls-files marks every file new (src/push.ts:1118-1122, src/resources/repo-resource.ts:76-87).
  • [P2 non-blocking] pushRepoResourceItem keeps an unused third argument and an as LocalConfig cast (src/resources/repo-resource.ts:99-104).
  • [P2 non-blocking] docs/guide/distribution.md:104 (both languages) and the --role/--project help (src/index.ts:173-176) still describe only skills, rules and agents.
  • [P2 non-blocking] review --branch reduces nested docs and wiki identities to the basename, so docs/a/api/guide.md and docs/b/ui/guide.md falsely conflict (src/review-branch.ts:104-108).
  • [P2 non-blocking] review --branch replaces MCP server names with mcp, so mcp/a/mcp.yaml and mcp/b/mcp.yaml falsely conflict (src/review-branch.ts:139-141).

Test Plan. The five unit test paths it lists do not exist in the repo. The changed tests are under src/__tests__/.

Overlap. #1251 also edits the diagnostics list in src/__tests__/workflow-records.test.ts. It drops bad_session and this PR adds two codes. Each PR owns its own entries, and whichever merges second resolves the list.

For the maintainer. The PR adds optional workflows, prompts and scorers keys to manifest/projects.yaml and the roles manifest (src/manifest-schema.ts:74-84,118-120). Members on v0.25.0 through v0.26.0-beta.3 reject unknown resource keys (src/projects.ts, src/roles.ts at those tags). v0.26.0-beta.4 and every later release, including v0.27.0 and v0.28.0-beta.8, warn and keep them (warnUnknownResourceKeys in src/manifest-schema.ts). An admin who declares the new keys breaks pull for teammates on the rejecting versions.

…acement, role learnings, wiki source, and P2s

P1 fixes:
- repo-resource: use fs.copyFile byte copy instead of UTF-8 text I/O to prevent binary doc corruption
- workflow/cache: resolve prompt namespaces independently of workflow layout; support nested prompt paths
- env: add status field to envPushItem so placement routes env/env.yaml to env/<ns>/env.yaml
- roles: include learnings in role namespace resolution (Tencent#1101: 'Learnings should support --project and --role')
- wiki: point handler at teamwiki/evidence/code/ (authoritative source per manifest-schema.ts:112)
- push: filter LOCAL_TOOL_TYPES from pre-refresh scan to suppress spurious warnings

P2 fixes:
- push-namespaces: add rootSegmentsFor() and withNamespace(rootSegments) for wiki's multi-segment root
- review-branch: fix nested path identity collision (use full sub-path as name); preserve handler-provided names
- review-branch: skip undeclared-namespace check in manifest-free teams
- --role: warn when not declared in manifest/roles.yaml
- status: count models/claudemd/learnings/wiki/workflows/prompts/scorers
- push: simplify pushRepoResourceItem signature to LocalConfig only
- docs: update distribution.md and --role/--project help text
- push: tighten pre-refresh snapshot to only capture clone-backed items
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/review-branch.ts:108 — Nested docs identities now contain /, but namingFindings still requires a single safe path segment. A valid docs/a/api/guide.md becomes api/guide and fails review. Validate path components separately.
  • [P1 blocking] src/review-branch.ts:149 — Preserving handler names also preserves the namespace prefix carried by rule names. Namespaced rules consequently fail single-segment name validation, and equivalent rules in different namespaces no longer share a duplicate-check identity. Normalize rule names separately from MCP/skill names.
  • [P1 blocking] src/workflow/cache.ts:557 — Manifest-free prompt resolution remains disabled because activePrompts is null. With workflows/deploy.yaml referencing prompts/deploy.md and the prompt stored at prompts/ops/deploy.md, the workflow is rejected as unreadable. Derive active prompt namespaces from disk in legacy mode, as workflows already do.
  • [P2 non-blocking] src/review-branch.ts:127 — Wiki scanning still uses <repo>/wiki, and READ_ROOTS also includes wiki rather than teamwiki/evidence/code. Wiki resources at the newly corrected authoritative root are never reviewed.
  • [P2 non-blocking] docs/guide/distribution.md:104 — The behavior documentation changes only in English. Update the Chinese distribution guide and regenerate the command reference for the changed option help, as required by the repository instructions.

Review Status
The five earlier findings are resolved in their originally reported form; the findings above identify remaining regressions.

Testing documentation meets the criterion: the description records a build and a real-CLI smoke test. The deferred full E2E suite is not blocking. Only the requested diff was read; no PR code was run, built, or installed.

- push: track prePlacementPath so the snapshot delete finds the entry keyed
  by the root path after placement rewrites item.relativePath
- workflow/cache: a manifest that cannot be read now resolves to an empty
  namespace set (conservative) instead of falling back to 'every namespace
  active', which cached inactive workflows as runnable
- commands.md: regenerate snapshot for updated --role/--project help text

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

@legend91019 @jeff-r2026 This re-reviews 93809cd9 against the comment at 91209e54. The Request changes of 2026-10-20 stays.

What we ran is a built CLI against a local git team repo. The items under Traced were read in source and were not run.

Ran

A 70-byte PNG reached docs/svc-a/diagram.png unchanged (blob f37764b1). The binary-copy finding is resolved. The exit 1 after the branch push is Invalid Git repo URL in this environment.

[P1 blocking] After that push, docs/diagram.png is back, untracked, same 70 bytes. Reproduced. In src/push.ts the snapshot is stored under the scan-time path and removed under the path placement rewrote, so the root file is written back.

[P1 blocking] With env: [svc-a], the CLI logs env/env.yaml → env/svc-a/env.yaml and exits 1 because env/svc-a/env.yaml was never created. The source stays at env/env.yaml. Reproduced. EnvHandler.pushItem in src/resources/env.ts does not copy the file outside self mode, and the push then stages the rewritten path.

Traced

[P1 blocking] Manifest-free prompt namespaces stay unresolved. resolveOwnResourceNamespaces in src/resource-namespaces.ts returns null when there is no role, no active projects, and no projects manifest, so activePrompts stays null and the lookup in src/workflow/cache.ts is skipped. Workflows still fall back to every on-disk namespace. Concrete case: no manifest/roles.yaml and no manifest/projects.yaml, workflows/deploy.yaml references prompts/deploy.md, and the file is only at prompts/ops/deploy.md. The cache does not read prompts/ops/deploy.md.

[P1 blocking] A bad roles manifest still activates every workflow namespace. The catch in src/workflow/cache.ts leaves namespaces unset for any roles-manifest error other than a missing file, and every on-disk workflow namespace stays active. Concrete case: a malformed manifest/roles.yaml makes workflows/svc-b/deploy.yaml cached and runnable for a svc-a member.

[P1 blocking] The PR body has no teamai push record. Testing before a PR in AGENTS.md requires one real built-CLI run of a runtime change. The body still has no teamai push command, project or role mapping, or path and content on the pushed branch.

[P1 blocking] Nested resource names contain / and fail review. parseRelativePath in src/review-branch.ts keeps the sub-path as the name, and namingFindings rejects it because isSafeNamespaceSegment in src/manifest-schema.ts treats / as unsafe. Concrete case: docs/a/api/guide.md with a declared on the docs axis is named api/guide, so teamai review --branch fails it. prompts/ops/topic/note.md with ops declared is named topic/note and fails the same check.

[P1 blocking] Rule names keep the namespace prefix. scanResources in src/review-branch.ts keeps the handler name for rules, and RulesHandler.scanTeamForPull in src/resources/rules.ts sets that name to the relative path with .md removed and sets no namespace. Concrete case: rules/ops/foo.md is named ops/foo, so teamai review fails a normal namespaced rule. rules/a/guide.md and rules/b/guide.md are not compared, because duplicate detection skips items with no namespace.

Non-blocking

[P2 non-blocking] Scorers are still not read on pull: RepoResourceHandler.pullItem in src/resources/repo-resource.ts is empty, and nothing in src/pull.ts reads scorer file contents.

[P2 non-blocking] --role still writes an undeclared namespace: src/push.ts still places files under that role string, the warning does not run when the manifest cannot be read, and it is not checked per resource axis.

[P2 non-blocking] Nested prompts are still treated as a namespace on review: on a team that has a manifest, shared prompts/topic/deep/note.md fails structure because topic is read as the namespace.

[P2 non-blocking] Scan errors still become an empty change set: the pre-refresh catch in src/push.ts returns no changes, and a thrown git ls-files marks repo-resource and env files as new.

[P2 non-blocking] The Chinese distribution guide and the command reference are stale: docs/guide/zh-CN/distribution.md still says only skills and rules are pushed and that docs, hooks, mcp, and models are committed by hand, and skill-data/core/references/commands.md still limits --role and --project to the old type lists.

[P2 non-blocking] Review still reads wiki/, not teamwiki/evidence/code: a page under teamwiki/evidence/code is not copied into the review and is not checked, and wiki/guide.md is parsed with three root segments so the name is empty and review fails it.

[P2 non-blocking] Contribute and the post-contribute index still ignore role learnings: pull indexes the role namespace, while contribute sees only project namespaces and writes the file at the shared root, which pull then delivers to every member.

Overlap

#1191 owns the "Assigning roles" section of docs/guide/distribution.md, roles list --json and the roles set description in src/index.ts, and activeRoleIds's roleAssignment argument in src/roles.ts. This change owns the push sentence in distribution.md, the push --role / --project help in src/index.ts, and the learnings loop in resolveRoleResourceNamespaces. #1208 owns init --provider help, which adds gitea; this change owns the push --role / --project help, different options. #1251 still edits src/__tests__/workflow-records.test.ts. That file is not in 91209e54..93809cd9 and remains in the whole PR. Each PR owns its own diagnostic entries.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/resources/env.ts:384 — New env files now undergo namespace placement, but EnvHandler.pushItem remains a no-op outside self mode. Pushing an untracked env/env.yaml with --role ops stages env/ops/env.yaml without creating it, so Git rejects the pathspec and push fails. Relocate the file before staging.
  • [P1 blocking] src/review-branch.ts:108 — Nested docs identities contain /, but namingFindings still requires one safe path segment. Valid docs/a/api/guide.md becomes api/guide and fails review. Validate individual path components.
  • [P1 blocking] src/review-branch.ts:149 — Preserving handler names also preserves the namespace prefix in rule names. A valid namespaced rule consequently fails single-segment name validation, and equivalent rules across namespaces no longer have the same duplicate-check identity. Normalize rule names separately.
  • [P1 blocking] src/workflow/cache.ts:268 — Manifest-free prompt resolution still leaves activePrompts null. With workflows/deploy.yaml referencing prompts/deploy.md and the prompt at prompts/ops/deploy.md, namespace lookup is skipped and the workflow is rejected as unreadable. Derive active prompt namespaces from disk in legacy mode.
  • [P2 non-blocking] src/review-branch.ts:127 — Wiki scanning uses <repo>/wiki, and READ_ROOTS likewise materializes wiki, not teamwiki/evidence/code. Resources at the newly corrected authoritative wiki root are never reviewed.
  • [P2 non-blocking] docs/guide/distribution.md:104 — The push behavior documentation changes only in English. Update the Chinese distribution guide to match the newly supported resource types and namespace placement.

Review Status
The earlier untracked-path staging and binary-copy findings are resolved. The command reference is now updated; the bilingual documentation issue remains. The other findings from the latest substantive review remain unresolved.

Testing documentation meets the criterion: the description records a build and a representative real-CLI smoke test. The deferred full E2E suite is not blocking. Only the requested diff was read; no PR code was run, built, or installed.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

@legend91019 @jeff-r2026 This re-reviews d77e8f07 against the comment at 93809cd9. The Request changes of 2026-10-20 stays.

What we ran is a built CLI against a local git team repo. The items under Traced were read in source and were not run.

Ran

teamai push --all logs diagram.png → docs/svc-a/diagram.png and pushes teamai/push/member/20261021-182045 at 0c656437. docs/svc-a/diagram.png is 70 bytes, blob f37764b1, identical to the source. docs/diagram.png is absent. A second push --all exits 0 with No new or modified resources to push. The fresh-push restore is resolved. The exit 1 after the branch push is Failed to create PR: Invalid Git repo URL in this environment.

Traced

[P1 blocking] An update of an open push still restores the root file. In src/push.ts, reuseRecordedDestinations rewrites relativePath and does not save the snapshot key. placeNewResources then skips that item, so prePlacementPath stays unset and the delete misses the root path. Concrete case: the branch already has docs/svc-a/diagram.png, the pending record matches diagram.png, and docs/diagram.png is untracked in the clone. teamai push --all takes the reuse path and writes docs/diagram.png back after pr-failed or pushed. The fresh push above had already removed the root file, so that run did not enter this path.

[P1 blocking] A new env/env.yaml is not written to the namespace. Not re-run. src/resources/env.ts is not in this range, and the 93809cd9 repro stands. With env: [svc-a], the CLI logs env/env.yaml → env/svc-a/env.yaml and exits 1 because env/svc-a/env.yaml was never created. The source stays at env/env.yaml. Outside self mode EnvHandler.pushItem does not copy, and the push then stages the rewritten path.

[P1 blocking] Manifest-free prompt namespaces stay unresolved. resolveOwnResourceNamespaces in src/resource-namespaces.ts returns null when there is no role, no active projects, and no projects manifest, so activePrompts stays null in src/workflow/cache.ts and the namespaced lookup is skipped. Workflows still fall back to every on-disk namespace. Concrete case: no manifest/roles.yaml and no manifest/projects.yaml, workflows/deploy.yaml references prompts/deploy.md, and the file is only at prompts/ops/deploy.md. The cache does not read prompts/ops/deploy.md.

[P1 blocking] The PR body's push record quotes a landing the e2e test rejects. Testing before a PR in AGENTS.md requires one real built-CLI run of a runtime change. The body says teamai push --all --skill .claude/skills/a-skill --role platform lands at skills/platform/a-skill/SKILL.md. In src/__tests__/e2e/push-active-namespaces.test.ts, platform is a project id, that command expects to: skills/svc-a/a-skill, and the test asserts skills/platform/a-skill/SKILL.md is absent. The section quotes no file content. Its green claim is for 93809cd, not this head. The PNG run above does not fill this description.

[P1 blocking] Nested resource names contain / and fail review. parseRelativePath in src/review-branch.ts keeps the sub-path as the name, and namingFindings rejects it because isSafeNamespaceSegment in src/manifest-schema.ts treats / as unsafe. Concrete case: docs/a/api/guide.md with a declared on the docs axis is named api/guide, so teamai review --branch fails it. prompts/ops/topic/note.md with ops declared is named topic/note and fails the same check.

[P1 blocking] Rule names keep the namespace prefix. RulesHandler.scanTeamForPull in src/resources/rules.ts sets the name to the relative path with .md removed and sets no namespace. scanResources in src/review-branch.ts keeps that name, and duplicate detection skips items with no namespace. Concrete case: rules/ops/foo.md is named ops/foo, so teamai review fails a normal namespaced rule. rules/a/guide.md and rules/b/guide.md are not compared.

Resolved

A bad roles manifest no longer activates every workflow namespace. Traced, not run. The catch in src/workflow/cache.ts now resolves to an empty namespace set. A malformed manifest/roles.yaml no longer caches workflows/svc-b/deploy.yaml for a svc-a member. It activates no workflow namespace, including the member's own.

Non-blocking

[P2 non-blocking] Scorers are still not read on pull: RepoResourceHandler.pullItem in src/resources/repo-resource.ts is empty, and nothing in src/pull.ts reads scorer file contents.

[P2 non-blocking] --role still writes an undeclared namespace: src/push.ts warns from one flat set of every axis, the warning does not run when the manifest cannot be read, and the role string is still used.

[P2 non-blocking] Nested prompts are still treated as a namespace on review: on a team that has a manifest, shared prompts/topic/deep/note.md fails structure because topic is read as the namespace.

[P2 non-blocking] Scan errors still become an empty change set: the pre-refresh catch in src/push.ts returns no changes, and a thrown git ls-files marks repo-resource and env files as new.

[P2 non-blocking] The Chinese distribution guide is still stale: docs/guide/zh-CN/distribution.md still says only skills and rules are pushed and that docs, hooks, mcp, and models are committed by hand. The commands.md --role / --project help now matches src/index.ts.

[P2 non-blocking] Review still reads wiki/, not teamwiki/evidence/code: a page under teamwiki/evidence/code is not copied into the review, and wiki/guide.md is parsed with an empty name and review fails it.

[P2 non-blocking] Contribute and the post-contribute index still ignore role learnings: pull indexes the role namespace, while contribute sees only project namespaces and writes the file at the shared root.

[P3 nit] The comment above the cache catch in src/workflow/cache.ts still says an unreadable manifest falls back to every on-disk namespace. The catch now assigns an empty set.

Overlap

This range owns the push --role / --project help, prePlacementPath, and the empty-namespace catch. #1191 owns the roles help in commands.md. #1208 owns the init --provider help, which adds gitea. #1251 does not touch these three files.

A placement record held only the destination path, so the self-mode scan
compared the author's root copy with the placed file as it is now. Once a
teammate changed that file, an untouched copy read as an edit and was pushed
over their change, with no warning.

Keep the delivered blob with the record and compare the author's copy with
it: equal is already delivered, different is the author's edit, which goes to
the placed file unless that file changed too, in which case the scan stops and
names the file to edit. An edit that lands refreshes the record. A record
holding only the path, as earlier builds wrote, is dropped.
The self-mode env scan compared an unchanged .teamai/env copy with the placed
file as it is now. After a teammate fixed env/ops/env.yaml on the team repo,
the author's old copy read as an edit and the next push carried it over the fix.

Use the same placed-copy check as the other repo-backed scans, which compares
against the blob the push delivered.
--role was taken as a literal namespace on every axis. In a team with
manifest/roles.yaml or manifest/projects.yaml, a namespace no role or project
declares for a type is one pull delivers to nobody, yet push wrote rules/<ns>/,
docs/<ns>/, workflows/<ns>/ and the rest there after a warning that checked
roles.yaml only.

Resolve --role per type against the namespaces both manifests declare for that
type's axis, the way --project resolves against its project, and stop with
exit 2 otherwise. A team with neither manifest still takes any namespace. The
push-role unit fixtures now declare the namespaces their tests pass.
Roles now declare learnings namespaces, and pull delivers learnings/<ns>/ to
the role's members, but contribute still resolved learnings from active
projects only. A role-only member's contribution went to the shared root, and
--namespace rejected the role's own namespace.

Resolve the active learnings namespaces from roles and projects, in the order
pull merges them, in the one function contribute, recall, import and
`projects list` read.
… path

The pre-refresh scan swallowed a handler's error and treated that type as
having no changes, so the clone was refreshed without knowing what the
type held. Let the error propagate to the same catch the env scan uses.
The conflict message read as if the author's edit should replace the team's
change in the placed file. Say the edit is merged into that file, keeping the
team's change, before the shared-root copy is deleted.
Pull has no scorers reader yet (Tencent#1085). Resolve the scorers namespaces a
member does not select, in the same shape prompts use, so that reader can
withhold them.
A member whose roles or project declare one learnings namespace contributed
there by default, and --namespace only accepted declared namespaces, so that
member could not write to the shared root. A member with several namespaces
reaches it by default.

--namespace "" now names the shared root for every member.
@SaulMoro

SaulMoro commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

@legend91019 @jeff-r2026 To close the open items from our reviews, we pushed nine commits on top of 7e6dd4da. We kept your three latest commits as they are. Where they covered the same thing as ours, yours won.

Placement records

  • c15069a stores the delivered blob with each repo-backed placement record ({ path, blob }), and placedCopy compares the author's copy against it. Before this, an unchanged copy of a placed doc was pushed over a teammate's later fix of docs/ops/guide.md, with no warning. A path-only record from an earlier build is read as no record.
  • 87bb24b makes the self-mode env scan use the same placedCopy. Your 161016dd compared against the placed file as it is now, which is the same stale-copy pattern. Your env test now uses the blob record.
  • Kept from you: the env original retired after placement (bb591990, so we dropped our equivalent), the record dropped when its path was deleted since the last check, the shared namespaceOfPlacedPath, explicit prompt namespaces in resolvePromptFiles, and the .md scorer samples.

Namespaces

  • 4c1d3bd: push --role <ns> fails with exit 2 when no role or project declares <ns> for that type, as --project already does. A team with neither manifest is unchanged. This is a behavior change, noted in the CHANGELOG (2ced2a9).
  • 0a2723c: scorers resolve inactive namespaces the same way prompts do. Reading scorers stays with [feat] Add workflow scorers and recorded quality labels #1085, which will read scorers/<ns>/<name>.md through activeNamespaces.scorers.

Contribute

Push

  • 634e6ee: a scan error before the refresh is no longer swallowed as an empty change set. It now reaches the existing pull-failure path.
  • ba89548: when the placed file changed too, the conflict message now says to merge the edit into the placed file.

Verified with the built CLI in self mode against a local bare team repo, on this head and on 7e6dd4da:

  • Teammate fix, then an unrelated push: 7e6dd4da lists guide.md (modified) and the branch carries the old content. This head lists only the new doc and keeps the fix.
  • Push, merge, pull, edit only the placed doc, push again: one item (modified) to docs/ops/guide.md, and env is not listed. pull and doctor both exit 0.
  • --role rogue: 7e6dd4da writes rules/rogue/, docs/rogue/ and workflows/rogue/. This head exits 2 and pushes nothing.
  • Placed path deleted and then re-created by someone else: both heads stop with "cannot be placed: docs/ops/guide.md already exists".

tsc, lint, the full unit suite, commands-reference, and the e2e files push-active-namespaces and contribute-namespace all pass.

@jeff-r2026, one decision is still open. With no active project and no flag, new resources still go to the shared root with a warning per resource. That is main's behavior for rules, and push-active-namespaces.test.ts pins it. We did not change it. We propose narrowing this #1101 criterion to:

A mapping that cannot be resolved fails instead of falling back to the shared root: an unknown --project, a --project that declares no namespace for the type, a --role namespace that no role or project declares for the type, several candidate namespaces with no terminal to ask on, or a roles or projects manifest that cannot be read. When no flag is passed and neither the member's roles nor their active projects declare a namespace for the type, which includes a member with no role and no active project, new resources go to the shared root with a warning per resource, as main does for rules.

@legend91019, the record in the body still covers only skill placement. Something like this would cover this head:

Built-CLI record at bdd76d6, self mode (`.teamai/` checkout of a local bare team repo, provider git, roles.yaml declares `ops` for docs, env and prompts):
1. `teamai push --all --role ops` with a new root doc, a root env and a nested prompt:
   docs/guide.md → docs/ops/guide.md, env/env.yaml → env/ops/env.yaml, prompts/topic/note.md → prompts/ops/topic/note.md
   (exit 1 is PR creation on a generic git host; the branch is pushed)
2. Merge the branch on the bare repo, fast-forward, `teamai pull` (exit 0). state.placements holds { path, blob } for all three.
3. Edit the placed doc only, `teamai push --all --role ops`: Found 1 resource(s): [docs] guide.md (modified) to docs/ops/guide.md.
4. `teamai pull` exit 0, `teamai doctor` exit 0 "All checks passed!".
5. Teammate fixes docs/ops/guide.md on main; the author pushes an unrelated new doc: only the new doc is listed, and the branch keeps the teammate's fix.
6. `teamai push --all --role rogue` (not declared): exit 2, nothing pushed.

Thanks for the quick round on env and prompts.

@github-actions

Copy link
Copy Markdown

No new findings confidently identified in the current diff.

  • Earlier findings are resolved in their originally reported form, including clone-local env cleanup and explicit active-namespace prompt resolution.
  • The PR description meets the testing criterion: it records a build and representative real-CLI E2E assertions. No full provider × agent matrix is required.
  • Reviewed the exact requested three-dot diff against the trusted base review rules. No PR code was run, built, or installed.

Five placement-record cases pushed a rule, a skill and an agent with
--role be-know, which the roles manifest declares only for knowledge. Since
--role has to name a declared namespace for each type it places, that push
stops with exit 2. The cases are about rule records, so they push the rule
alone.
@SaulMoro

Copy link
Copy Markdown
Collaborator

1debe94 fixes the failing e2e shard. In push-namespace-e2e.test.ts, five rule-record cases pushed a rule, a skill and an agent with --role be-know. The fixture declares be-know only for knowledge, so under the new --role check that push stops with exit 2. These cases are about rule records, so they now push only the rule.

@jeff-r2026, I corrected the proposed #1101 wording in the comment above. Without a flag, a member whose active project declares no namespace for a type still goes to the shared root. Verified with the built CLI: an active project that declares only docs sends a new rule to rules/nf-rule.md with the warning. The first wording said that case fails. The corrected text describes what this head does, so accepting it needs no further code change.

@github-actions

Copy link
Copy Markdown

No new findings confidently identified in the current diff.

  • Earlier findings are resolved in their originally reported form, including clone-local env cleanup and explicit active-namespace prompt resolution.
  • The PR description meets the testing criterion: it records a build and representative real-CLI E2E assertions. No full provider × agent matrix is required.
  • Reviewed the exact requested three-dot diff against the trusted base review rules. No PR code was run, built, or installed.

@legend91019

Copy link
Copy Markdown
Contributor Author

@SaulMoro Thank you — I kept your nine commits as they are and rewrote the body's built-CLI record to cover this head (1debe94). Steps 1–4 and 6 reproduce here exactly as you wrote them; doctor is clean and --role rogue exits 2 without pushing.

One thing I could not reproduce is step 5 when the author's checkout is behind main. If the author is current with main, only the new doc is listed and the branch keeps the teammate's fix, as you describe. But if the author has not pulled the teammate's commit yet, the placed copy itself is listed and the pushed branch drops the teammate's line:

member places docs/guide.md → docs/ops/guide.md, merges, pulls
member edits the root copy (v2), pushes, merges, pulls   # record blob = v2
teammate edits docs/ops/guide.md on main (+ "teammate fix")
member does NOT pull, adds docs/handbook.md, push --all --role ops

Found 2 resource(s) to push:
  1. [docs] handbook.md (new)
  2. [docs] ops/guide.md (modified)        <- the placed copy, from .teamai/docs/ops/guide.md
...
branch diff vs main:  M .teamai/docs/ops/guide.md  A .teamai/docs/ops/handbook.md
guide.md on the branch: "# Guide" / "" / "v2 authored"    (the teammate line is gone)

My reading is that placedCopy protects the author's root copy (docs:docs/guide.md → { path, blob }), while the placed path is scanned as an ordinary repo file, so the push refresh's restore hands back the pre-refresh bytes and the branch carries them over the teammate's commit. I did not touch it, since it may be a case you excluded on purpose — is this expected, or should the record suppress a placed copy that still matches the blob it delivered?

Everything else: CI is green on 1debe94 (17 pass, 1 skip for the GitHub-provider E2E), and the body now records the built-CLI run.

… main

Single-repo push compares the member's .teamai/ with a fresh origin/<default>,
so on a checkout behind it a file a teammate changed reads as the member's edit
and the branch carries the old content back. Cover a file only main changed, a
file both changed, a namespaced destination, a format change that deletes a
file main changed, a tool-directory copy edited on top of main, a shallow
clone, a checkout with no shared history, and a stop for a file the push does
not send. A genuine edit of a stale .teamai/rules copy now stops instead of
overwriting the teammate's update.
…ut behind it

For each item sourced from .teamai/, compare the member's file with the
merge-base and the tip of the default branch, at its source, its destination
and any file the push deletes. A file only the default branch changed is left
out with a line saying git pull brings it in. A file both sides changed, a
file the push would delete, or one that cannot be classified for lack of
shared history stops the push with exit 2 before anything is written, naming
the files and the remedy. The stop applies only to items going out.

Tool-directory copies keep their own checks: the pre-push sync moves them to
the default branch's version while .teamai/ stays behind.
@SaulMoro

SaulMoro commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

@legend91019 Thanks for the repro. Your sequence reproduces on 1debe94. Placement is not the cause, though. The same push also lists the never-placed docs/README.md as (modified) and drops the teammate's line from it. In single-repo mode, push compares the member's .teamai/ with a freshly fetched origin/<default>, so on a checkout behind main every file a teammate changed reads as the member's edit. Main already does this with env/env.yaml. This PR widened it to docs, prompts and the other repo-backed types, so I fixed it here.

2c0e940 adds the failing e2e test and 6c4bb84 is the fix, both in src/push.ts with no scan touched. For each item sourced from .teamai/, push now checks its source, its destination and any file a format change would delete against what origin/<default> changed since the merge-base:

The member's copy Result
matches the merge-base (only main changed it) left out, with Left out <file>: changed on origin/main since this checkout branched from it, and not edited here.
differs from both (both changed it) exit 2, names the file, nothing pushed
a format change would delete a file main changed exit 2, names the file to carry the change into
no merge-base (shallow clone, unrelated history) exit 2, suggests git fetch --unshallow origin or a merge

The stop applies only to files in the final selection, so push --skill other is not blocked by an unrelated env.yaml.

One existing test changed. In push-sync-followups-823, editing a stale .teamai/rules copy that a teammate updated on main used to push over that update. It now stops with exit 2 and names the file.

Out of scope here, both pre-existing on main:

Verified on 6c4bb84:

  • npx tsc --noEmit, npm run lint, and npx vitest run: 12238 passed, 0 failed.
  • The 7 e2e files for self-mode and namespaced push pass. The new self-mode-push-behind test fails on 2c0e940 and passes on 6c4bb84.
  • Your sequence with the built CLI. Only docs/ops/handbook.md is pushed. README.md and ops/guide.md keep the teammate's line on the branch, and push prints Left out .teamai/docs/README.md, .teamai/docs/ops/guide.md. The same check with env/env.yaml leaves env out instead of restoring the old value.

If you agree, please add this case to the body's record. Your step 5 with the checkout behind main now lists only handbook.md.

@github-actions

Copy link
Copy Markdown

No new findings confidently identified in the current diff.

  • Earlier findings are resolved in their originally reported form, including clone-local env cleanup and explicit active-namespace prompt resolution.
  • The PR description meets the testing criterion: it records a build and representative real-CLI E2E assertions. The record at older commit 1debe94 is not blocking; no full provider × agent matrix is required.
  • Reviewed the exact requested three-dot diff against the checked-out base’s trusted Code Review Rules. No PR code was run, built, or installed.

@legend91019

Copy link
Copy Markdown
Contributor Author

@SaulMoro Agreed — and thanks for the fix. 6c4bb84 resolves the case, and your diagnosis was right: it was the stale checkout, not placement. docs/README.md, which was never placed, was listed the same way, which is what I missed.

I re-ran the steps on 6c4bb84 with the built CLI (self mode, manifest/roles.yaml declares ops for docs, env and prompts):

  • Steps 1–4 reproduce as before: the three placements still land at docs/ops/guide.md, env/ops/env.yaml, prompts/ops/topic/note.md with { path, blob } records; step 3 still lists only docs/ops/guide.md (modified); pull and doctor exit 0 (All checks passed!).
  • Step 5, checkout behind main:
    ℹ Left out .teamai/docs/ops/guide.md, .teamai/docs/README.md: changed on origin/main since this checkout branched from it, and not edited here.
    Found 1 resource(s) to push:
        1. [docs] handbook.md (new)  → docs/ops/handbook.md
    
    Branch diff vs main is A .teamai/docs/ops/handbook.md only, and ops/guide.md and README.md both keep the teammate's line on the branch.
  • When the same file is edited on the stale checkout too, push stops with exit 2 and names it (your self-mode-push-behind case).
  • npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/self-mode-push-behind.test.ts — 8/8 locally on the built CLI.
  • --role rogue still exits 2 without pushing.

Body updated as you asked: the built-CLI record is now at 6c4bb84, step 5 covers current-with-main / behind-main / behind-main-and-edited, and a 2c0e940..6c4bb84 section records the guard, the table and the push-sync-followups-823 change. CI is green on 6c4bb84 — 17 checks pass, GitHub-provider E2E skipped.

Agreed on leaving the two items out of scope: both need delivery records to be fixed properly, so they belong in their own change. I can open issues for the skill directory source and the stale tool-directory copy so they are tracked — or leave them as #1101 follow-ups if you prefer.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Approving 6c4bb84. This replaces our requests for changes of 2026-10-20 and 2026-10-22. Every finding from those reviews is resolved. All CI checks pass, and the Codex review on this head reports no findings.

@jeff-r2026, against the #1101 acceptance criteria:

Criterion Status at 6c4bb84
Every pull-distributed resource has a push path Met. All 14 types are placed with --role and with --project.
--project resolves from manifest/projects.yaml Met. An unknown project exits 2.
--role targets a namespace allowed by manifest/roles.yaml Met. An undeclared namespace now exits 2. A team with no manifest still accepts any namespace.
Missing or ambiguous mappings fail instead of falling back to the shared root Met for --project and --role, and for several namespaces with none chosen. One case keeps main's behavior: with no flag and no namespace for that type, push goes to the shared root and prints a warning. That is the corrected wording I proposed above. Failing there would stop a member with no role or project from contributing anything.
PR update and direct push work for every type Met. Re-push and placed updates were verified, including env and a checkout behind main.
Pull and push use the same namespace rules Met for push and for workflows and prompts on pull. Reading namespaced scorers on pull belongs to #1085.

We are merging it as a squash, so it lands as the author's contribution. If you want the shared-root case to fail instead, or anything else changed, it can be reverted or followed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Make all distributed resources pushable with project and role namespaces

3 participants