fix(naming): remove identifier-first name post-processing (#9821)
* fix(naming): remove identifier-first name post-processing Workspace display names and tab titles were being rewritten after generation by prompt-scanning heuristics from #8238 — a stray "#1" in prose became a workspace named "#1 - Fix", and the rewrite ran downstream of generation so user naming instructions couldn't override it. Per the same principle as #9088, naming defaults stay minimal and user overrides own the style. - Delete work-item-reference.ts and display-name-from-work.ts (+tests). - Auto-rename display names return to the humanized branch slug; tab titles return to the cleaned first prompt clause. - Explicit create-from-work-item naming returns to action-first ("Review PR 1234"). - Keep #8238's URL-before-markdown strip-order bugfix in tab titles, with regression tests adjusted to the natural expectations. * test(naming): pin incidental marker regression
This commit is contained in:
Brennan Benson
GitHub
parent
b81e782ffc
commit
9097ea0409
|
|
@ -82,9 +82,9 @@ describe('maybeAutoRenameBranchOnFirstWork', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('renames a fresh creature branch and its display name from the generated slug', async () => {
|
||||
it('keeps incidental work-item markers from overriding the generated display name', async () => {
|
||||
const { deps, onRenamed, setDisplayName } = makeDeps()
|
||||
await maybeAutoRenameBranchOnFirstWork(workingEvent(), deps)
|
||||
await maybeAutoRenameBranchOnFirstWork(workingEvent({ prompt: 'Fix auth from note #1' }), deps)
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
||||
['branch', '-m', 'you/fix-auth'],
|
||||
expect.objectContaining({ cwd: '/repo/wt' })
|
||||
|
|
@ -219,7 +219,7 @@ describe('maybeAutoRenameBranchOnFirstWork', () => {
|
|||
getCurrentDisplayName: () => 'Platform workspace'
|
||||
})
|
||||
|
||||
await maybeAutoRenameBranchOnFirstWork(workingEvent(), deps)
|
||||
await maybeAutoRenameBranchOnFirstWork(workingEvent({ prompt: 'Fix auth from note #1' }), deps)
|
||||
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
expect(resolveTextGenerationParamsMock).toHaveBeenCalledWith(
|
||||
|
|
|
|||
|
|
@ -4,10 +4,10 @@ import { getRepoIdFromWorktreeId, splitWorktreeIdForFilesystem } from '../../sha
|
|||
import { parseWorkspaceKey } from '../../shared/workspace-scope'
|
||||
import { parsePaneKey } from '../../shared/stable-pane-id'
|
||||
import {
|
||||
humanizeBranchSlug,
|
||||
isAutoGeneratedCreatureBranchName,
|
||||
stripConfiguredBranchPrefix
|
||||
} from '../../shared/branch-name-from-work'
|
||||
import { deriveWorkspaceDisplayName } from '../../shared/display-name-from-work'
|
||||
import { getCommitMessageModelDiscoveryHostKey } from '../../shared/commit-message-host-key'
|
||||
import { computeBranchName, getConfiguredBranchPrefix } from '../ipc/worktree-logic'
|
||||
import { gitExecFileAsync } from '../git/runner'
|
||||
|
|
@ -280,7 +280,7 @@ async function runAutoRename(
|
|||
const newBranchLeaf = newBranch.slice(newBranch.lastIndexOf('/') + 1)
|
||||
// Only sync the sidebar name when it's still the auto-generated creature name, so a user-typed name is left alone.
|
||||
const currentDisplayName = deps.getCurrentDisplayName(worktreeId)
|
||||
const newDisplayName = deriveWorkspaceDisplayName({ prompt, slug, resolvedLeaf: newBranchLeaf })
|
||||
const newDisplayName = humanizeBranchSlug(newBranchLeaf)
|
||||
const updateDisplay = !currentDisplayName || isAutoGeneratedCreatureBranchName(currentDisplayName)
|
||||
if (updateDisplay) {
|
||||
deps.setDisplayName(worktreeId, newDisplayName)
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { deriveWorkspaceDisplayName } from '../../shared/display-name-from-work'
|
||||
import { humanizeBranchSlug } from '../../shared/branch-name-from-work'
|
||||
import {
|
||||
generateBranchNameFromContext,
|
||||
resolveTextGenerationParams
|
||||
|
|
@ -56,7 +56,7 @@ export async function runFolderWorkspaceTitleAutoRename(
|
|||
return retry(`generation failed: ${generated.error}`)
|
||||
}
|
||||
|
||||
const newDisplayName = deriveWorkspaceDisplayName({ prompt, slug: generated.slug })
|
||||
const newDisplayName = humanizeBranchSlug(generated.slug)
|
||||
deps.setDisplayName(worktreeId, newDisplayName)
|
||||
deps.setRenameError(worktreeId, null)
|
||||
deps.onRenamed(worktreeId)
|
||||
|
|
|
|||
|
|
@ -77,11 +77,11 @@ vi.mock('@/lib/new-workspace', () => ({
|
|||
? {
|
||||
displayName:
|
||||
args.workItem.type === 'pr'
|
||||
? `PR ${args.workItem.number} - Review`
|
||||
? `Review PR ${args.workItem.number}`
|
||||
: `Issue ${args.workItem.number}`,
|
||||
seedName:
|
||||
args.workItem.type === 'pr'
|
||||
? `pr-${args.workItem.number}-review`
|
||||
? `review-pr-${args.workItem.number}`
|
||||
: `issue-${args.workItem.number}`
|
||||
}
|
||||
: null,
|
||||
|
|
@ -253,12 +253,12 @@ describe('launchWorkItemDirect', () => {
|
|||
})
|
||||
expect(mocks.createWorktree).toHaveBeenCalledWith(
|
||||
'repo-1',
|
||||
'pr-6934-review',
|
||||
'review-pr-6934',
|
||||
'abc123',
|
||||
'inherit',
|
||||
undefined,
|
||||
'sidebar',
|
||||
'PR 6934 - Review',
|
||||
'Review PR 6934',
|
||||
undefined,
|
||||
6934,
|
||||
{ remoteName: 'origin', branchName: 'feature/fix' },
|
||||
|
|
|
|||
|
|
@ -16,10 +16,12 @@ describe('deriveGeneratedTabTitle', () => {
|
|||
).toBe('Refactor the auth middleware to use JWT')
|
||||
})
|
||||
|
||||
it('strips markup, links, emoji, and punctuation from generated titles', () => {
|
||||
it('strips markup, links, emoji, and punctuation without promoting incidental markers', () => {
|
||||
expect(
|
||||
deriveGeneratedTabTitle('Please fix `src/auth.ts`!!! https://example.com 🔥 then add tests')
|
||||
).toBe('Fix src auth')
|
||||
deriveGeneratedTabTitle(
|
||||
'Please fix auth note #1 with `src/auth.ts`!!! https://example.com 🔥'
|
||||
)
|
||||
).toBe('Fix auth note 1 with src auth')
|
||||
})
|
||||
|
||||
it('preserves non-ASCII title text while folding Unicode whitespace', () => {
|
||||
|
|
@ -28,50 +30,24 @@ describe('deriveGeneratedTabTitle', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('leads with the issue identifier and keeps the description after it', () => {
|
||||
it('keeps useful text after common issue prefixes', () => {
|
||||
expect(deriveGeneratedTabTitle('Issue #2056: Opt-in generated tab titles for agents')).toBe(
|
||||
'Issue 2056 - Opt in generated tab'
|
||||
'Opt in generated tab titles for agents'
|
||||
)
|
||||
})
|
||||
|
||||
it('recovers a PR number from a URL the pipeline strips, leading with it', () => {
|
||||
expect(
|
||||
deriveGeneratedTabTitle(
|
||||
'Review this community PR https://github.com/EveryInc/plugin/pull/1094'
|
||||
)
|
||||
).toBe('PR 1094 - Review this community')
|
||||
})
|
||||
|
||||
it('leads with a GitLab MR identifier', () => {
|
||||
expect(
|
||||
deriveGeneratedTabTitle('fix https://gitlab.com/group/app/-/merge_requests/42 quickly')
|
||||
).toBe('MR 42 - Fix quickly')
|
||||
})
|
||||
|
||||
it('strips a URL containing underscores intact', () => {
|
||||
expect(
|
||||
deriveGeneratedTabTitle('inspect https://gitlab.com/g/p/-/work_items/9 then report')
|
||||
).toBe('Issue 9 - Inspect then report')
|
||||
).toBe('Inspect then report')
|
||||
})
|
||||
|
||||
it('leads with the identifier for a URL wrapped in markdown emphasis, without leaking fragments', () => {
|
||||
it('strips a URL wrapped in markdown emphasis without leaking fragments', () => {
|
||||
const title = deriveGeneratedTabTitle('Review _https://github.com/o/r/pull/5_ now')
|
||||
expect(title).toBe('PR 5 - Review now')
|
||||
expect(title).toBe('Review now')
|
||||
expect(title).not.toMatch(/https|pull/)
|
||||
})
|
||||
|
||||
it('leads with a bare ticket key', () => {
|
||||
expect(deriveGeneratedTabTitle('implement ENG-456 login flow')).toBe(
|
||||
'ENG-456 - Implement login flow'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not treat a cipher token as an identifier', () => {
|
||||
expect(deriveGeneratedTabTitle('implement SHA-256 hashing in the signer')).toBe(
|
||||
'Implement SHA 256 hashing in the signer'
|
||||
)
|
||||
})
|
||||
|
||||
it('bounds titles to the maximum length without adding punctuation', () => {
|
||||
const title = deriveGeneratedTabTitle(
|
||||
'I want to replace the terminal reconnection hydration flow with a safer retry path'
|
||||
|
|
|
|||
|
|
@ -1,9 +1,3 @@
|
|||
import {
|
||||
extractWorkIdentifier,
|
||||
formatIdentifierFirst,
|
||||
stripWorkIdentifierEcho
|
||||
} from './work-item-reference'
|
||||
|
||||
export const GENERATED_TAB_TITLE_MAX_LENGTH = 40
|
||||
export const GENERATED_TAB_TITLE_SOURCE_SCAN_LIMIT = 512
|
||||
|
||||
|
|
@ -109,18 +103,6 @@ export function deriveGeneratedTabTitle(prompt: string): string | null {
|
|||
|
||||
candidate = foldGeneratedTabTitleWhitespace(candidate.replace(/[^\p{L}\p{N}\s]/gu, ' '))
|
||||
|
||||
// Lead with the review target (`PR 1094 - …`) so the tab matches the sidebar
|
||||
// workspace name; the pipeline above strips the URL/prefix, so the number is
|
||||
// recovered from the raw prompt rather than the cleaned clause.
|
||||
const identifier = extractWorkIdentifier(promptPreview)
|
||||
if (identifier) {
|
||||
const detail = capitalizeFirstLetter(stripWorkIdentifierEcho(candidate, identifier))
|
||||
return truncateAtWordBoundary(
|
||||
formatIdentifierFirst(identifier.label, detail),
|
||||
GENERATED_TAB_TITLE_MAX_LENGTH
|
||||
)
|
||||
}
|
||||
|
||||
if (!candidate) {
|
||||
return null
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,60 +0,0 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { deriveWorkspaceDisplayName } from './display-name-from-work'
|
||||
|
||||
// Identifier extraction itself is covered in work-item-reference.test.ts; these
|
||||
// cases exercise the end-to-end display-name composition.
|
||||
describe('deriveWorkspaceDisplayName', () => {
|
||||
it('leads with the identifier and a single action verb', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({
|
||||
prompt: 'Carefully evaluate https://github.com/o/r/pull/1033. Fix the merge conflict.',
|
||||
slug: 'review-community-pr-conflict'
|
||||
})
|
||||
).toBe('PR 1033 - Review')
|
||||
})
|
||||
|
||||
it('drops identifier tokens the slug also carried', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({
|
||||
prompt: 'look at this community PR https://github.com/o/r/pull/1094',
|
||||
slug: 'review-community-pr-1094'
|
||||
})
|
||||
).toBe('PR 1094 - Review')
|
||||
})
|
||||
|
||||
it('uses a namespaced ticket id bare, without a type prefix', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({ prompt: 'fix ENG-456 crash', slug: 'fix-eng-456-crash' })
|
||||
).toBe('ENG-456 - Fix')
|
||||
})
|
||||
|
||||
it('returns the identifier alone when no action word survives', () => {
|
||||
expect(deriveWorkspaceDisplayName({ prompt: 'PR 12', slug: 'pr-12' })).toBe('PR 12')
|
||||
})
|
||||
|
||||
it('carries a collision suffix so same-target worktrees stay distinct', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({
|
||||
prompt: 'review https://github.com/o/r/pull/1033',
|
||||
slug: 'review-conflict',
|
||||
resolvedLeaf: 'review-conflict-2'
|
||||
})
|
||||
).toBe('PR 1033 - Review (2)')
|
||||
})
|
||||
|
||||
it('falls back to the humanized leaf when no identifier is present', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({ prompt: 'add a dark mode toggle', slug: 'add-dark-mode-toggle' })
|
||||
).toBe('Add dark mode toggle')
|
||||
})
|
||||
|
||||
it('humanizes the resolved leaf (with suffix) on the fallback path', () => {
|
||||
expect(
|
||||
deriveWorkspaceDisplayName({
|
||||
prompt: 'add a logout button',
|
||||
slug: 'add-logout-button',
|
||||
resolvedLeaf: 'add-logout-button-2'
|
||||
})
|
||||
).toBe('Add logout button 2')
|
||||
})
|
||||
})
|
||||
|
|
@ -1,76 +0,0 @@
|
|||
import { humanizeBranchSlug } from './branch-name-from-work'
|
||||
import { extractWorkIdentifier, formatIdentifierFirst } from './work-item-reference'
|
||||
|
||||
// Why: the git branch must stay lowercase kebab-case, but the sidebar display
|
||||
// name is free-form. When the first prompt references a review target, that
|
||||
// identifier is the highest-signal, most searchable token — so we lead with it
|
||||
// (`PR 1033 - Review`) instead of a paraphrased slug like "Review community pr
|
||||
// 1033". The identifier comes from the raw prompt (via work-item-reference),
|
||||
// where the number still exists, not from the lossy branch slug.
|
||||
|
||||
// Type words that are never a useful action verb, skipped even if a slug leads
|
||||
// with one (e.g. `pr-1094-review` → `Review`, not `Pr`).
|
||||
const ACTION_STOPWORDS = new Set([
|
||||
'pr',
|
||||
'mr',
|
||||
'pull',
|
||||
'merge',
|
||||
'request',
|
||||
'requests',
|
||||
'issue',
|
||||
'issues',
|
||||
'the',
|
||||
'a',
|
||||
'an',
|
||||
'to',
|
||||
'for',
|
||||
'of'
|
||||
])
|
||||
|
||||
// The first slug word that is neither a digit nor an identifier/type token,
|
||||
// capitalized — the task verb ("Review", "Fix"). '' when none remains.
|
||||
function leadingActionWord(slug: string, identifierTokens: string[]): string {
|
||||
const skip = new Set([...identifierTokens, ...ACTION_STOPWORDS])
|
||||
for (const word of slug.split('-').filter(Boolean)) {
|
||||
if (/^\d+$/.test(word) || skip.has(word)) {
|
||||
continue
|
||||
}
|
||||
return word.charAt(0).toUpperCase() + word.slice(1)
|
||||
}
|
||||
return ''
|
||||
}
|
||||
|
||||
// A `-N` collision suffix (`fix-auth` → `fix-auth-2`) that resolveUniqueBranchName
|
||||
// appended, so two worktrees on the same identifier stay distinguishable.
|
||||
function collisionSuffixFromLeaf(
|
||||
baseSlug: string,
|
||||
resolvedLeaf: string | undefined
|
||||
): number | null {
|
||||
if (!resolvedLeaf || resolvedLeaf === baseSlug || !resolvedLeaf.startsWith(`${baseSlug}-`)) {
|
||||
return null
|
||||
}
|
||||
const rest = resolvedLeaf.slice(baseSlug.length + 1)
|
||||
return /^\d+$/.test(rest) ? Number(rest) : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the sidebar display name for a freshly auto-renamed workspace. When the
|
||||
* prompt names a review target, returns `<identifier> - <action>` (e.g.
|
||||
* `PR 1033 - Review`); otherwise falls back to the plain humanized branch slug.
|
||||
* `resolvedLeaf` is the final branch leaf (after any collision suffix); omit it
|
||||
* for folder workspaces that have no git branch.
|
||||
*/
|
||||
export function deriveWorkspaceDisplayName(input: {
|
||||
prompt: string
|
||||
slug: string
|
||||
resolvedLeaf?: string
|
||||
}): string {
|
||||
const identifier = extractWorkIdentifier(input.prompt)
|
||||
if (!identifier) {
|
||||
return humanizeBranchSlug(input.resolvedLeaf ?? input.slug)
|
||||
}
|
||||
const action = leadingActionWord(input.slug, identifier.tokens)
|
||||
const base = formatIdentifierFirst(identifier.label, action)
|
||||
const suffix = collisionSuffixFromLeaf(input.slug, input.resolvedLeaf)
|
||||
return suffix ? `${base} (${suffix})` : base
|
||||
}
|
||||
|
|
@ -1,127 +0,0 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { extractWorkIdentifier, stripWorkIdentifierEcho } from './work-item-reference'
|
||||
|
||||
describe('extractWorkIdentifier', () => {
|
||||
it('reads a GitHub pull request URL', () => {
|
||||
expect(extractWorkIdentifier('Review https://github.com/EveryInc/plugin/pull/1033')).toEqual({
|
||||
label: 'PR 1033',
|
||||
tokens: ['pr', '1033']
|
||||
})
|
||||
})
|
||||
|
||||
it('reads a Bitbucket Cloud pull-requests URL', () => {
|
||||
expect(
|
||||
extractWorkIdentifier('Look at https://bitbucket.org/team/repo/pull-requests/77')?.label
|
||||
).toBe('PR 77')
|
||||
})
|
||||
|
||||
it('reads a Bitbucket Server pull-requests URL', () => {
|
||||
expect(
|
||||
extractWorkIdentifier(
|
||||
'Review https://bitbucket.example.com/projects/ENG/repos/orca/pull-requests/1288'
|
||||
)
|
||||
).toEqual({ label: 'PR 1288', tokens: ['pr', '1288'] })
|
||||
// Personal (fork) repos live under /users instead of /projects.
|
||||
expect(
|
||||
extractWorkIdentifier(
|
||||
'see https://bitbucket.example.com/users/jane/repos/orca/pull-requests/9/overview'
|
||||
)?.label
|
||||
).toBe('PR 9')
|
||||
})
|
||||
|
||||
it('reads Azure DevOps pull request URLs (dev.azure.com and visualstudio.com)', () => {
|
||||
expect(
|
||||
extractWorkIdentifier('Look at https://dev.azure.com/contoso/Orca/_git/orca/pullrequest/4521')
|
||||
).toEqual({ label: 'PR 4521', tokens: ['pr', '4521'] })
|
||||
expect(
|
||||
extractWorkIdentifier(
|
||||
'https://contoso.visualstudio.com/Orca/_git/orca/pullrequest/4521?_a=files'
|
||||
)?.label
|
||||
).toBe('PR 4521')
|
||||
})
|
||||
|
||||
it('reads a GitLab merge request URL as MR, and a work_items URL as an issue', () => {
|
||||
expect(extractWorkIdentifier('Check https://gitlab.com/group/app/-/merge_requests/42')).toEqual(
|
||||
{ label: 'MR 42', tokens: ['mr', '42'] }
|
||||
)
|
||||
expect(extractWorkIdentifier('https://gitlab.example.com/g/p/-/work_items/9')?.label).toBe(
|
||||
'Issue 9'
|
||||
)
|
||||
})
|
||||
|
||||
it('reads an issue URL', () => {
|
||||
expect(extractWorkIdentifier('Fix https://github.com/o/r/issues/88')?.label).toBe('Issue 88')
|
||||
})
|
||||
|
||||
it('ignores URLs whose path only resembles a work item (no owner/repo, wrong host shape)', () => {
|
||||
// A CDN asset path contains `/pull/2023` but is not a pull request.
|
||||
expect(
|
||||
extractWorkIdentifier('Load https://cdn.vendor.com/assets/pull/2023/data.json')
|
||||
).toBeNull()
|
||||
// No trailing number after the item segment.
|
||||
expect(extractWorkIdentifier('see https://github.com/o/r/pull/notanumber')).toBeNull()
|
||||
})
|
||||
|
||||
it('tolerates trailing punctuation around a URL', () => {
|
||||
expect(extractWorkIdentifier('(see https://github.com/o/r/pull/5).')?.label).toBe('PR 5')
|
||||
})
|
||||
|
||||
it('reads a URL wrapped in markdown emphasis (trailing underscore/asterisk)', () => {
|
||||
expect(extractWorkIdentifier('Review _https://github.com/o/r/pull/5_ now')?.label).toBe('PR 5')
|
||||
expect(extractWorkIdentifier('Review **https://github.com/o/r/pull/1094**')?.label).toBe(
|
||||
'PR 1094'
|
||||
)
|
||||
})
|
||||
|
||||
it('reads textual references', () => {
|
||||
expect(extractWorkIdentifier('please review PR #1094')?.label).toBe('PR 1094')
|
||||
expect(extractWorkIdentifier('triage pull request 500')?.label).toBe('PR 500')
|
||||
expect(extractWorkIdentifier('reproduce issue 12')?.label).toBe('Issue 12')
|
||||
expect(extractWorkIdentifier('handle merge request !9')?.label).toBe('MR 9')
|
||||
})
|
||||
|
||||
it('reads a namespaced ticket id bare', () => {
|
||||
expect(extractWorkIdentifier('implement ENG-456 login flow')).toEqual({
|
||||
label: 'ENG-456',
|
||||
tokens: ['eng', '456']
|
||||
})
|
||||
})
|
||||
|
||||
it('does not treat standards, ciphers, or encodings as tickets', () => {
|
||||
expect(extractWorkIdentifier('implement SHA-256 hashing')).toBeNull()
|
||||
expect(extractWorkIdentifier('parse UTF-8 input')).toBeNull()
|
||||
expect(extractWorkIdentifier('handle ISO-8601 dates')).toBeNull()
|
||||
})
|
||||
|
||||
it('skips a denylisted prefix but still finds a real key after it', () => {
|
||||
expect(extractWorkIdentifier('encrypt with AES-256 for ticket ENG-99')?.label).toBe('ENG-99')
|
||||
})
|
||||
|
||||
it('prefers a provider URL over an incidental ticket-shaped token', () => {
|
||||
expect(
|
||||
extractWorkIdentifier('per RFC-2616 notes, review https://github.com/o/r/pull/7')?.label
|
||||
).toBe('PR 7')
|
||||
})
|
||||
|
||||
it('falls back to a bare number, then to null', () => {
|
||||
expect(extractWorkIdentifier('look at #321 when free')?.label).toBe('#321')
|
||||
expect(extractWorkIdentifier('add a dark mode toggle to settings')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('stripWorkIdentifierEcho', () => {
|
||||
it('removes the identifier tokens from a description', () => {
|
||||
expect(
|
||||
stripWorkIdentifierEcho('Review this community PR', {
|
||||
label: 'PR 1094',
|
||||
tokens: ['pr', '1094']
|
||||
})
|
||||
).toBe('Review this community')
|
||||
})
|
||||
|
||||
it('removes a ticket key echoed in the description', () => {
|
||||
expect(
|
||||
stripWorkIdentifierEcho('Fix ENG 456 crash', { label: 'ENG-456', tokens: ['eng', '456'] })
|
||||
).toBe('Fix crash')
|
||||
})
|
||||
})
|
||||
|
|
@ -1,190 +0,0 @@
|
|||
// Why: a single, host-aware parser for the review target named in a prompt (a
|
||||
// PR, MR, issue, or ticket), shared by the sidebar workspace name and the tab
|
||||
// title so both surface the same identifier. URLs are validated by *path
|
||||
// structure* (owner/repo/pull/N, GitLab's `/-/` marker) rather than hostname,
|
||||
// which keeps GitHub Enterprise and self-hosted GitLab working while rejecting
|
||||
// stray URLs that merely contain `/pull/<n>` (CDN assets, docs pages).
|
||||
|
||||
export type WorkIdentifier = {
|
||||
/** Human label, identifier-first, e.g. `PR 1033`, `MR 42`, `ENG-456`. */
|
||||
label: string
|
||||
/** Lowercased identifier tokens, so consumers can drop them from a slug or
|
||||
* description rather than echoing `Pr`, a bare number, or the ticket twice. */
|
||||
tokens: string[]
|
||||
}
|
||||
|
||||
// Prompts can be paste-sized, and a review target is named up front — so bound
|
||||
// the scan to a prefix rather than running regexes over the whole prompt.
|
||||
const IDENTIFIER_SCAN_LIMIT = 4096
|
||||
|
||||
// Uppercase prefixes that look like Jira/Linear keys but are standards, ciphers,
|
||||
// or encodings — kept off the ticket path so `SHA-256` / `UTF-8` / `ISO-8601`
|
||||
// don't become workspace names. Single-letter prefixes (`P-256`) can't match the
|
||||
// two-letter-minimum pattern, so they need no entry here.
|
||||
const NON_TICKET_PREFIXES = new Set([
|
||||
'UTF',
|
||||
'SHA',
|
||||
'MD',
|
||||
'ISO',
|
||||
'RFC',
|
||||
'AES',
|
||||
'RSA',
|
||||
'EC',
|
||||
'ES',
|
||||
'RS',
|
||||
'HS',
|
||||
'PS',
|
||||
'GPT',
|
||||
'MPEG',
|
||||
'UTC',
|
||||
'GMT',
|
||||
'IPV',
|
||||
'IEEE',
|
||||
'ANSI',
|
||||
'ASCII',
|
||||
'TLS',
|
||||
'SSL',
|
||||
'HTTP',
|
||||
'HTTPS'
|
||||
])
|
||||
|
||||
const URL_IN_TEXT = /https?:\/\/[^\s<>()[\]"']+/gi
|
||||
// GitLab's project-internal `/-/` marker is unambiguous; a GitLab issue path
|
||||
// also would not match the GitHub pattern, so ordering GitLab first is safe.
|
||||
const GITLAB_ITEM_PATH = /\/-\/(issues|work_items|merge_requests)\/(\d+)(?:[/?#]|$)/i
|
||||
const GITHUB_ITEM_PATH = /^\/[^/]+\/[^/]+\/(issues|pull)\/(\d+)(?:[/?#]|$)/i
|
||||
// Bitbucket Cloud: /workspace/repo/pull-requests/N
|
||||
const BITBUCKET_CLOUD_ITEM_PATH = /^\/[^/]+\/[^/]+\/pull-requests\/(\d+)(?:[/?#]|$)/i
|
||||
// Bitbucket Server / Data Center nests the repo under a project or user, so the
|
||||
// PR path carries more segments than Cloud: /projects/KEY/repos/REPO/pull-requests/N.
|
||||
const BITBUCKET_SERVER_ITEM_PATH =
|
||||
/\/(?:projects|users)\/[^/]+\/repos\/[^/]+\/pull-requests\/(\d+)(?:[/?#]|$)/i
|
||||
// Azure DevOps (dev.azure.com, *.visualstudio.com, on-prem collections) always
|
||||
// routes a PR through /_git/REPO/pullrequest/N, regardless of org/project prefix.
|
||||
const AZURE_DEVOPS_ITEM_PATH = /\/_git\/[^/]+\/pullrequests?\/(\d+)(?:[/?#]|$)/i
|
||||
|
||||
function taggedIdentifier(type: 'PR' | 'MR' | 'Issue', num: string): WorkIdentifier {
|
||||
return { label: `${type} ${num}`, tokens: [type.toLowerCase(), num] }
|
||||
}
|
||||
|
||||
function urlToIdentifier(raw: string): WorkIdentifier | null {
|
||||
let url: URL
|
||||
try {
|
||||
url = new URL(raw)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
if (url.protocol !== 'https:' && url.protocol !== 'http:') {
|
||||
return null
|
||||
}
|
||||
const path = url.pathname
|
||||
const gitlab = GITLAB_ITEM_PATH.exec(path)
|
||||
if (gitlab) {
|
||||
return gitlab[1].toLowerCase() === 'merge_requests'
|
||||
? taggedIdentifier('MR', gitlab[2])
|
||||
: taggedIdentifier('Issue', gitlab[2])
|
||||
}
|
||||
const github = GITHUB_ITEM_PATH.exec(path)
|
||||
if (github) {
|
||||
return github[1].toLowerCase() === 'pull'
|
||||
? taggedIdentifier('PR', github[2])
|
||||
: taggedIdentifier('Issue', github[2])
|
||||
}
|
||||
const bitbucketCloud = BITBUCKET_CLOUD_ITEM_PATH.exec(path)
|
||||
if (bitbucketCloud) {
|
||||
return taggedIdentifier('PR', bitbucketCloud[1])
|
||||
}
|
||||
const bitbucketServer = BITBUCKET_SERVER_ITEM_PATH.exec(path)
|
||||
if (bitbucketServer) {
|
||||
return taggedIdentifier('PR', bitbucketServer[1])
|
||||
}
|
||||
const azureDevops = AZURE_DEVOPS_ITEM_PATH.exec(path)
|
||||
if (azureDevops) {
|
||||
return taggedIdentifier('PR', azureDevops[1])
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function findUrlIdentifier(text: string): WorkIdentifier | null {
|
||||
const urls = text.match(URL_IN_TEXT)
|
||||
if (!urls) {
|
||||
return null
|
||||
}
|
||||
for (const raw of urls) {
|
||||
// Trim trailing sentence punctuation and markdown emphasis (`_`/`*`/`~`): a
|
||||
// URL wrapped like `_…/pull/5_` otherwise keeps the `_`, breaking the path
|
||||
// anchor so the identifier is lost. Interior `_` (`merge_requests`) is kept.
|
||||
const identifier = urlToIdentifier(raw.replace(/[.,;:!?*_~]+$/, ''))
|
||||
if (identifier) {
|
||||
return identifier
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Pull the review-target identifier out of raw prompt text. Precedence runs from
|
||||
* most reliable (provider URLs) to least (a bare `#123`), so a real URL wins over
|
||||
* an incidental ticket-shaped token. Returns null when the prompt names none.
|
||||
*/
|
||||
export function extractWorkIdentifier(text: string): WorkIdentifier | null {
|
||||
const scanned = text.slice(0, IDENTIFIER_SCAN_LIMIT)
|
||||
|
||||
const urlIdentifier = findUrlIdentifier(scanned)
|
||||
if (urlIdentifier) {
|
||||
return urlIdentifier
|
||||
}
|
||||
|
||||
// Textual references ("pull request #12", "PR 12", "issue 88").
|
||||
let match = scanned.match(/\bmerge\s+request\s*[#!]?\s*(\d+)/i)
|
||||
if (match) {
|
||||
return taggedIdentifier('MR', match[1])
|
||||
}
|
||||
match = scanned.match(/\bpull\s+request\s*#?\s*(\d+)/i) ?? scanned.match(/\bpr\s*#?\s*(\d+)/i)
|
||||
if (match) {
|
||||
return taggedIdentifier('PR', match[1])
|
||||
}
|
||||
match = scanned.match(/\bissue\s*#?\s*(\d+)/i)
|
||||
if (match) {
|
||||
return taggedIdentifier('Issue', match[1])
|
||||
}
|
||||
|
||||
// Namespaced ticket id (Jira/Linear), used bare. Uppercase-only so lowercase
|
||||
// tokens like `gpt-4` don't match; skip standards/cipher prefixes, and keep
|
||||
// scanning so a real key after one (e.g. `SHA-256 … ENG-456`) still resolves.
|
||||
for (const ticket of scanned.matchAll(/\b([A-Z]{2,10})-(\d{1,7})\b/g)) {
|
||||
if (!NON_TICKET_PREFIXES.has(ticket[1])) {
|
||||
return { label: `${ticket[1]}-${ticket[2]}`, tokens: [ticket[1].toLowerCase(), ticket[2]] }
|
||||
}
|
||||
}
|
||||
|
||||
// Bare `#123` as a last resort — identifier-first but provider-agnostic.
|
||||
match = scanned.match(/(?:^|\s)#(\d+)\b/)
|
||||
if (match) {
|
||||
return { label: `#${match[1]}`, tokens: [match[1]] }
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Compose an identifier-first label — `PR 1033 - Review`, or just `PR 1033` when
|
||||
* there is no trailing detail. The single source of the format shared by the
|
||||
* sidebar name, tab title, and auto-rename name so they cannot drift apart.
|
||||
*/
|
||||
export function formatIdentifierFirst(label: string, detail: string): string {
|
||||
return detail ? `${label} - ${detail}` : label
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove the identifier's own tokens from a description so a caller can prepend
|
||||
* the label without echoing it — `PR 1094 - Review this PR` becomes
|
||||
* `PR 1094 - Review this`.
|
||||
*/
|
||||
export function stripWorkIdentifierEcho(text: string, identifier: WorkIdentifier): string {
|
||||
let stripped = text
|
||||
for (const token of identifier.tokens) {
|
||||
stripped = stripped.replace(new RegExp(`\\b${token}\\b`, 'gi'), ' ')
|
||||
}
|
||||
return stripped.replace(/\s+/g, ' ').trim()
|
||||
}
|
||||
|
|
@ -95,8 +95,8 @@ describe('getWorkspaceIntentName', () => {
|
|||
}
|
||||
})
|
||||
).toEqual({
|
||||
displayName: 'Issue 2635 - Fix',
|
||||
seedName: 'issue-2635-fix'
|
||||
displayName: 'Fix Issue 2635',
|
||||
seedName: 'fix-issue-2635'
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -111,8 +111,8 @@ describe('getWorkspaceIntentName', () => {
|
|||
}
|
||||
})
|
||||
).toEqual({
|
||||
displayName: 'PR 1234 - Review',
|
||||
seedName: 'pr-1234-review'
|
||||
displayName: 'Review PR 1234',
|
||||
seedName: 'review-pr-1234'
|
||||
})
|
||||
expect(
|
||||
getWorkspaceIntentName({
|
||||
|
|
@ -125,8 +125,8 @@ describe('getWorkspaceIntentName', () => {
|
|||
}
|
||||
})
|
||||
).toEqual({
|
||||
displayName: 'MR 77 - Fix',
|
||||
seedName: 'mr-77-fix'
|
||||
displayName: 'Fix MR 77',
|
||||
seedName: 'fix-mr-77'
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ import {
|
|||
collectCompactWorkspaceWords,
|
||||
foldWorkspaceNameWhitespaceToHyphen
|
||||
} from './workspace-name-text-scanner'
|
||||
import { formatIdentifierFirst } from './work-item-reference'
|
||||
|
||||
function normalizeApostrophes(input: string): string {
|
||||
return input.replace(/[‘’]/g, "'")
|
||||
|
|
@ -215,10 +214,7 @@ export function getWorkspaceIntentName(args: {
|
|||
const action = detectIntentAction(sourceText) ?? defaultActionForWorkItem(item)
|
||||
const identity = workItemIdentity(item)
|
||||
if (action) {
|
||||
// Identifier-first so the sidebar leads with the searchable token
|
||||
// (`PR 1033 - Review`); the shared formatIdentifierFirst keeps first-create,
|
||||
// auto-rename, and tab-title names on one format.
|
||||
displayName = formatIdentifierFirst(identity, action)
|
||||
displayName = `${action} ${identity}`
|
||||
} else {
|
||||
const subject = compactWorkItemTitle(item.title, item)
|
||||
displayName = [identity, subject].filter(Boolean).join(' ')
|
||||
|
|
|
|||
Loading…
Reference in New Issue