mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-12 22:14:22 +00:00
fix(management): serialize lifecycle maintenance paths
- serialize deleteInstance with the same profile and plugin-layout locks as ensure - lock non-account marketplace normalization paths and ignore .locks as an instance source
This commit is contained in:
1 parent
fc02c4b968
commit
36e8ed5d87
9 files changed
+115
-11
No files matched your search
@@ -163,7 +163,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
|
|||||||
|
|
||||||
if (!profileExistedBeforeCreate) {
|
if (!profileExistedBeforeCreate) {
|
||||||
try {
|
try {
|
||||||
ctx.instanceMgr.deleteInstance(profileName);
|
await ctx.instanceMgr.deleteInstance(profileName);
|
||||||
} catch {
|
} catch {
|
||||||
// Best-effort cleanup.
|
// Best-effort cleanup.
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -68,7 +68,7 @@ export async function handleRemove(ctx: CommandContext, args: string[]): Promise
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Delete instance
|
// Delete instance
|
||||||
ctx.instanceMgr.deleteInstance(profileName);
|
await ctx.instanceMgr.deleteInstance(profileName);
|
||||||
|
|
||||||
// Delete profile from appropriate config
|
// Delete profile from appropriate config
|
||||||
if (isUnifiedMode() && existsUnified) {
|
if (isUnifiedMode() && existsUnified) {
|
||||||
|
|||||||
@@ -153,15 +153,22 @@ class InstanceManager {
|
|||||||
/**
|
/**
|
||||||
* Delete instance for profile
|
* Delete instance for profile
|
||||||
*/
|
*/
|
||||||
deleteInstance(profileName: string): void {
|
async deleteInstance(profileName: string): Promise<void> {
|
||||||
const instancePath = this.getInstancePath(profileName);
|
const instancePath = this.getInstancePath(profileName);
|
||||||
|
|
||||||
if (!fs.existsSync(instancePath)) {
|
if (!fs.existsSync(instancePath)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Recursive delete
|
await this.contextSyncLock.withLock(profileName, async () => {
|
||||||
fs.rmSync(instancePath, { recursive: true, force: true });
|
await this.pluginLayoutLock.withNamedLock('__plugin-layout__', async () => {
|
||||||
|
if (!fs.existsSync(instancePath)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
fs.rmSync(instancePath, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -173,6 +180,10 @@ class InstanceManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return fs.readdirSync(this.instancesDir).filter((name) => {
|
return fs.readdirSync(this.instancesDir).filter((name) => {
|
||||||
|
if (name.startsWith('.')) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
const instancePath = path.join(this.instancesDir, name);
|
const instancePath = path.join(this.instancesDir, name);
|
||||||
return fs.statSync(instancePath).isDirectory();
|
return fs.statSync(instancePath).isDirectory();
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -176,6 +176,79 @@ class ProfileContextSyncLock {
|
|||||||
async withLock<T>(profileName: string, callback: () => Promise<T>): Promise<T> {
|
async withLock<T>(profileName: string, callback: () => Promise<T>): Promise<T> {
|
||||||
return this.withNamedLock(profileName, callback);
|
return this.withNamedLock(profileName, callback);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
withNamedLockSync<T>(lockName: string, callback: () => T): T {
|
||||||
|
const lockPath = this.getLockPath(lockName);
|
||||||
|
const retryDelayMs = 50;
|
||||||
|
const staleLockMs = 30000;
|
||||||
|
const timeoutMs = staleLockMs + 5000;
|
||||||
|
const start = Date.now();
|
||||||
|
const ownerPayload: ContextSyncLockPayload = {
|
||||||
|
version: 1,
|
||||||
|
pid: process.pid,
|
||||||
|
nonce: createHash('sha1')
|
||||||
|
.update(`${process.pid}:${Date.now()}:${Math.random()}`)
|
||||||
|
.digest('hex')
|
||||||
|
.slice(0, 16),
|
||||||
|
acquiredAtMs: Date.now(),
|
||||||
|
};
|
||||||
|
const ownerPayloadRaw = JSON.stringify(ownerPayload);
|
||||||
|
|
||||||
|
fs.mkdirSync(this.locksDir, { recursive: true, mode: 0o700 });
|
||||||
|
|
||||||
|
while (true) {
|
||||||
|
try {
|
||||||
|
const fd = fs.openSync(lockPath, 'wx', 0o600);
|
||||||
|
fs.writeFileSync(fd, ownerPayloadRaw, 'utf8');
|
||||||
|
fs.closeSync(fd);
|
||||||
|
break;
|
||||||
|
} catch (error) {
|
||||||
|
const err = error as NodeJS.ErrnoException;
|
||||||
|
if (err.code !== 'EEXIST') {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
|
||||||
|
const lockSnapshot = this.readContextSyncLockSnapshot(lockPath);
|
||||||
|
if (lockSnapshot) {
|
||||||
|
if (this.tryRemoveDeadOwnerLock(lockPath, lockSnapshot)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!lockSnapshot.owner) {
|
||||||
|
try {
|
||||||
|
const lockStats = fs.statSync(lockPath);
|
||||||
|
if (Date.now() - lockStats.mtimeMs > staleLockMs) {
|
||||||
|
if (this.tryRemoveLockIfUnchanged(lockPath, lockSnapshot.raw)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Best-effort stale lock cleanup.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Date.now() - start > timeoutMs) {
|
||||||
|
throw new Error(`Timed out waiting for profile context lock: ${lockName}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
const until = Date.now() + retryDelayMs;
|
||||||
|
while (Date.now() < until) {
|
||||||
|
// Busy wait only during rare lock contention.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
return callback();
|
||||||
|
} finally {
|
||||||
|
this.tryRemoveLockIfUnchanged(lockPath, ownerPayloadRaw);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
withLockSync<T>(profileName: string, callback: () => T): T {
|
||||||
|
return this.withNamedLockSync(profileName, callback);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
export default ProfileContextSyncLock;
|
export default ProfileContextSyncLock;
|
||||||
@@ -9,6 +9,7 @@
|
|||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
|
import ProfileContextSyncLock from './profile-context-sync-lock';
|
||||||
import { ok, info, warn } from '../utils/ui';
|
import { ok, info, warn } from '../utils/ui';
|
||||||
import { AccountContextPolicy, DEFAULT_ACCOUNT_CONTEXT_GROUP } from '../auth/account-context';
|
import { AccountContextPolicy, DEFAULT_ACCOUNT_CONTEXT_GROUP } from '../auth/account-context';
|
||||||
import { getCcsDir } from '../utils/config-manager';
|
import { getCcsDir } from '../utils/config-manager';
|
||||||
@@ -114,6 +115,7 @@ class SharedManager {
|
|||||||
private readonly sharedDir: string;
|
private readonly sharedDir: string;
|
||||||
private readonly claudeDir: string;
|
private readonly claudeDir: string;
|
||||||
private readonly instancesDir: string;
|
private readonly instancesDir: string;
|
||||||
|
private readonly pluginLayoutLock: ProfileContextSyncLock;
|
||||||
private readonly sharedItems: SharedItem[];
|
private readonly sharedItems: SharedItem[];
|
||||||
private readonly sharedPluginEntries: readonly SharedItem[] = [
|
private readonly sharedPluginEntries: readonly SharedItem[] = [
|
||||||
{ name: 'cache', type: 'directory' },
|
{ name: 'cache', type: 'directory' },
|
||||||
@@ -134,6 +136,7 @@ class SharedManager {
|
|||||||
this.sharedDir = path.join(ccsDir, 'shared');
|
this.sharedDir = path.join(ccsDir, 'shared');
|
||||||
this.claudeDir = path.join(this.homeDir, '.claude');
|
this.claudeDir = path.join(this.homeDir, '.claude');
|
||||||
this.instancesDir = path.join(ccsDir, 'instances');
|
this.instancesDir = path.join(ccsDir, 'instances');
|
||||||
|
this.pluginLayoutLock = new ProfileContextSyncLock(this.instancesDir);
|
||||||
this.sharedItems = [
|
this.sharedItems = [
|
||||||
{ name: 'commands', type: 'directory' },
|
{ name: 'commands', type: 'directory' },
|
||||||
{ name: 'skills', type: 'directory' },
|
{ name: 'skills', type: 'directory' },
|
||||||
@@ -785,6 +788,12 @@ class SharedManager {
|
|||||||
this.normalizeMarketplaceRegistryPaths(configDir);
|
this.normalizeMarketplaceRegistryPaths(configDir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
normalizeSharedPluginMetadataPathsLocked(configDir?: string): void {
|
||||||
|
this.pluginLayoutLock.withNamedLockSync('__plugin-layout__', () => {
|
||||||
|
this.normalizeSharedPluginMetadataPaths(configDir);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Normalize plugin registry paths to use canonical ~/.claude/ paths
|
* Normalize plugin registry paths to use canonical ~/.claude/ paths
|
||||||
* instead of instance-specific ~/.ccs/instances/<name>/ paths.
|
* instead of instance-specific ~/.ccs/instances/<name>/ paths.
|
||||||
@@ -890,7 +899,7 @@ class SharedManager {
|
|||||||
|
|
||||||
if (fs.existsSync(this.instancesDir)) {
|
if (fs.existsSync(this.instancesDir)) {
|
||||||
for (const entry of fs.readdirSync(this.instancesDir, { withFileTypes: true })) {
|
for (const entry of fs.readdirSync(this.instancesDir, { withFileTypes: true })) {
|
||||||
if (!entry.isDirectory()) {
|
if (!entry.isDirectory() || entry.name.startsWith('.')) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -168,7 +168,7 @@ async function resolveExtensionEnv(
|
|||||||
profileType: result.type,
|
profileType: result.type,
|
||||||
target: 'claude',
|
target: 'claude',
|
||||||
});
|
});
|
||||||
new SharedManager().normalizeSharedPluginMetadataPaths(continuity.claudeConfigDir);
|
new SharedManager().normalizeSharedPluginMetadataPathsLocked(continuity.claudeConfigDir);
|
||||||
if (continuity.claudeConfigDir) {
|
if (continuity.claudeConfigDir) {
|
||||||
notes.push(`Default profile inherits continuity from account "${continuity.sourceAccount}".`);
|
notes.push(`Default profile inherits continuity from account "${continuity.sourceAccount}".`);
|
||||||
return {
|
return {
|
||||||
@@ -250,7 +250,7 @@ async function resolveExtensionEnv(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
new SharedManager().normalizeSharedPluginMetadataPaths(env.CLAUDE_CONFIG_DIR);
|
new SharedManager().normalizeSharedPluginMetadataPathsLocked(env.CLAUDE_CONFIG_DIR);
|
||||||
|
|
||||||
if (result.type === 'copilot') {
|
if (result.type === 'copilot') {
|
||||||
warnings.push(
|
warnings.push(
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ export function execClaude(
|
|||||||
|
|
||||||
if (profileType !== 'account') {
|
if (profileType !== 'account') {
|
||||||
try {
|
try {
|
||||||
new SharedManager().normalizeSharedPluginMetadataPaths(env.CLAUDE_CONFIG_DIR);
|
new SharedManager().normalizeSharedPluginMetadataPathsLocked(env.CLAUDE_CONFIG_DIR);
|
||||||
} catch {
|
} catch {
|
||||||
// Best-effort normalization should never block Claude launch.
|
// Best-effort normalization should never block Claude launch.
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -324,7 +324,7 @@ router.delete('/reset-default', (_req: Request, res: Response): void => {
|
|||||||
/**
|
/**
|
||||||
* DELETE /api/accounts/:name - Delete an account
|
* DELETE /api/accounts/:name - Delete an account
|
||||||
*/
|
*/
|
||||||
router.delete('/:name', (req: Request, res: Response): void => {
|
router.delete('/:name', async (req: Request, res: Response): Promise<void> => {
|
||||||
try {
|
try {
|
||||||
const { name } = req.params;
|
const { name } = req.params;
|
||||||
|
|
||||||
@@ -371,7 +371,7 @@ router.delete('/:name', (req: Request, res: Response): void => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Match CLI remove ordering: delete instance first, metadata second.
|
// Match CLI remove ordering: delete instance first, metadata second.
|
||||||
instanceMgr.deleteInstance(name);
|
await instanceMgr.deleteInstance(name);
|
||||||
|
|
||||||
if (existsUnified) {
|
if (existsUnified) {
|
||||||
registry.removeAccountUnified(name);
|
registry.removeAccountUnified(name);
|
||||||
|
|||||||
@@ -159,6 +159,17 @@ describe('InstanceManager MCP sync', () => {
|
|||||||
expect(String(warnSpy.mock.calls[0]?.[0] || '')).toContain('MCP sync skipped');
|
expect(String(warnSpy.mock.calls[0]?.[0] || '')).toContain('MCP sync skipped');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not list lock housekeeping as an instance', async () => {
|
||||||
|
spyOn(SharedManager.prototype, 'syncProjectContext').mockResolvedValue(undefined);
|
||||||
|
spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined);
|
||||||
|
spyOn(InstanceManager.prototype, 'syncMcpServers').mockImplementation(() => false);
|
||||||
|
|
||||||
|
const manager = new InstanceManager();
|
||||||
|
await manager.ensureInstance('work', { mode: 'isolated' });
|
||||||
|
|
||||||
|
expect(manager.listInstances()).toEqual(['work']);
|
||||||
|
});
|
||||||
|
|
||||||
it('skips shared symlinks and MCP sync for bare instance creation', async () => {
|
it('skips shared symlinks and MCP sync for bare instance creation', async () => {
|
||||||
const linkSharedSpy = spyOn(
|
const linkSharedSpy = spyOn(
|
||||||
SharedManager.prototype,
|
SharedManager.prototype,
|
||||||
|
|||||||
Reference in new issue
Block a user