fix(view): sync command skills, memory, and grok detection

[muqsitnawaz]

Closes #187.

Summary

• Fixes Bug A1 without changing the registry result shape. The issue hypothesis about SyncResult / writer result shape was wrong: writers still return { synced: string[] }, syncResourcesToVersion maps that into result.commands, and the view reporter reads the existing keys.
• Fixes command-as-skill collisions by distinguishing marker-bearing generated command skills from genuine same-name skills. Generated command skills are rewritten from the command source; genuine source-skill collisions are reported as synced no-ops so the view reporter stays accurate.
• Fixes Bug A2 by making the memory skip guard skip only explicit empty memory selections, not every partial selection where memory is absent.
• Fixes Bug B by resolving Grok binaries from each managed version home (home/.grok/downloads) instead of the host ~/.grok config symlink.

Root Causes

• A1: installCommandSkillToVersion treated every same-name source skill as a hard skip, so colliding command names produced an empty writer result and stale command-skill files were never refreshed.
• A1 follow-through: in the full sync path, marker-bearing source skills could also be copied by the skills writer after command sync. syncResourcesToVersion now keeps generated command skills authoritative for command-as-skill agents while leaving genuine skills alone.
• A2: skipMemory treated selection.memory === undefined as an explicit exclusion, so view-sync partial selections skipped rules even when the agent was rules-capable and AGENTS.md was missing.
• B: Grok install detection looked under host ~/.grok/downloads; after config symlink switching, that path could point at a version home with no binary.

Verification

• bun test src/lib/staleness/writers/commands.test.ts -> 2 passed, 0 failed.
• bun test src/lib/versions.test.ts -> 8 passed, 0 failed.
• bun run build -> exit 0. The build script printed the existing non-fatal cp: bin/Agents CLI.app: No such file or directory from the guarded macOS app-copy branch.
• bun run test -> 189 test files passed, 3 skipped; 2364 tests passed, 51 skipped.
• Real PTY E2E: isolated HOME, dist/index.js view grok, host .grok symlink pointed at empty 0.2.32, binary placed only in 0.2.33/home/.grok/downloads. Output included Grok (available), accepted Sync new resources? Yes, sync all new, and reported Synced to Grok@0.2.33: commands, memory. Filesystem checks confirmed the per-version binary still existed, skills/debug/SKILL.md contained fresh command body + agents_command: "debug", and .grok/AGENTS.md contained the composed memory body.

[prix-cloud]

Code Reviewer

Verdict: Changes requested

Build: Clean (tsc exit 0, expected non-fatal macOS app-copy warning on Linux)
Tests: 10/10 for changed files. Full suite: 1 failure in src/lib/browser/port.test.ts — pre-existing environment issue (port-occupant detection via lsof//proc/net doesn't work in this container); CI on GitHub passes green on both Node 22 and 24.

Operating instructions read: AGENTS.md (repo root — CLAUDE.md is a symlink to it, confirmed).

---

Changes that work well

Bug A2 (skipMemory) — the one-liner at src/lib/versions.ts:1922 is correct. Array.isArray(undefined) is false, so a partial selection like { skills: [] } no longer skips the rules writer when memory is absent from the selection object. The test at versions.test.ts:199 reproduces the exact symptom and passes.

Bug B (resolveGrokBinary + getBinaryPath) — per-version home paths now decouple binary detection from the host ~/.grok symlink. The "latest by mtime" fallback in agents.ts:128-142 now stores the full binary path in latest directly (old code stored the filename then rejoined at the return site — new code does it right). The test at versions.test.ts:223 exercises the symlink-traps-binary scenario end-to-end.

commandsSubdir guard at versions.ts:1809: if (commandsAsSkills && agentConfig.commandsSubdir) prevents removePath(agentDir) for grok, whose commandsSubdir is ''. The old if (commandsAsSkills) would have deleted the entire grok agent dir on every sync. Good catch.

Collision detection in installCommandSkillToVersion — switching from { success: false, skipped: true } to { success: true, skipped: true } for genuine-skill collisions is correct: the command shows as synced in the view reporter without overwriting the user's real skill.

---

Issue that needs attention

Orphan sweep deletes command skills in full-sync mode

File: src/lib/versions.ts:1873

const trustedSkills = new Set(skillsToSync);  // ← missing command names

In a no-selection (full) sync, when a source skill at ~/.agents/skills/<name>/SKILL.md carries agents_command: "<name>", the PR correctly filters <name> out of skillsToSync (lines 1846-1857) so the skills writer doesn't clobber what the commands writer just produced. But the orphan sweep on line 1873 builds trustedSkills from that same filtered list. <name> is no longer in trustedSkills, so the sweep deletes agentDir/skills/<name>/ — the directory the commands writer populated a few lines earlier.

Concrete path that triggers this:

1. User has ~/.agents/commands/debug.md AND ~/.agents/skills/debug/SKILL.md with agents_command: "debug" in frontmatter (the exact scenario Bug A1 targets).
2. User runs agents commands add debug → calls syncResourcesToVersion(agentId, version) with no selection (src/commands/commands.ts:321).
3. Commands writer writes agentDir/.grok/skills/debug/SKILL.md ✓
4. skillsToSync filter excludes debug ✓
5. Orphan sweep: trustedSkills = Set([]), finds agentDir/.grok/skills/debug/, removes it ✗

Same path fires on agents sync --yes grok@<version> (non-interactive, selection stays undefined).

Why the existing tests miss this: both A1 tests use explicit selections ({ commands: ['debug'], skills: ['debug'] }) which set userPassedSelection = true, bypassing the !userPassedSelection guard at line 1872. A test with selection = undefined and a marker-bearing source skill would catch this.

Fix (one line):

// versions.ts:1873 — include command-as-skill names in the trusted set
const trustedSkills = new Set([
  ...skillsToSync,
  ...(commandsAsSkills ? commandsToSync : []),
]);

This ensures that names the commands writer just wrote into agentDir/skills/ are never swept by the skills orphan pass.

---

Things to verify manually

The PTY E2E in the PR description uses agents view grok (interactive sync prompt), which results in an explicit selection — userPassedSelection = true — so it does not exercise the orphan sweep. Please add a no-selection full-sync test that confirms the generated command skill survives after syncResourcesToVersion('grok', version, undefined, { cwd }) when a marker-bearing source skill is present.

---

Reviewed by Code Reviewer — actually ran the build and tests on this branch.