fix(forks): name the workspace a sync overwrites instead of "target" (#6822)

* fix(forks): name the workspace a sync overwrites instead of "target" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): name the target workspace in the blocker resolution line too Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-24 15:45:35 +08:00 · 2026-08-18 14:56:39 -07:00
parent 56a270ed54
commit e522bc4c5d
5 changed files with 55 additions and 19 deletions
@@ -149,21 +149,36 @@ describe('splitForkClearedRefs', () => {

 describe('forkBlockerResolution', () => {
  it('phrases each blocker reason with its actionable resolution', () => {
-    expect(forkBlockerResolution(referenceRef('table', 'tbl-1'))).toBe(
+    expect(forkBlockerResolution(referenceRef('table', 'tbl-1'), 'Acme Prod')).toBe(
      'map it to a target or select it for copy'
    )
-    expect(forkBlockerResolution(referenceRef('mcp-server', 'srv-1'))).toBe(
+    expect(forkBlockerResolution(referenceRef('mcp-server', 'srv-1'), 'Acme Prod')).toBe(
      'map it to a target or select it for copy'
    )
-    expect(forkBlockerResolution(referenceRef('knowledge-base', 'kb-gone', 'KB', true))).toBe(
-      'deleted in the source — map it to an existing knowledge base in the target'
-    )
-    expect(forkBlockerResolution(workflowRef('wf-other', 'Workflow'))).toBe(
+    expect(
+      forkBlockerResolution(referenceRef('knowledge-base', 'kb-gone', 'KB', true), 'Acme Prod')
+    ).toBe('deleted in the source — map it to an existing knowledge base in Acme Prod')
+    expect(forkBlockerResolution(workflowRef('wf-other', 'Workflow'), 'Acme Prod')).toBe(
      'deploy "Source" in the source or remove the reference'
    )
  })

+  /**
+   * The source-deleted line phrases the same resolution as the mapping row's hint, so it must
+   * name the workspace the sync writes - "the target" is what this copy set out to remove.
+   */
+  it('names the target workspace in the source-deleted resolution', () => {
+    const resolution = forkBlockerResolution(
+      referenceRef('knowledge-base', 'kb-gone', 'KB', true),
+      'this workspace'
+    )
+    expect(resolution).toBe(
+      'deleted in the source — map it to an existing knowledge base in this workspace'
+    )
+    expect(resolution).not.toContain('in the target')
+  })
+
  it('returns null for non-blocking dependent entries', () => {
-    expect(forkBlockerResolution(dependentRef('credential', 'cred-1'))).toBeNull()
+    expect(forkBlockerResolution(dependentRef('credential', 'cred-1'), 'Acme Prod')).toBeNull()
  })
 })
@@ -77,15 +77,24 @@ export const FORK_RESOURCE_KIND_LABEL: Record<string, string> = {
 /**
 * The actionable resolution line for a blocking entry, phrased for "{block} would lose {field}
 * in {workflow} - {resolution}". Null for non-blocking (dependent) entries.
+ *
+ * `targetWorkspaceName` is required rather than defaulted: the source-deleted line phrases the
+ * same resolution as the mapping row's own hint, and naming the workspace is the only way either
+ * says WHICH side the sync writes. A default would let "the target" quietly return.
 */
-export function forkBlockerResolution(ref: ForkClearedRef): string | null {
+export function forkBlockerResolution(
+  ref: ForkClearedRef,
+  targetWorkspaceName: string
+): string | null {
  const reason = forkSyncBlockerReasonFor(ref)
  if (!reason) return null
  switch (reason) {
+    // "a target" here is the target RESOURCE picked in the mapping row, not the workspace -
+    // it matches the picker's own "Select target" label, so it stays unnamed.
    case 'unmapped-copyable':
      return 'map it to a target or select it for copy'
    case 'source-deleted':
-      return `deleted in the source — map it to an existing ${FORK_RESOURCE_KIND_LABEL[ref.kind] ?? 'resource'} in the target`
+      return `deleted in the source — map it to an existing ${FORK_RESOURCE_KIND_LABEL[ref.kind] ?? 'resource'} in ${targetWorkspaceName}`
    case 'workflow-missing':
      return `deploy "${ref.sourceLabel}" in the source or remove the reference`
  }
@@ -457,8 +457,8 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) {
        {entry.sourceDeleted ? (
          <p className='text-[var(--text-muted)] text-small'>
            Deleted in the source — its name can't be shown. Map it to an existing{' '}
-            {FORK_RESOURCE_KIND_LABEL[entry.kind] ?? 'resource'} in the target, or fix the reference
-            in the source and redeploy.
+            {FORK_RESOURCE_KIND_LABEL[entry.kind] ?? 'resource'} in {controller.targetWorkspaceName}
+            , or fix the reference in the source and redeploy.
          </p>
        ) : null}
        {entry.candidatesTruncated ? (
@@ -961,7 +961,8 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp
                  <span className='min-w-0'>
                    <span className='text-[var(--text-body)]'>{ref.blockLabel}</span> would lose{' '}
                    <span className='text-[var(--text-body)]'>{ref.fieldLabel}</span> in{' '}
-                    {ref.workflowName} — {forkBlockerResolution(ref)}
+                    {ref.workflowName} —{' '}
+                    {forkBlockerResolution(ref, controller.targetWorkspaceName)}
                  </span>
                  {/* Only a source-deleted reference can be dropped: an unmapped copyable can still
                      be copied and a missing workflow can still be deployed, so neither is a dead
@@ -104,8 +104,10 @@ export interface ForkSyncController {
  otherWorkspaceName: string
  /**
   * The workspace this sync WRITES, named for user-facing copy: the other workspace on push,
-   * "this workspace" on pull. Derived once here so every surface that names the target - the
-   * overwrite confirm, the Trigger URLs heading - says the same thing.
+   * this one on pull. Always a NAME rather than "the target" - the page header shows the OTHER
+   * workspace's name, so an unnamed target reads as that one even on pull. Falls back to
+   * "this workspace" only until the name loads. Derived once here so every surface that names
+   * it - the overwrite confirm, the Trigger URLs heading - says the same thing.
   */
  targetWorkspaceName: string
  isLoading: boolean
@@ -272,12 +274,15 @@ function takenTargetOwners(
 */
 export function useForkSync(params: {
  workspaceId: string
+  /** This workspace's name, for copy that must say which side a pull overwrites. */
+  workspaceName?: string
  otherWorkspaceId?: string
  otherWorkspaceName: string
  direction: ForkDirection
  enabled: boolean
 }): ForkSyncController {
-  const { workspaceId, otherWorkspaceId, otherWorkspaceName, direction, enabled } = params
+  const { workspaceId, workspaceName, otherWorkspaceId, otherWorkspaceName, direction, enabled } =
+    params

  // User's IN-SESSION mapping overrides only - NOT the source of truth. The displayed/persisted
  // target falls back to each entry's stored `targetId` (see `targetFor`), so a reopened edge
@@ -982,7 +987,8 @@ export function useForkSync(params: {
  return {
    direction,
    otherWorkspaceName,
-    targetWorkspaceName: direction === 'push' ? otherWorkspaceName : 'this workspace',
+    targetWorkspaceName:
+      direction === 'push' ? otherWorkspaceName : workspaceName || 'this workspace',
    isLoading: enabled && mapping.isLoading,
    isError: mapping.isError,
    errorMessage: mapping.isError ? getErrorMessage(mapping.error, 'Failed to load mapping') : null,
@@ -85,6 +85,8 @@ function ForkListRow({ name, actions }: ForkListRowProps) {
 interface ForkSyncDetailViewProps {
  title: string
  workspaceId: string
+  /** This workspace's name — a pull overwrites it, and the copy has to say which side that is. */
+  workspaceName?: string
  /** The other side of the edge being synced (this workspace's parent). */
  otherWorkspaceId: string
  otherWorkspaceName: string
@@ -105,6 +107,7 @@ interface ForkSyncDetailViewProps {
 function ForkSyncDetailView({
  title,
  workspaceId,
+  workspaceName,
  otherWorkspaceId,
  otherWorkspaceName,
  onBack,
@@ -118,6 +121,7 @@ function ForkSyncDetailView({

  const controller = useForkSync({
    workspaceId,
+    workspaceName,
    otherWorkspaceId,
    otherWorkspaceName,
    direction,
@@ -186,11 +190,11 @@ function ForkSyncDetailView({
        open={confirmSyncOpen}
        onOpenChange={setConfirmSyncOpen}
        srTitle='Sync workspace'
-        title='Overwrite target workspace'
+        title={`Overwrite ${targetWorkspaceName}`}
        text={[
-          'The target may have been modified since the last sync. Syncing will ',
+          'Syncing will ',
          { text: 'overwrite any changes', bold: true },
-          ' there. Continue?',
+          ` made in ${targetWorkspaceName} since the last sync. Continue?`,
        ]}
        confirm={{
          label: 'Sync',
@@ -431,6 +435,7 @@ export function Forks() {
          key={parent.id}
          title={parent.name}
          workspaceId={workspaceId}
+          workspaceName={workspaceName}
          otherWorkspaceId={parent.id}
          otherWorkspaceName={parent.name}
          onBack={() => void setSelectedForkId(null, { history: 'replace' })}