Skip to content

Commit 9f29bfd

Browse files
fix(issues): Sim-only delegated body access, pinned review handoff, consistent list snapshot
1 parent 2339004 commit 9f29bfd

9 files changed

Lines changed: 70 additions & 27 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/issues/components/issue-detail.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22

33
import { useCallback, useState } from 'react'
4-
import { Chip, ChipLink, cn, pageHeadingClassName, toast } from '@sim/emcn'
4+
import { Chip, ChipLink, cn, OverflowText, pageHeadingClassName, toast } from '@sim/emcn'
55
import { ArrowUpRight, ListChecks } from '@sim/emcn/icons'
66
import { getErrorMessage } from '@sim/utils/errors'
77
import { generateId } from '@sim/utils/id'
@@ -195,7 +195,7 @@ function IssueBar({ workspaceId, issue, canEdit, visible }: IssueBarProps) {
195195
)}
196196
>
197197
<div className={cn(COLUMN_CLASS, 'flex h-[48px] items-center gap-3 text-small')}>
198-
<span className='min-w-0 flex-1 truncate text-[var(--text-primary)]'>{issue.title}</span>
198+
<OverflowText label={issue.title} className='flex-1 text-[var(--text-primary)]' />
199199
<div className='flex shrink-0 items-center gap-2'>
200200
<IssueActions
201201
workspaceId={workspaceId}

‎apps/sim/app/workspace/[workspaceId]/issues/issues.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client'
22

33
import { useState } from 'react'
4+
import { OverflowText } from '@sim/emcn'
45
import { ListChecks, Plus } from '@sim/emcn/icons'
56
import Link from 'next/link'
67
import { ShimmerText } from '@/components/ui/shimmer-text'
@@ -52,7 +53,7 @@ function noteFor(issue: IssueRecord): string | null {
5253

5354
export function Issues({ workspaceId }: IssuesProps) {
5455
const [creating, setCreating] = useState(false)
55-
const query = useIssueList(workspaceId)
56+
const query = useIssueList(workspaceId, { followProgress: true })
5657
const groups = groupIssues(query.data ?? [])
5758

5859
return (
@@ -114,9 +115,11 @@ function IssueRow({ workspaceId, issue }: IssueRowProps) {
114115
)}
115116
</span>
116117
<span className='sr-only'>{issueStatusLabel(issue)}</span>
117-
<span className='min-w-0 flex-1 truncate text-[var(--text-primary)] group-hover:underline group-hover:underline-offset-4'>
118-
{issue.title}
119-
</span>
118+
<OverflowText
119+
label={issue.title}
120+
focusTarget='nearest-interactive'
121+
className='flex-1 text-[var(--text-primary)] group-hover:underline group-hover:underline-offset-4'
122+
/>
120123
{note &&
121124
(issue.status === 'in_progress' && issue.workingChat?.running ? (
122125
<ShimmerText className='max-w-[280px] truncate text-caption'>{note}</ShimmerText>

‎apps/sim/hooks/queries/issues.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,24 @@ export const issueKeys = {
3434
detail: (workspaceId: string, key: string) => [...issueKeys.details(), workspaceId, key] as const,
3535
}
3636

37-
export function useIssueList(workspaceId: string, options?: { enabled?: boolean }) {
37+
interface UseIssueListOptions {
38+
enabled?: boolean
39+
/** Refetch while any issue is in progress, so a page left open sees Sim hand work back. */
40+
followProgress?: boolean
41+
}
42+
43+
export function useIssueList(workspaceId: string, options?: UseIssueListOptions) {
3844
return useQuery({
3945
queryKey: issueKeys.list(workspaceId),
4046
queryFn: ({ signal }) =>
4147
requestJson(listIssuesContract, { params: { id: workspaceId }, signal }),
4248
enabled: Boolean(workspaceId) && (options?.enabled ?? true),
4349
staleTime: ISSUE_LIST_STALE_TIME,
50+
refetchInterval: (query) =>
51+
options?.followProgress &&
52+
query.state.data?.issues.some((issue) => issue.status === 'in_progress')
53+
? ISSUE_RUNNING_REFETCH_INTERVAL
54+
: false,
4455
select: (data) => data.issues,
4556
})
4657
}

‎apps/sim/lib/issues/__integration__/issue-body-access.integration.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ import path from 'node:path'
66
import type { Principal } from '@sim/auth/principal'
77
import { db, dbFor } from '@sim/db'
88
import { copilotChats, issue, issueEvent, organization, user, workspace } from '@sim/db/schema'
9-
import { createSessionPrincipal, createWorkspaceApiKeyPrincipal } from '@sim/testing'
9+
import {
10+
createExecutorPrincipal,
11+
createSessionPrincipal,
12+
createWorkspaceApiKeyPrincipal,
13+
} from '@sim/testing'
1014
import { and, eq, inArray } from 'drizzle-orm'
1115
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
1216

@@ -130,6 +134,12 @@ describe('issue bodies in PostgreSQL', () => {
130134
await expect(
131135
readBody(filed.bodyFileId, createWorkspaceApiKeyPrincipal({ workspaceId: ids.workspaceId }))
132136
).rejects.toMatchObject(notFound)
137+
await expect(
138+
readBody(
139+
filed.bodyFileId,
140+
createExecutorPrincipal({ subjectUserId: ids.aliceId, workspaceId: ids.workspaceId })
141+
)
142+
).rejects.toMatchObject(notFound)
133143

134144
flags.issues = false
135145
try {

‎apps/sim/lib/issues/application/issues.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,10 @@ export const requestIssueReview = defineAuthorizedWorkspaceUseCase({
415415
)
416416
await transition(
417417
context.issue.id,
418-
{ statuses: ['in_progress'] },
418+
{
419+
statuses: ['in_progress'],
420+
...(context.issue.workingChatId ? { workingChatId: context.issue.workingChatId } : {}),
421+
},
419422
{ status: 'inbox', reviewSummary: summary },
420423
(row) => [{ kind: 'review_requested', payload: { chatId: row.workingChatId, summary } }],
421424
principal

‎apps/sim/lib/issues/repository.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,20 @@ function issueListQuery(executor: DbOrTx) {
123123
/** Every open issue, plus the most recently closed ones. */
124124
export async function listWorkspaceIssues(workspaceId: string): Promise<IssueListRow[]> {
125125
const active = and(eq(issue.workspaceId, workspaceId), isNull(issue.deletedAt))
126-
const [open, done] = await Promise.all([
127-
issueListQuery(db)
128-
.where(and(active, ne(issue.status, 'done')))
129-
.orderBy(desc(issue.updatedAt)),
130-
issueListQuery(db)
131-
.where(and(active, eq(issue.status, 'done')))
132-
.orderBy(desc(issue.completedAt))
133-
.limit(LISTED_DONE_ISSUES),
134-
])
135-
return [...open, ...done]
126+
// One snapshot for both reads, so an issue closing in between is listed exactly once.
127+
return db.transaction(
128+
async (tx) => {
129+
const open = await issueListQuery(tx)
130+
.where(and(active, ne(issue.status, 'done')))
131+
.orderBy(desc(issue.updatedAt))
132+
const done = await issueListQuery(tx)
133+
.where(and(active, eq(issue.status, 'done')))
134+
.orderBy(desc(issue.completedAt))
135+
.limit(LISTED_DONE_ISSUES)
136+
return [...open, ...done]
137+
},
138+
{ isolationLevel: 'repeatable read', accessMode: 'read only' }
139+
)
136140
}
137141

138142
export async function getIssueListRow(issueId: string): Promise<IssueListRow | null> {
@@ -144,6 +148,8 @@ export interface IssueStateGuard {
144148
statuses: readonly IssueStatus[]
145149
/** Inbox with a working chat is waiting for review; without one it is new. */
146150
hasWorkingChat?: boolean
151+
/** The exact working chat the caller read, so a replaced chat's late request changes nothing. */
152+
workingChatId?: string
147153
/** Fields that must still hold the values the caller read, so an edit never logs a stale "from". */
148154
unchanged?: { title?: string; priority?: number; ownerId?: string | null }
149155
}
@@ -172,6 +178,9 @@ export async function updateIssueInTx(
172178
eq(issue.id, issueId),
173179
inArray(issue.status, [...guard.statuses]),
174180
chatCondition,
181+
guard.workingChatId === undefined
182+
? undefined
183+
: eq(issue.workingChatId, guard.workingChatId),
175184
guard.unchanged?.title === undefined ? undefined : eq(issue.title, guard.unchanged.title),
176185
guard.unchanged?.priority === undefined
177186
? undefined

‎apps/sim/lib/workspace-files/application/read-workspace-file-artifact.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/appl
88
import { fileOperations } from '@/lib/workspace-files/application/operations'
99
import { resolveRenderedWorkspaceArtifact } from '@/lib/workspace-files/application/resolve-rendered-workspace-artifact'
1010
import { resolveReferencedWorkspaceFileContext } from '@/lib/workspace-files/application/resolve-workspace-file-reference'
11+
import { secretProvenanceContextOf } from '@/lib/workspace-files/secret-provenance-context'
1112

1213
interface ReadWorkspaceFileArtifactInput {
1314
workspaceId: string
@@ -35,7 +36,7 @@ export const readWorkspaceFileArtifact = defineAuthorizedWorkspaceFileUseCase({
3536
const safe = await isOpaqueWorkspaceFileEgressSafe(context.workspaceId, {
3637
fileId: file.id,
3738
key: file.key,
38-
context: file.storageContext ?? 'workspace',
39+
context: secretProvenanceContextOf(file),
3940
...(file.contentUpdatedAt ? { contentUpdatedAt: file.contentUpdatedAt } : {}),
4041
})
4142
if (!safe) throw new OrchestrationError('forbidden', MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE)

‎apps/sim/lib/workspace-files/application/read-workspace-file-text.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
type ReferencedWorkspaceFileContext,
3131
resolveReferencedWorkspaceFileContext,
3232
} from '@/lib/workspace-files/application/resolve-workspace-file-reference'
33+
import { secretProvenanceContextOf } from '@/lib/workspace-files/secret-provenance-context'
3334
import { parseWorkspaceFileText } from '@/lib/workspace-files/text-extraction'
3435
import { workspaceFileTextFormat } from '@/lib/workspace-files/text-format'
3536
import { sliceFileTextLines } from '@/lib/workspace-files/text-lines'
@@ -137,7 +138,7 @@ export async function extractWorkspaceFileRecordText(
137138
? await getBoundWorkspaceFileSecretProvenance(file.workspaceId, {
138139
fileId: file.id,
139140
key: file.key,
140-
context: file.storageContext ?? 'workspace',
141+
context: secretProvenanceContextOf(file),
141142
contentUpdatedAt: file.contentUpdatedAt ?? undefined,
142143
})
143144
: undefined)

‎apps/sim/lib/workspace-files/application/workspace-file-context.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,29 @@ export interface WorkspaceFileContextInput {
2525
issueBodyPrincipal?: Principal
2626
}
2727

28-
/** Principals the issue operations admit; workspace API keys and system callers never reach a body. */
28+
/**
29+
* Principals the issue operations admit. A delegated caller must be Sim itself: a workflow's File
30+
* tool delegates as the executor, and the issue operations refuse it.
31+
*/
2932
const ISSUE_BODY_PRINCIPAL_KINDS = new Set<Principal['kind']>([
3033
'session',
3134
'personal_api_key',
3235
'oauth_access_token',
33-
'delegated',
3436
])
3537

38+
function isIssueBodyPrincipal(principal: Principal): boolean {
39+
return principal.kind === 'delegated'
40+
? principal.serviceId === 'copilot'
41+
: ISSUE_BODY_PRINCIPAL_KINDS.has(principal.kind)
42+
}
43+
3644
/** An issue body follows its live issue: the rollout flag, and the issue operations' principal kinds. */
3745
export async function assertIssueBodyAccess(
3846
principal: Principal,
3947
context: ActiveWorkspaceFileContext
4048
): Promise<void> {
4149
if (context.fileContext !== 'issue') return
42-
if (
43-
!ISSUE_BODY_PRINCIPAL_KINDS.has(principal.kind) ||
44-
!(await hasLiveIssueForBody(context.fileId))
45-
) {
50+
if (!isIssueBodyPrincipal(principal) || !(await hasLiveIssueForBody(context.fileId))) {
4651
throw new OrchestrationError('not_found', 'File not found')
4752
}
4853
await requireIssuesEnabled(context.workspaceOrganizationId)

0 commit comments

Comments
 (0)