fix(cli): close second review on prime-agent sync + switcher (auth, ownership, switching)
Some checks failed
CI/CD / lint (pull_request) Successful in 1m4s
CI/CD / typecheck (pull_request) Successful in 1m5s
CI/CD / test (pull_request) Successful in 3m7s
CI/CD / smoke (pull_request) Failing after 3m13s
CI/CD / build (pull_request) Successful in 2m12s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / lint (pull_request) Successful in 1m4s
CI/CD / typecheck (pull_request) Successful in 1m5s
CI/CD / test (pull_request) Successful in 3m7s
CI/CD / smoke (pull_request) Failing after 3m13s
CI/CD / build (pull_request) Successful in 2m12s
CI/CD / publish (pull_request) Has been skipped
Addresses the second round of `config prime-agent` review (10 findings). auth.json (config/prime-agent.ts) — the settings.json data-loss fix had a twin: - loadPrimeAgentAuth now fails loudly on corrupt JSON instead of swallow-and- rewrite, so one syntax error can no longer destroy the provider API key and every other project's credential. hasPrimeAgentAuth shares that guarantee. - writePrimeAgentAuth writes 0600 (preserving an existing file's mode) instead of the default umask — bearer tokens are no longer world-readable on first creation. state ownership (commands/skills.ts) — the ownership model edge cases: - orphan-removal guard now normalises a canonical scope (project name, or null for globals; legacy undefined adopted to current scope) instead of comparing null against undefined, so global-only syncs and pre-PR state can no longer leave stale skills on disk forever. - a same-named skill *tracked* to a different project is preserved (with a warning) rather than silently overwritten in the shared flat tree. - mcpServers auto-attach is gated behind !isPrimeAgent with an explicit warning (prime-agent's HTTP gateway must not mutate shared mcpd project attachments); this also makes the earlier dropped-attach concern explicit rather than silent. single-active project + switcher (config/prime-agent.ts, prime-agent-extension.ts): - registerPrimeAgentMcp tags the project's entry mcpctlManaged:true and removes other mcpctl-managed entries, so prime-agent has one *active* mcpctl project while preserving untagged servers (hand-configured sre, websearch, etc). - the /mcpctl extension now reads that tag as the single source of truth for the active project, fixing the false short-circuit / no-op switch. config.ts command: - an explicit -p now updates a differing up-tree .mcpctl-project marker (scope no longer silently reverts on the next sync), no-ops when it matches, and still never scopes $HOME. - a project left with no usable credential now exits non-zero (the /mcpctl extension checks child exit status, so it no longer reports a successful switch after provisioning failed). - skills sync is treated as best-effort: settings+auth determine switch success, so a skills error no longer falsely fails the switch. - token minting now revokes prior active `prime-agent` tokens before creating a fresh one (no more never-expiring token litter / lost-credential duplication). Tests (544 green): corrupt auth.json refusal, 0600 mode, mint-failure exit code, single-active dedup preserving untagged sre, cross-project overwrite preservation, and global-orphan removal on global-only sync.
This commit is contained in:
@@ -183,4 +183,46 @@ describe('runPrimeAgentSkillsSync', () => {
|
||||
expect(result2.removed).toContain('old-skill');
|
||||
expect(existsSync(join(installRoot, 'old-skill'))).toBe(false);
|
||||
});
|
||||
|
||||
it('does not overwrite a same-named skill owned by a different project', async () => {
|
||||
// Project A installs skill X.
|
||||
const av = [
|
||||
{ id: 'a-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:a', metadata: {}, scope: 'project' },
|
||||
];
|
||||
const af = { 'a-1': { id: 'a-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:a', content: '# version-a\n', files: {} } };
|
||||
const clientA = mockClient({ visible: av, full: af });
|
||||
await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(clientA));
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n');
|
||||
|
||||
// Project B also has a skill named X with different content.
|
||||
const bv = [
|
||||
{ id: 'b-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:b', metadata: {}, scope: 'project' },
|
||||
];
|
||||
const bf = { 'b-1': { id: 'b-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:b', content: '# version-b\n', files: {} } };
|
||||
const clientB = mockClient({ visible: bv, full: bf });
|
||||
const resultB = await runPrimeAgentSkillsSync({ project: 'projB', installRoot, statePath }, deps(clientB));
|
||||
|
||||
expect(resultB.preserved).toContain('x-skill');
|
||||
// A's version is untouched (not clobbered by B's).
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n');
|
||||
});
|
||||
|
||||
it('removes global orphans on a global-only sync', async () => {
|
||||
// First sync a global skill.
|
||||
const v = [
|
||||
{ id: 'g-1', name: 'gone-global', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', metadata: {}, scope: 'global' },
|
||||
];
|
||||
const f = { 'g-1': { id: 'g-1', name: 'gone-global', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', content: '# g\n', files: {} } };
|
||||
const client1 = mockClient({ visible: v, full: f });
|
||||
const empty = join(tmpDir, 'empty2');
|
||||
mkdirSync(empty, { recursive: true });
|
||||
await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(client1));
|
||||
expect(existsSync(join(installRoot, 'gone-global'))).toBe(true);
|
||||
|
||||
// Next global-only sync: the global is gone from the visible set.
|
||||
const client2 = mockClient({ visible: [], full: {} });
|
||||
const result2 = await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(client2));
|
||||
expect(result2.removed).toContain('gone-global');
|
||||
expect(existsSync(join(installRoot, 'gone-global'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user