fix(pull): report actual sync outcome instead of team-repo item count

[Morrowga]

Fixes #574
Fixes #585

What this fixes

pull and init --agent both had the same underlying issue: the reported outcome didn't reflect what was actually written to disk.

pull (rules): pull.ts used the team-repo's rule count (scanTeamForPull's result) for the Synced N rule(s) message and totalSynced tally — regardless of whether any configured tool's rules/ directory actually existed. RulesHandler.pullItem() correctly skips writing to an uninstalled tool (by design, per isToolInstalled's own docstring), but that skip is only logged at debug level. So a user with no tool directories yet would see ✔ Synced 7 rule(s) with zero files actually written — exactly #574's repro.

init --agent: declaring --agent claude,codex never created those tool directories or ran an initial sync — it only printed "will auto-sync on each session start," relying entirely on a SessionStart hook that may not fire before the user's first real session.

--force: didn't bypass isToolInstalled anywhere, despite --force's own description ("sync regardless").

Changes

• pull.ts: added hasInstalledRulesTarget() a single check per pull (not per-rule, since isToolInstalled doesn't vary across rules in the same batch) that gates whether the success message/count fires. When no tool is installed, warns with actionable next steps instead of claiming success.
• rules.ts: pullAllRules() takes an optional force param; when true, pre-creates each configured tool's rules/ directory before the write loop, so isToolInstalled naturally passes and the existing write path proceeds — rather than special-casing a bypass inside pullItem (which isn't possible without changing the ResourceHandler abstract contract shared by every resource type).
• known-agents.ts / init.ts / bootstrap.ts: renamed seedSelfModeToolDirs → seedEnabledAgentDirs (per this issue's own suggestion) and reused it in both init's git and http paths. init --agent now seeds the declared tools' directories and runs a real pullForScope immediately.
• pull.ts: exported pullForScope so init.ts can call it directly for the initial sync.

Scope

This covers rules specifically, matching this issue's repro and acceptance criteria. The same phantom-success pattern (team-repo count vs. actual-write count) exists in pull.ts for skills/docs/env/agents/team-culture/shared-instructions — same shape, different call sites. Flagging this as a known related gap rather than expanding this PR further; happy to follow up separately if useful.

Verification

• npx tsc --noEmit clean
• Full existing suite: 234 files / 3267 tests passing (one pre-existing flaky timeout in local-agent.test.ts's uninstall_teamai tests, confirmed via git stash to fail identically on unmodified main under full-suite load — unrelated to this change)
• Added tests for both the rules-reporting fix and the init --agent sync, each confirmed to fail on unmodified code and pass with the fix (via git stash/stash pop)
• Manually ran teamai init <repo> --agent claude end-to-end against a real throwaway GitHub repo and fresh directory — confirmed .claude/rules/*.md actually lands on disk with no manual mkdir, matching the acceptance criteria

Not covered

• --force bypass is implemented for rules only, not the other resource types
• Multi-provider verification (gitlab/tgit) — tested against GitHub specifically

[SaulMoro]

The hasInstalledRulesTarget gate is the right shape, and the test that fails on unmodified code is the part that makes this reviewable. One observation that could turn the "known related gap" in your Scope section into a few lines here, rather than a follow-up PR.

That per-tool installed-directory test already exists seven times in src/pull.ts. At cccbe1d, before this PR:

where	line	field it reads
skills sync	368	toolPath.skills
getInstalledResourceTargets	487	skills, rules, agents
tombstone cleanup	771	toolPathField, from the tombstoneTypes table at 753
skills cleanup	825	toolPath.skills
rules cleanup	992, 1023	toolPath.rules
agents cleanup	1266	toolPath.agents

Every one walks scopedToolPaths, reads a single toolPath field, calls ResourceHandler.isToolInstalled, and filters on isAgentExcluded. hasInstalledRulesTarget makes it eight.

If that helper takes the field as a parameter instead, something like hasInstalledTargetFor(teamConfig, localConfig, field), then the generic branch at src/pull.ts:740-745 can gate skills, docs and agents with no new code, and getInstalledResourceTargets reduces to the same helper over its three fields. The phantom count for the other resource types closes here instead of staying open.

Not blocking. Rules-only is a coherent scope if you would rather keep the diff tight for review, and the tests you added do not change either way.

[Morrowga]

The hasInstalledRulesTarget gate is the right shape, and the test that fails on unmodified code is the part that makes this reviewable. One observation that could turn the "known related gap" in your Scope section into a few lines here, rather than a follow-up PR.

That per-tool installed-directory test already exists seven times in src/pull.ts. At cccbe1d, before this PR:

where line field it reads
skills sync 368 toolPath.skills
getInstalledResourceTargets 487 skills, rules, agents
tombstone cleanup 771 toolPathField, from the tombstoneTypes table at 753
skills cleanup 825 toolPath.skills
rules cleanup 992, 1023 toolPath.rules
agents cleanup 1266 toolPath.agents
Every one walks scopedToolPaths, reads a single toolPath field, calls ResourceHandler.isToolInstalled, and filters on isAgentExcluded. hasInstalledRulesTarget makes it eight.

If that helper takes the field as a parameter instead, something like hasInstalledTargetFor(teamConfig, localConfig, field), then the generic branch at src/pull.ts:740-745 can gate skills, docs and agents with no new code, and getInstalledResourceTargets reduces to the same helper over its three fields. The phantom count for the other resource types closes here instead of staying open.

Not blocking. Rules-only is a coherent scope if you would rather keep the diff tight for review, and the tests you added do not change either way.

Good catch generalized it. installedToolsFor(teamConfig, localConfig, field) now backs both the phantom-success gate and getInstalledResourceTargets, and I applied the same gate to the shared skills/agents write path.

One adjustment: left docs out. DocsHandler.pullItem writes unconditionally to a single fixed directory via fse.copy (which creates the destination as needed) there's no isToolInstalled check or per-tool skip there, so there's nothing to gate. Happy to be corrected if I'm missing something on that.

Merge conflict with #598's cleanupTombstonedResources refactor is resolved too kept both, no overlap.

[SaulMoro]

installedToolsFor is the right shape, and you were right about docs — I was wrong to include it. DocsHandler.pullItem copies into one fixed localDocsDir with fse.copy, which creates the destination, so there is no per-tool skip to gate. Nothing to correct there.

One thing from reading the new commit, not blocking.

The gate is a boolean over tools, and the write path is per tool. hasInstalledTargetFor (src/pull.ts:499) reduces installedToolsFor to .length > 0, while SkillsHandler.pullItem skips each uninstalled tool on its own (src/resources/skills.ts:505, and src/resources/agents.ts:421 for agents). So with enabledAgents: [claude, codex], Claude's directory present and Codex's absent, the gate passes and Synced 12 skills prints while Codex receives nothing — #574's report, one level down.

The list is already in hand, since installedToolsFor returns the tools and only the caller discards them:

-const hasTarget = await hasInstalledTargetFor(freshConfig, localConfig, type as 'skills' | 'agents');
-if (!hasTarget) {
+const installed = await installedToolsFor(freshConfig, localConfig, type as 'skills' | 'agents');
+if (installed.length === 0) {
   log.warn(`… no installed tool directory found — nothing written…`);
 } else {
   …
-  log.success(`[${scopeLabel}] Synced ${items.length} ${type}`);
+  log.success(`[${scopeLabel}] Synced ${items.length} ${type} → ${installed.join(', ')}`);

That turns the count into a claim a reader can check, and it costs a variable. Whether a partially missing tool should warn as well as be omitted from that list is a design call I would leave to you — naming the tools that were written is already most of the value for someone reading their terminal.

Unrelated, in case the file list raises an eyebrow: #599 (phase 1 of #598) rewrites src/doctor.ts, which also appears here. The two do not collide — your branch's copy of that file is main's, carried in with the upstream merge.

[Morrowga]

installedToolsFor is the right shape, and you were right about docs — I was wrong to include it. DocsHandler.pullItem copies into one fixed localDocsDir with fse.copy, which creates the destination, so there is no per-tool skip to gate. Nothing to correct there.

One thing from reading the new commit, not blocking.

The gate is a boolean over tools, and the write path is per tool. hasInstalledTargetFor (src/pull.ts:499) reduces installedToolsFor to .length > 0, while SkillsHandler.pullItem skips each uninstalled tool on its own (src/resources/skills.ts:505, and src/resources/agents.ts:421 for agents). So with enabledAgents: [claude, codex], Claude's directory present and Codex's absent, the gate passes and Synced 12 skills prints while Codex receives nothing — #574's report, one level down.

The list is already in hand, since installedToolsFor returns the tools and only the caller discards them:

-const hasTarget = await hasInstalledTargetFor(freshConfig, localConfig, type as 'skills' | 'agents');
-if (!hasTarget) {
+const installed = await installedToolsFor(freshConfig, localConfig, type as 'skills' | 'agents');
+if (installed.length === 0) {
   log.warn(`… no installed tool directory found — nothing written…`);
 } else {
   …
-  log.success(`[${scopeLabel}] Synced ${items.length} ${type}`);
+  log.success(`[${scopeLabel}] Synced ${items.length} ${type} → ${installed.join(', ')}`);

That turns the count into a claim a reader can check, and it costs a variable. Whether a partially missing tool should warn as well as be omitted from that list is a design call I would leave to you — naming the tools that were written is already most of the value for someone reading their terminal.

Unrelated, in case the file list raises an eyebrow: #599 (phase 1 of #598) rewrites src/doctor.ts, which also appears here. The two do not collide — your branch's copy of that file is main's, carried in with the upstream merge.

Fixed exactly as you showed installedToolsFor now feeds the message directly instead of getting collapsed to a boolean. Synced 1 agents → claude when codex isn't installed, confirmed with a test (two configured tools, one installed) that fails on the old boolean-based code via git stash and passes with the fix.

One thing worth flagging: the same gap exists in the rules block (src/pull.ts:718) —hasInstalledTargetFor(freshConfig, localConfig, 'rules') has the identical boolean-collapse problem, and rules is actually #574's original report. I left it as-is for now since your diff was scoped to skills/agents, but happy to apply the same fix there too if you'd like this PR to close that gap as well rather than leaving it open.

Also caught and fixed an unrelated typo of my own while making this change a missing closing brace that broke the file's parse, caught by tsc --noEmit before it got anywhere near a commit.

[SaulMoro]

Yes, close rules here. It is the case #574 actually reported, so fixing the phantom count everywhere except that line would read oddly from the issue's side. Same shape as what you just did:

-          const hasTarget = await hasInstalledTargetFor(freshConfig, localConfig, 'rules');
-          if (hasTarget) {
-            log.success(`[${scopeLabel}] Synced ${items.length} rule(s)${skipped.length > 0 ? ` (skipped ${skipped.length} by tags)` : ''}`);
+          const installed = await installedToolsFor(freshConfig, localConfig, 'rules');
+          if (installed.length > 0) {
+            log.success(`[${scopeLabel}] Synced ${items.length} rule(s) → ${installed.join(', ')}${skipped.length > 0 ? ` (skipped ${skipped.length} by tags)` : ''}`);

After that, hasInstalledTargetFor (src/pull.ts:499) has no callers left. It only ever existed for the line you are replacing, so it can go with it.

Skills kept the opaque count

The gate landed for skills, the message did not. Skills logs through logSyncDetail, which never receives the list:

src/pull.ts:830   const installed = await installedToolsFor(...)
            831   if (installed.length === 0)   log.warn(...)
            839   else if (type === 'skills')   logSyncDetail(...)   prints "Synced 12 skills (all updated)"
            841   else                          log.success(...)     prints "Synced 1 agents → claude"

So with enabledAgents: [claude, codex], .claude/skills present and .codex absent, skills still prints Synced 12 skills (all updated) while codex receives nothing. That is the terminal line #574 was filed about, and skills is the resource type with the most items in most teams.

logSyncDetail has exactly one caller, so the parameter is cheap:

 function logSyncDetail(
   type: ResourceType,
   items: ResourceItem[],
   existingNames: Set<string>,
   verbose: boolean,
   scopeLabel?: string,
   skippedCount?: number,
+  installedTools?: string[],
 ): void {
   const prefix = scopeLabel ? `[${scopeLabel}] ` : '';
+  const target = installedTools?.length ? ` → ${installedTools.join(', ')}` : '';

Then ${type}${target} in its three log.success branches, and pass installed at the call site on line 839.

One asymmetry to know about

In rules the check runs after pullAllRules, and pullAllRules pre-creates the tool directories when --force is set. So under --force the rules list will name every configured tool, which is the right answer, but it is reporting what exists now. In skills and agents the check runs before the writes, so those lists report what existed before. Neither is a bug. It matters if you write a test that asserts on the rules message with --force.

[jeff-r2026]

Please resolve the P1 finding and conflicts.

[github-actions]

Findings

• [P1 blocking] src/resources/agent-format.ts:56 redeclares both AGENT_FILE_EXTENSIONS and agentStemFromFilename, already declared at lines 37 and 43. This produces duplicate-identifier errors and prevents TypeScript compilation/build.
• [P1 blocking] src/init.ts:488 and src/init.ts:1651 call pullForScope with two arguments, but src/pull.ts:738 requires the reported: Set<string> argument. This is another compile error; if bypassed, reported.add(...) can also fail at runtime.
• [P1 blocking] src/resources/rules.ts:330 makes pull --force create rule directories for every configured tool without checking isAgentExcluded. With default toolPaths, this creates directories for disabled or unselected agents. It also uses resolveBaseDir rather than resolveToolBaseDir, so user-scope Copilot files are seeded outside COPILOT_HOME. Additionally, --force is documented as bypassing the unchanged-revision cache, not opting users into every tool.
• [P1 blocking] src/init.ts:489 and src/init.ts:1652 always print “Synced skills, rules, and docs” after pullForScope returns, but pullForScope handles failures such as refresh errors or missing teamai.yaml by logging and returning normally. Init can therefore report a successful initial sync when nothing was synced—the same class of misleading output this PR aims to fix.
• [P1 blocking] The PR description lacks the repository-required e2e matrix. It records only one GitHub/Claude real-CLI run and explicitly says GitLab/TGit were not tested; AGENTS.md requires real-CLI verification for Claude, Codex, CodeBuddy, and OpenCode across git, gitlab, and github providers before PR.