Repository navigation
feat(push): support all namespaced resource types - #1234
Conversation
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. |
There was a problem hiding this comment.
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.mdwith--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 roguepreviewsworkflows/rogue/deploy.yaml, which pull ignores. - [P2 non-blocking]
review --branchflags valid pull sources as undeclared namespaces, for exampleclaudemd/legacy/in a manifest-free team and nestedprompts/topic/deep/note.md(src/review-branch.ts:99-109). - [P2 non-blocking]
teamai statusnow lists the new types but always shows0for 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.scanRepoResourceForPushalso treats every file as new ifgit ls-filesfails. - [P2 non-blocking]
pushRepoResourceItem(item, localConfigOrTeamConfig, maybeLocalConfig?)withas LocalConfig: every caller passeslocalConfig. TakeLocalConfigdirectly. - [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:98and--role/--projecthelp still name only skills/rules/agents. After changing help, regeneratecommands.md. - Test record: the real-CLI run is
teamai review --branch main --json, which does not exercisepush --project/--role. Please add one realpushrun per changed path, e.g. a new docs file with--projectand 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. Withsupersedesunset, the newResourceItem.supersedesfield intypes.tshas no writer and can go. skills/rules/agentswere 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:
pushrunsvalidateWorkflows→syncWorkflowDefinitionsbefore the pull, andsyncWorkflowDefinitionsnow callsresolveResourceNamespaces(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 inpushCorefor 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 inpushCoredo.
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.
|
Findings
Review Status |
688fee6 to
b48ef1f
Compare
|
Review blocked: the exact requested diff is unavailable.
Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed. |
|
CI fixes pushed (3 bugs: manifest timing, unhandled rejection, capture/restore path confusion). Triggering CI. |
|
Review blocked: the exact requested diff is unavailable.
Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed. |
91b6c57 to
7961eb0
Compare
|
Review blocked: the exact requested diff is unavailable.
Please provide the exact three-dot diff or a checkout containing the missing history. No PR code was run, built, or installed. |
|
Review blocked: the exact requested diff is unavailable.
Please provide the exact three-dot diff or a checkout with sufficient history. No PR code was run, built, or installed. |
|
Findings
Review Status 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.
|
Findings
Review Status 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
left a comment
There was a problem hiding this comment.
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 underdocs/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 laterpush --allexits 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 rootworkflows/review.yamlwithprompts: [ops]declared ignoresprompts/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 malformedmanifest/roles.yamlmakesworkflows/svc-b/deploy.yamlcached and runnable for asvc-amember, written before push stops. Traced. - [P1 blocking]
src/resources/env.ts:288-289. Env items carry nostatus, soplaceNewResources(src/push.ts:596-599) skips them. A newenv/env.yamlfor a project declaringenv: [ops]stays at the shared root. Traced. - [P1 blocking]
src/roles.ts:212-227. Roles still resolve learnings to[].--role opswriteslearnings/ops/, which no role-only teammate receives. Traced. - [P1 blocking]
src/resources/index.ts:23. The wiki handler scanswiki/, butresources.wikiscopesteamwiki/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 runspush, 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:977is 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 roguewritesworkflows/rogue/without checking declared namespaces (src/push.ts:241-243). - [P2 non-blocking]
review --branchfails valid sources such as manifest-freeclaudemd/legacy/or nested prompt paths (src/review-branch.ts:99-109,232-242). - [P2 non-blocking]
teamai statusshows 0 for the new types (src/status.ts:103-135). - [P2 non-blocking] Pre-refresh scan errors become
[], and a failedgit ls-filesmarks every file new (src/push.ts:1118-1122,src/resources/repo-resource.ts:76-87). - [P2 non-blocking]
pushRepoResourceItemkeeps an unused third argument and anas LocalConfigcast (src/resources/repo-resource.ts:99-104). - [P2 non-blocking]
docs/guide/distribution.md:104(both languages) and the--role/--projecthelp (src/index.ts:173-176) still describe only skills, rules and agents. - [P2 non-blocking]
review --branchreduces nested docs and wiki identities to the basename, sodocs/a/api/guide.mdanddocs/b/ui/guide.mdfalsely conflict (src/review-branch.ts:104-108). - [P2 non-blocking]
review --branchreplaces MCP server names withmcp, somcp/a/mcp.yamlandmcp/b/mcp.yamlfalsely 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
|
Findings
Review Status 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
left a comment
There was a problem hiding this comment.
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.
|
Findings
Review Status 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
left a comment
There was a problem hiding this comment.
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.
|
@legend91019 @jeff-r2026 To close the open items from our reviews, we pushed nine commits on top of Placement records
Namespaces
Contribute
Push
Verified with the built CLI in self mode against a local bare team repo, on this head and on
@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
@legend91019, the record in the body still covers only skill placement. Something like this would cover this head: Thanks for the quick round on env and prompts. |
|
No new findings confidently identified in the current diff.
|
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.
|
@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 |
|
No new findings confidently identified in the current diff.
|
|
@SaulMoro Thank you — I kept your nine commits as they are and rewrote the body's built-CLI record to cover this head ( 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: My reading is that Everything else: CI is green on |
… 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.
|
@legend91019 Thanks for the repro. Your sequence reproduces on
The stop applies only to files in the final selection, so One existing test changed. In Out of scope here, both pre-existing on main:
Verified on
If you agree, please add this case to the body's record. Your step 5 with the checkout behind main now lists only |
|
No new findings confidently identified in the current diff.
|
|
@SaulMoro Agreed — and thanks for the fix. I re-ran the steps on
Body updated as you asked: the built-CLI record is now at 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
left a comment
There was a problem hiding this comment.
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.
Summary
Type of Change
Test Plan
npm run build/npm run typecheck/npm run lintnpx vitest run src/__tests__/push-role.test.ts— 94/94npx 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/48npx vitest run src/__tests__/commands-reference.test.ts— 1/1 (snapshot regenerated)npx vitest run src/__tests__/workflow-records.test.ts— 43/43Built-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.yamldeclaresopsfor docs, env and prompts.teamai push --all --role opswith a new root doc, a root env and a nested prompt:docs/guide.md→docs/ops/guide.mdenv/env.yaml→env/ops/env.yamlprompts/topic/note.md→prompts/ops/topic/note.mdteamai pull→ exit 0.state.placementsholds{ path, blob }for all three.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).teamai pull→ exit 0,teamai doctor→ exit 0 (All checks passed!). The record's blob is refreshed to what that push delivered.docs/ops/guide.mdanddocs/README.mdon main; the author pushes an unrelated new doc: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 mainbrings the change in.Found 1 resource(s)—[docs] handbook.md (new), placed todocs/ops/handbook.md; the branch adds only that file, and both files keep the teammate's line.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-namespaceand all threeE2E (fork-safe, no credentials)shards are green in CI on this head. Representative assertions frompush-active-namespaces:push --all --skill <path> --role svc-a→ lands atskills/svc-a/foo/SKILL.mdon the push branch (line 649, 683)--project platformmaps to the namespaceplatformdeclares inmanifest/projects.yamlcopy it under a new name and push that(line 448–455)Review Fixes
Commit 93809cd — addressing @SaulMoro's review
P1
pushRepoResourceItemusesfs.copyFile(byte copy) instead of UTF-8 text I/OenvPushItemitems carrystatus, so placement routesenv/env.yaml→env/<ns>/env.yamlresolveRoleResourceNamespacesincludeslearningsteamwiki/evidence/code/LOCAL_TOOL_TYPESP2 —
rootSegmentsFor()for wiki multi-segment root; review-branch nested identity + handler-provided names; manifest-free undeclared-namespace skip;--roleundeclared warning; status counts;pushRepoResourceItemsignature; snapshot tightening; docs + help textCommit d77e8f0 — re-review findings
prePlacementPathso the snapshot delete finds the entry keyed by the root path after placement rewritesitem.relativePath--role/--projecthelp textLater review rounds (through
7e6dd4d)reuseRecordedDestinationssavesprePlacementPathbefore rewritingrelativePathEnvHandler.pushItemcopies whenever source ≠ destination, and retires a clone-local original once it has been placedparseRelativePathkeeps each path segment valid on its ownRulesHandler.scanTeamForPullextracts name and namespace separatelyteamwiki/evidence/code, docs/wiki keep their bundle-relative path, and nested prompts are shared pathsprompt_filewhose first segment is already an active namespace resolves as written.mdnames, since a scorer isscorers/<name>.mdMaintainer follow-up
c15069a..1debe94(on top of this branch)Placement records store the delivered blob;
--rolemust name a declared namespace (exit 2 otherwise); scorers resolve inactive namespaces like prompts; contribute uses the member's role learnings and accepts an empty--namespacefor 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 fetchedorigin/<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 forenv/env.yaml; this PR widened it to docs, prompts and the other repo-backed types, so it is fixed here rather than left in.6c4bb84compares each push item sourced from.teamai/— at its source, its destination, and any file a format change would delete — against whatorigin/<default>changed since the merge-base:Left out <file>: changed on origin/main since this checkout branched from it, and not edited here.git fetch --unshallow originor a mergeThe stop covers only files in the final selection, so
push --skill otheris not blocked by an unrelatedenv.yaml.2c0e940addsself-mode-push-behind, which fails before6c4bb84;push-sync-followups-823changed with the fix: editing a stale.teamai/rulescopy 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.