Avoid live workspace inspection in task context reads (#17396)

This commit is contained in:
Nicky Leach authored and GitHub committed 2026-10-23 17:05:06 -07:00
1 parent cc127f5516
commit 100477e887
7 files changed
+39 -17

No files matched your search

+6
View File
@@ -107,6 +107,12 @@ and service tests do not establish that a live model followed the workflow.
See [implementation and acceptance](plans/2026-10-19-persistent-case-handoffs.md)
and the [agent case reference](../skills/paperclip/references/cases.md).
Task detail, heartbeat context, and closed-workspace admission guards read stored
workspace metadata. They do not inspect Git or refresh live runtime health.
Task context includes persisted runtime service state; delivery state is unknown
until an explicit workspace read or lifecycle inspection computes it. Use the
workspace detail and close-readiness APIs for current Git safety checks.
## Deployment Modes
For mode definitions and intended CLI behavior, see `doc/DEPLOYMENT-MODES.md`.
@@ -949,6 +949,14 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => {
cwd: seed.worktreePath, baseRef: "main", config,
deliveryState: "unknown", runtimeServices: [],
});
const runtimeServiceId = randomUUID();
const [workspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, seed.executionWorkspaceId));
await insertRunningWorkspaceService(runtimeServiceId, seed.companyId, workspace!.projectId!, seed.executionWorkspaceId, workspace!.sourceIssueId!, seed.worktreePath);
expect(await service.getStoredById(seed.companyId, seed.executionWorkspaceId, { includeRuntimeServices: true })).toMatchObject({
deliveryState: "unknown", runtimeServices: [{ id: runtimeServiceId, status: "running", healthStatus: "healthy" }],
});
await expect(service.getStoredById(randomUUID(), seed.executionWorkspaceId, { includeRuntimeServices: true }))
.rejects.toThrow("Task workspace access is no longer available");
await expect(service.getStoredById(randomUUID(), seed.executionWorkspaceId))
.rejects.toThrow("Task workspace access is no longer available");
expect(await service.getStoredById(seed.companyId, randomUUID())).toBeNull();
@@ -21,7 +21,8 @@ const mockIssueService = vi.hoisted(() => ({
}));
const mockExecutionWorkspaceService = vi.hoisted(() => ({
getById: vi.fn(),
getById: vi.fn(() => { throw new Error("Task routes must not inspect a workspace"); }),
getStoredById: vi.fn(),
reopenClosedIsolatedExecutionWorkspaceForIssue: vi.fn(),
clearReopenPendingConsumptionForUnconsumedReopen: vi.fn(async () => ({ cleared: true })),
refreshReopenPendingConsumption: vi.fn(async () => ({ refreshed: true })),
@@ -244,7 +245,7 @@ describe("closed isolated workspace issue routes", () => {
beforeEach(() => {
vi.clearAllMocks();
mockIssueService.getById.mockResolvedValue(makeIssue());
mockExecutionWorkspaceService.getById.mockResolvedValue(makeClosedWorkspace());
mockExecutionWorkspaceService.getStoredById.mockResolvedValue(makeClosedWorkspace());
mockExecutionWorkspaceService.refreshReopenPendingConsumption.mockResolvedValue({ refreshed: true });
// The guard reopens a closed isolated workspace and lets the request
// continue. The default is a successful reopen.
@@ -26,7 +26,8 @@ const mockAgentService = vi.hoisted(() => ({
}));
const mockExecutionWorkspaceService = vi.hoisted(() => ({
getById: vi.fn(),
getById: vi.fn(() => { throw new Error("Task routes must not inspect a workspace"); }),
getStoredById: vi.fn(),
}));
const mockFeedbackService = vi.hoisted(() => ({
@@ -230,7 +231,7 @@ describe("issue workspace command authorization", () => {
companyId: "company-1",
permissions: null,
});
mockExecutionWorkspaceService.getById.mockResolvedValue(null);
mockExecutionWorkspaceService.getStoredById.mockResolvedValue(null);
mockFeedbackService.listIssueVotesForUser.mockResolvedValue([]);
mockFeedbackService.saveIssueVote.mockResolvedValue({
vote: null,
@@ -34,7 +34,8 @@ const mockDocumentsService = vi.hoisted(() => ({
}));
const mockExecutionWorkspaceService = vi.hoisted(() => ({
getById: vi.fn(),
getById: vi.fn(() => { throw new Error("Task routes must not inspect a workspace"); }),
getStoredById: vi.fn(),
}));
const mockAccessService = vi.hoisted(() => ({
@@ -221,7 +222,7 @@ describe("issue goal context routes", () => {
mockIssueService.listAttachments.mockResolvedValue([]);
mockDocumentsService.getIssueDocumentPayload.mockResolvedValue({});
mockDocumentsService.getIssueDocumentByKey.mockResolvedValue(null);
mockExecutionWorkspaceService.getById.mockResolvedValue(null);
mockExecutionWorkspaceService.getStoredById.mockResolvedValue(null);
const emptyQuery: any = {};
emptyQuery.from = vi.fn(() => emptyQuery);
emptyQuery.innerJoin = vi.fn(() => emptyQuery);
@@ -393,7 +394,7 @@ describe("issue goal context routes", () => {
updatedAt: new Date("2026-03-20T00:00:00Z"),
},
});
mockExecutionWorkspaceService.getById.mockResolvedValueOnce({
mockExecutionWorkspaceService.getStoredById.mockResolvedValueOnce({
id: workspaceId,
companyId: "company-1",
projectId: legacyProjectLinkedIssue.projectId,
@@ -526,12 +527,12 @@ describe("issue goal context routes", () => {
]);
});
it("surfaces the current execution workspace from GET /issues/:id/heartbeat-context", async () => {
it.each(["", "/heartbeat-context"])("reads stored workspace context without Git inspection: %s", async (suffix) => {
mockIssueService.getById.mockResolvedValue({
...legacyProjectLinkedIssue,
executionWorkspaceId: "55555555-5555-4555-8555-555555555555",
});
mockExecutionWorkspaceService.getById.mockResolvedValue({
mockExecutionWorkspaceService.getStoredById.mockResolvedValue({
id: "55555555-5555-4555-8555-555555555555",
name: "PAP-581 workspace",
mode: "isolated_workspace",
@@ -549,11 +550,12 @@ describe("issue goal context routes", () => {
});
const res = await request(createApp()).get(
"/api/issues/11111111-1111-4111-8111-111111111111/heartbeat-context",
`/api/issues/11111111-1111-4111-8111-111111111111${suffix}`,
);
expect(res.status).toBe(200);
expect(mockExecutionWorkspaceService.getById).toHaveBeenCalledWith("55555555-5555-4555-8555-555555555555");
expect(mockExecutionWorkspaceService.getById).not.toHaveBeenCalled();
expect(mockExecutionWorkspaceService.getStoredById).toHaveBeenCalledWith("company-1", "55555555-5555-4555-8555-555555555555", { includeRuntimeServices: true });
expect(res.body.currentExecutionWorkspace).toEqual(expect.objectContaining({
id: "55555555-5555-4555-8555-555555555555",
mode: "isolated_workspace",
+5 -4
View File
@@ -7686,11 +7686,12 @@ export function issueRoutes(
return commentCreatedAtMs >= activeRunStartedAtMs;
}
async function getClosedIssueExecutionWorkspace(issue: {
companyId: string;
executionWorkspaceId?: string | null;
}) {
if (!issue.executionWorkspaceId) return null;
const workspace = await executionWorkspacesSvc.getById(
issue.executionWorkspaceId,
const workspace = await executionWorkspacesSvc.getStoredById(
issue.companyId, issue.executionWorkspaceId,
);
if (!workspace || !isClosedIsolatedExecutionWorkspace(workspace))
return null;
@@ -8925,7 +8926,7 @@ export function issueRoutes(
: null;
const currentExecutionWorkspacePromise = issue.executionWorkspaceId
? executionWorkspacesSvc.getById(issue.executionWorkspaceId)
? executionWorkspacesSvc.getStoredById(issue.companyId, issue.executionWorkspaceId, { includeRuntimeServices: true })
: Promise.resolve(null);
const [
{ project, goal },
@@ -9292,7 +9293,7 @@ export function issueRoutes(
inboxArchiveFieldsPromise,
timing.time("channel", () => getExternalChannelBindingSummary(db, issue.companyId, issue.id)),
timing.time("workspace", () => issue.executionWorkspaceId
? executionWorkspacesSvc.getById(issue.executionWorkspaceId)
? executionWorkspacesSvc.getStoredById(issue.companyId, issue.executionWorkspaceId, { includeRuntimeServices: true })
: Promise.resolve(null)),
timing.time("work_products", () => workProductsSvc.listForIssue(issue.id)),
timing.time("created_from", () => resolveVisibleIssueCreationSource(req, issue)),
+5 -2
View File
@@ -2910,10 +2910,13 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
return null;
},
getStoredById: async (companyId: string, id: string) => {
getStoredById: async (companyId: string, id: string, options?: { includeRuntimeServices: boolean }) => {
const [row] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, id));
if (row && row.companyId !== companyId) throw forbidden("Task workspace access is no longer available");
return row ? toExecutionWorkspace(row) : null;
if (!row) return null;
const runtimeServices = options?.includeRuntimeServices
? await loadEffectiveRuntimeServicesByExecutionWorkspace(db, companyId, [row]) : null;
return toExecutionWorkspace(row, (runtimeServices?.get(row.id) ?? []).map(toRuntimeService));
},
getById: async (id: string) => {