mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
Use artifact titles for created workspaces (#1643)
This commit is contained in:
@@ -2,6 +2,7 @@ import { join, resolve } from 'path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
sanitizeWorktreeName,
|
||||
sanitizeWorktreeDisplayName,
|
||||
ensurePathWithinWorkspace,
|
||||
computeBranchName,
|
||||
computeWorktreePath,
|
||||
@@ -74,6 +75,27 @@ describe('sanitizeWorktreeName', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('sanitizeWorktreeDisplayName', () => {
|
||||
it('keeps readable punctuation while collapsing unsafe controls and whitespace', () => {
|
||||
expect(sanitizeWorktreeDisplayName(' Fix: login / callback\n\tregression\u0000 ')).toBe(
|
||||
'Fix: login / callback regression'
|
||||
)
|
||||
})
|
||||
|
||||
it('strips bidi override controls from external artifact titles', () => {
|
||||
expect(sanitizeWorktreeDisplayName('Review \u202eexe.txt')).toBe('Review exe.txt')
|
||||
})
|
||||
|
||||
it('truncates very long titles', () => {
|
||||
const title = 'a'.repeat(200)
|
||||
expect(sanitizeWorktreeDisplayName(title)).toHaveLength(120)
|
||||
})
|
||||
|
||||
it('returns undefined when nothing displayable remains', () => {
|
||||
expect(sanitizeWorktreeDisplayName('\u0000\n\t')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensurePathWithinWorkspace', () => {
|
||||
it('returns resolved path when within workspace', () => {
|
||||
const result = ensurePathWithinWorkspace('/workspace/feature', '/workspace')
|
||||
|
||||
@@ -30,6 +30,23 @@ export function sanitizeWorktreeName(input: string): string {
|
||||
return sanitized
|
||||
}
|
||||
|
||||
export function sanitizeWorktreeDisplayName(input: string): string | undefined {
|
||||
const withoutControls = Array.from(input, (char) => {
|
||||
const code = char.charCodeAt(0)
|
||||
return code <= 0x1f || (code >= 0x7f && code <= 0x9f) ? ' ' : char
|
||||
}).join('')
|
||||
const sanitized = withoutControls
|
||||
// Why: titles come from external systems. Strip bidi override controls so a
|
||||
// malicious title cannot visually reorder adjacent sidebar text.
|
||||
.replace(/[\u202a-\u202e\u2066-\u2069]/g, '')
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim()
|
||||
.slice(0, 120)
|
||||
.trim()
|
||||
|
||||
return sanitized || undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure a target path is within the workspace directory (prevent path traversal).
|
||||
*/
|
||||
|
||||
@@ -30,6 +30,7 @@ import { getActiveMultiplexer } from './ssh'
|
||||
import type { SshGitProvider } from '../providers/ssh-git-provider'
|
||||
import {
|
||||
sanitizeWorktreeName,
|
||||
sanitizeWorktreeDisplayName,
|
||||
computeBranchName,
|
||||
computeWorktreePath,
|
||||
ensurePathWithinWorkspace,
|
||||
@@ -86,6 +87,9 @@ export async function createRemoteWorktree(
|
||||
const settings = store.getSettings()
|
||||
const requestedName = args.name
|
||||
const sanitizedName = sanitizeWorktreeName(args.name)
|
||||
const requestedDisplayName = args.displayName
|
||||
? sanitizeWorktreeDisplayName(args.displayName)
|
||||
: undefined
|
||||
|
||||
// Get git username from remote
|
||||
let username = ''
|
||||
@@ -222,9 +226,11 @@ export async function createRemoteWorktree(
|
||||
// max(lastActivityAt, createdAt + GRACE_MS) to keep it on top until the
|
||||
// window elapses. See smart-sort.ts `CREATE_GRACE_MS`.
|
||||
createdAt: now,
|
||||
...(shouldSetDisplayName(requestedName, branchName, sanitizedName)
|
||||
? { displayName: requestedName }
|
||||
: {})
|
||||
...(requestedDisplayName
|
||||
? { displayName: requestedDisplayName }
|
||||
: shouldSetDisplayName(requestedName, branchName, sanitizedName)
|
||||
? { displayName: requestedName }
|
||||
: {})
|
||||
}
|
||||
const meta = store.setWorktreeMeta(worktreeId, metaUpdates)
|
||||
const worktree = mergeWorktree(repo.id, created, meta)
|
||||
@@ -251,6 +257,9 @@ export async function createLocalWorktree(
|
||||
const username = getGitUsername(repo.path)
|
||||
const requestedName = args.name
|
||||
const sanitizedName = sanitizeWorktreeName(args.name)
|
||||
const requestedDisplayName = args.displayName
|
||||
? sanitizeWorktreeDisplayName(args.displayName)
|
||||
: undefined
|
||||
|
||||
// Why (§3.3): determine the base branch (and therefore the remote we need to
|
||||
// fetch) FIRST, so the fetch can overlap all pre-create work below. Neither
|
||||
@@ -476,9 +485,11 @@ export async function createLocalWorktree(
|
||||
// worktree from ambient PTY bumps in other worktrees for CREATE_GRACE_MS.
|
||||
createdAt: now,
|
||||
baseRef: baseBranch,
|
||||
...(shouldSetDisplayName(effectiveRequestedName, branchName, effectiveSanitizedName)
|
||||
? { displayName: effectiveRequestedName }
|
||||
: {}),
|
||||
...(requestedDisplayName
|
||||
? { displayName: requestedDisplayName }
|
||||
: shouldSetDisplayName(effectiveRequestedName, branchName, effectiveSanitizedName)
|
||||
? { displayName: effectiveRequestedName }
|
||||
: {}),
|
||||
...(sparseDirectories.length > 0
|
||||
? {
|
||||
sparseDirectories,
|
||||
|
||||
@@ -313,6 +313,37 @@ describe('registerWorktreeHandlers', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('persists a sanitized artifact title as the worktree display name', async () => {
|
||||
listWorktreesMock.mockResolvedValue([
|
||||
{
|
||||
path: '/workspace/improve-dashboard',
|
||||
head: 'abc123',
|
||||
branch: 'improve-dashboard',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta)
|
||||
|
||||
const result = await handlers['worktrees:create'](null, {
|
||||
repoId: 'repo-1',
|
||||
name: 'improve-dashboard',
|
||||
displayName: ' Fix: dashboards\nfor PRs\u0000 '
|
||||
})
|
||||
|
||||
expect(store.setWorktreeMeta).toHaveBeenCalledWith(
|
||||
'repo-1::/workspace/improve-dashboard',
|
||||
expect.objectContaining({
|
||||
displayName: 'Fix: dashboards for PRs'
|
||||
})
|
||||
)
|
||||
expect(result).toEqual({
|
||||
worktree: expect.objectContaining({
|
||||
displayName: 'Fix: dashboards for PRs'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('does not await a cold fetch when the remote-tracking base exists locally', async () => {
|
||||
const remoteBase = {
|
||||
remote: 'origin',
|
||||
|
||||
@@ -1296,7 +1296,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
||||
...(effectivePresetId ? { presetId: effectivePresetId } : {})
|
||||
}
|
||||
: undefined,
|
||||
telemetrySource
|
||||
telemetrySource,
|
||||
linkedWorkItem?.title
|
||||
)
|
||||
const worktree = result.worktree
|
||||
|
||||
@@ -1376,6 +1377,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
||||
applyWorktreeMeta,
|
||||
issueCommandTemplate,
|
||||
effectiveLinkedPR,
|
||||
linkedWorkItem?.title,
|
||||
linkedWorkItem?.url,
|
||||
normalizedSparseDirectories,
|
||||
note,
|
||||
@@ -1444,7 +1446,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
||||
...(effectivePresetId ? { presetId: effectivePresetId } : {})
|
||||
}
|
||||
: undefined,
|
||||
telemetrySource
|
||||
telemetrySource,
|
||||
linkedWorkItem?.title
|
||||
)
|
||||
const worktree = result.worktree
|
||||
|
||||
|
||||
@@ -219,7 +219,8 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom
|
||||
baseBranch,
|
||||
finalSetupDecision,
|
||||
undefined,
|
||||
telemetrySource
|
||||
telemetrySource,
|
||||
item.title
|
||||
)
|
||||
worktreeId = result.worktree.id
|
||||
const worktreePath = result.worktree.path
|
||||
|
||||
@@ -68,7 +68,8 @@ export type WorktreeSlice = {
|
||||
/** Telemetry-only: which renderer surface initiated this create. Optional
|
||||
* so existing callers default to `unknown`; specify when the surface
|
||||
* matters for the activation funnel. */
|
||||
telemetrySource?: WorkspaceCreateTelemetrySource
|
||||
telemetrySource?: WorkspaceCreateTelemetrySource,
|
||||
displayName?: string
|
||||
) => Promise<CreateWorktreeResult>
|
||||
removeWorktree: (
|
||||
worktreeId: string,
|
||||
|
||||
@@ -222,7 +222,8 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
baseBranch,
|
||||
setupDecision = 'inherit',
|
||||
sparseCheckout,
|
||||
telemetrySource
|
||||
telemetrySource,
|
||||
displayName
|
||||
) => {
|
||||
const retryableConflictPatterns = [
|
||||
/already exists locally/i,
|
||||
@@ -242,6 +243,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
baseBranch,
|
||||
setupDecision,
|
||||
sparseCheckout,
|
||||
...(displayName ? { displayName } : {}),
|
||||
...(telemetrySource ? { telemetrySource } : {})
|
||||
})
|
||||
// Why: a file watcher (worktrees.onChanged) can fire between the
|
||||
|
||||
@@ -845,6 +845,10 @@ export type SparsePreset = {
|
||||
export type CreateWorktreeArgs = {
|
||||
repoId: string
|
||||
name: string
|
||||
/** Optional user-facing label to persist separately from the git-safe
|
||||
* branch/path seed. Used when a workspace is created from a GitHub or
|
||||
* Linear artifact whose title should remain readable in the sidebar. */
|
||||
displayName?: string
|
||||
baseBranch?: string
|
||||
setupDecision?: SetupDecision
|
||||
sparseCheckout?: CreateSparseCheckoutRequest
|
||||
|
||||
Reference in New Issue
Block a user