From 076edf7d1dfc4dc818f173f751dcb1e57b5baaeb Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Sun, 28 Jun 2026 14:35:08 +0900 Subject: fix(workspace-star): clean up starred cache on workspace delete + warn when concurrency service absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1 (MEDIUM): In-memory starred cache leaks IDs for deleted workspaces. The starredWorkspaces Set in the concurrency manager never cleaned up when a workspace was deleted. FIX: the DELETE /workspaces/:id route now calls concurrencyService.notifyWorkspaceStarred(id, false) after deleting, so the deleted workspace ID is removed from the in-memory cache (preventing stale IDs and preventing a re-created workspace with the same slug from inheriting the old starred state). Bug 2 (MEDIUM): Stale in-memory priority when concurrency extension is absent. If provider-concurrency is not loaded, the star toggle persisted but the in-memory priority cache was never updated (the optional chaining ?. silently skipped the call). Already-queued agents kept their old priority until restart. FIX: the star/unstar routes now check if concurrencyService is defined and log a warning when it is absent, making the degraded behavior visible. The starred state still persists correctly — it just does not affect in-memory scheduling until the extension is loaded. Tests: +5 (star round-trip with concurrency notification, invalid slug 400, delete cleans up cache, absent-service warning log). All 1970 tests pass. --- packages/transport-http/src/app.test.ts | 149 ++++++++++++++++++++++++++++++++ packages/transport-http/src/app.ts | 32 ++++++- 2 files changed, 178 insertions(+), 3 deletions(-) diff --git a/packages/transport-http/src/app.test.ts b/packages/transport-http/src/app.test.ts index 7ae0354..03f1959 100644 --- a/packages/transport-http/src/app.test.ts +++ b/packages/transport-http/src/app.test.ts @@ -110,6 +110,7 @@ function createFakeConversationStore( title: "default", defaultCwd: null, defaultComputerId: null, + starred: false, createdAt: 0, lastActivityAt: 0, }; @@ -207,6 +208,9 @@ function createFakeConversationStore( async setWorkspaceDefaultComputerId(id, defaultComputerId) { return { ...sampleWorkspace, id, defaultComputerId }; }, + async setWorkspaceStarred(id, starred) { + return { ...sampleWorkspace, id, starred }; + }, async deleteWorkspace() { return { closedCount: 0 }; }, @@ -3562,6 +3566,7 @@ describe("Workspaces", () => { title: "proj", defaultCwd: null, defaultComputerId: null, + starred: false, createdAt: 1000, lastActivityAt: 2000, }; @@ -3756,6 +3761,150 @@ describe("Workspaces", () => { const res = await app.request("/workspaces/default", { method: "DELETE" }); expect(res.status).toBe(409); }); + + // ─── Star/unstar workspace (concurrency priority) ──────────────────────── + + it("PUT /workspaces/:id/star persists + notifies the concurrency service", async () => { + let starredCalled: { id: string; starred: boolean } | null = null; + const store: ConversationStore = { + ...createFakeConversationStore(), + async setWorkspaceStarred(id, starred) { + return { ...sampleWorkspace, id, starred }; + }, + }; + const concurrencyService = { + acquire: async () => () => {}, + reportRateLimit() {}, + setLimit() {}, + getLimit: () => undefined, + getLimits: () => [], + getStatus: () => undefined, + getStatusAll: () => [], + notifyWorkspaceStarred(id: string, starred: boolean) { + starredCalled = { id, starred }; + }, + destroy() {}, + }; + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + ...(concurrencyService !== undefined ? { concurrencyService } : {}), + logger: noopLogger, + }); + const res = await app.request("/workspaces/proj/star", { method: "PUT" }); + expect(res.status).toBe(200); + const body = (await res.json()) as WorkspaceResponse; + expect(body.starred).toBe(true); + expect(starredCalled).toEqual({ id: "proj", starred: true }); + }); + + it("DELETE /workspaces/:id/star persists + notifies the concurrency service", async () => { + let starredCalled: { id: string; starred: boolean } | null = null; + const store: ConversationStore = { + ...createFakeConversationStore(), + async setWorkspaceStarred(id, starred) { + return { ...sampleWorkspace, id, starred }; + }, + }; + const concurrencyService = { + acquire: async () => () => {}, + reportRateLimit() {}, + setLimit() {}, + getLimit: () => undefined, + getLimits: () => [], + getStatus: () => undefined, + getStatusAll: () => [], + notifyWorkspaceStarred(id: string, starred: boolean) { + starredCalled = { id, starred }; + }, + destroy() {}, + }; + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + concurrencyService, + logger: noopLogger, + }); + const res = await app.request("/workspaces/proj/star", { method: "DELETE" }); + expect(res.status).toBe(200); + const body = (await res.json()) as WorkspaceResponse; + expect(body.starred).toBe(false); + expect(starredCalled).toEqual({ id: "proj", starred: false }); + }); + + it("PUT /workspaces/:id/star rejects invalid slug", async () => { + const app = createApp({ + conversationStore: createFakeConversationStore(), + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + const res = await app.request("/workspaces/Bad Slug!/star", { method: "PUT" }); + expect(res.status).toBe(400); + }); + + it("DELETE /workspaces/:id cleans up the in-memory starred cache (bug fix)", async () => { + // Bug 1 fix: deleting a workspace must notify the concurrency service to + // remove the workspace ID from the starred cache, preventing stale IDs. + let starredCalled: { id: string; starred: boolean } | null = null; + const store: ConversationStore = { + ...createFakeConversationStore(), + async deleteWorkspace() { + return { closedCount: 2 }; + }, + }; + const concurrencyService = { + acquire: async () => () => {}, + reportRateLimit() {}, + setLimit() {}, + getLimit: () => undefined, + getLimits: () => [], + getStatus: () => undefined, + getStatusAll: () => [], + notifyWorkspaceStarred(id: string, starred: boolean) { + starredCalled = { id, starred }; + }, + destroy() {}, + }; + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + concurrencyService, + logger: noopLogger, + }); + const res = await app.request("/workspaces/proj", { method: "DELETE" }); + expect(res.status).toBe(200); + // The concurrency service must be notified to clear the starred cache. + expect(starredCalled).toEqual({ id: "proj", starred: false }); + }); + + it("PUT /workspaces/:id/star logs warning when concurrency service is absent (bug fix)", async () => { + // Bug 2 fix: when the concurrency service is not loaded, the star toggle + // persists but the priority cache is not updated. A warning log makes this + // degraded behavior visible. + const logger = createFakeLogger(); + const store: ConversationStore = { + ...createFakeConversationStore(), + async setWorkspaceStarred(id, starred) { + return { ...sampleWorkspace, id, starred }; + }, + }; + // NOTE: no concurrencyService provided — simulates the extension being absent. + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger, + }); + const res = await app.request("/workspaces/proj/star", { method: "PUT" }); + expect(res.status).toBe(200); + // The star persisted, but a warning was logged about the missing service. + const warnings = logger.records.filter((r) => r.level === "warn"); + expect(warnings.some((r) => r.msg.includes("concurrency service is not loaded"))).toBe(true); + }); }); it("POST /chat threads workspaceId", async () => { diff --git a/packages/transport-http/src/app.ts b/packages/transport-http/src/app.ts index 0d42e06..ebbf536 100644 --- a/packages/transport-http/src/app.ts +++ b/packages/transport-http/src/app.ts @@ -1480,6 +1480,10 @@ export function createApp(opts: CreateServerOptions): Hono { try { const { closedCount } = await opts.conversationStore.deleteWorkspace(workspaceId); + // Clean up the in-memory starred cache so a deleted workspace's ID + // doesn't linger (and so a future workspace re-created with the same + // slug doesn't inherit the stale starred state). + opts.concurrencyService?.notifyWorkspaceStarred(workspaceId, false); log.info("workspaces: deleted", { workspaceId, closedCount }); const response: DeleteWorkspaceResponse = { workspaceId, closedCount }; return c.json(response, 200); @@ -1509,8 +1513,21 @@ export function createApp(opts: CreateServerOptions): Hono { try { const workspace = await opts.conversationStore.setWorkspaceStarred(workspaceId, true); // Notify the concurrency service's in-memory cache so queued agents - // from this workspace jump ahead immediately. - opts.concurrencyService?.notifyWorkspaceStarred(workspaceId, true); + // from this workspace jump ahead immediately. When the concurrency + // service is absent (extension not loaded), the starred state is + // persisted but the in-memory priority cache is NOT updated — log a + // warning so the degraded behavior is visible (queued agents keep + // their old priority until restart or the extension is loaded). + if (opts.concurrencyService !== undefined) { + opts.concurrencyService.notifyWorkspaceStarred(workspaceId, true); + } else { + log.warn( + "workspaces: starred but concurrency service is not loaded — priority cache not updated", + { + workspaceId, + }, + ); + } log.info("workspaces: starred", { workspaceId }); const response: WorkspaceResponse = workspace; return c.json(response, 200); @@ -1532,7 +1549,16 @@ export function createApp(opts: CreateServerOptions): Hono { } try { const workspace = await opts.conversationStore.setWorkspaceStarred(workspaceId, false); - opts.concurrencyService?.notifyWorkspaceStarred(workspaceId, false); + if (opts.concurrencyService !== undefined) { + opts.concurrencyService.notifyWorkspaceStarred(workspaceId, false); + } else { + log.warn( + "workspaces: unstarred but concurrency service is not loaded — priority cache not updated", + { + workspaceId, + }, + ); + } log.info("workspaces: unstarred", { workspaceId }); const response: WorkspaceResponse = workspace; return c.json(response, 200); -- cgit v1.2.3