fix(secrets): ignore Note blocks in fork mapping and secret references, refetch references on return

Author: icecrasher321

.agents/skills/ship/SKILL.md

@@ -66,6 +66,10 @@ When the user runs `/ship`:
   # Runs every audit CI runs, concurrently, and replays the output of any that fail.
   # The audit list is derived in scripts/run-audits.ts — do not hand-list audits here.
   bun run check:audits || { echo "❌ audit(s) failed — do not ship"; exit 1; }
+  # CI's "Verify docs manifest is in sync" step is not a `check:*` script, so the runner above
+  # does not cover it. (CI's "Security audit" `bun audit` step is `continue-on-error` — advisory
+  # only, not a gate — so it is deliberately not run here.)
+  bun run docs-manifest:check || { echo "❌ docs manifest out of sync — do not ship"; exit 1; }
   ```
   If Phase A regenerated a file, its matching `:check` in Phase B now passes trivially — that parity is the point. Do not ship with any generator or audit failing; fix the cause (never silence it) and re-run. `check:migrations` and `type-check` are covered by steps 5 and CI respectively and are not repeated here.
 7. **Stage and commit** the changes with the generated message — including any files Phase A regenerated in step 6

apps/docs/content/docs/platform/enterprise/forks.mdx

@@ -331,6 +331,8 @@ Servers that **publish workflows as MCP tools**.
 | **Fork** | **Values never leave the source.** Workflow text still contains `{{KEY}}` names. Create matching secrets (or the names you will map to) under the child’s **Secrets**. |
 | **Sync** | Map source key names to target key names. Values stay in each workspace. Unmapped required secrets block Sync. |
 
+Notes are documentation: a `{{KEY}}` inside a Note block never needs mapping and never blocks Sync.
+
 **Example:** Workflows use `{{OPENAI_API_KEY}}`. After fork, add that secret in the child (or map `OPENAI_API_KEY` to whatever name the child uses) before runs and syncs succeed.
 
 ---

apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts

@@ -1835,6 +1835,75 @@ describe('canonical mode policy (fork/promote)', () => {
     expect(scan.references).toEqual([])
   })
 
+  it('does NOT detect {{ENV}} in a Note block (an annotation never executes)', () => {
+    vi.mocked(getBlock).mockReturnValue(
+      blockWith([{ id: 'content', title: 'Content', type: 'long-input' }])
+    )
+    const scan = scanWorkflowReferences(
+      [
+        {
+          id: 'b1',
+          name: 'Setup notes',
+          type: 'note',
+          subBlocks: {
+            content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.'),
+          },
+        },
+      ],
+      () => null
+    )
+    expect(scan.references).toEqual([])
+    expect(scan.unmapped).toEqual([])
+  })
+
+  it('still detects {{ENV}} named by both a Note and an executing block, attributed to the executing block', () => {
+    vi.mocked(getBlock).mockReturnValue(
+      blockWith([
+        { id: 'content', title: 'Content', type: 'long-input' },
+        { id: 'apiKey', title: 'API Key', type: 'short-input' },
+      ])
+    )
+    const scan = scanWorkflowReferences(
+      [
+        {
+          id: 'b1',
+          name: 'Setup notes',
+          type: 'note',
+          subBlocks: {
+            content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.'),
+          },
+        },
+        {
+          id: 'b2',
+          name: 'Agent',
+          type: 'agent',
+          subBlocks: { apiKey: entry('apiKey', 'short-input', '{{OPENAI_API_KEY}}') },
+        },
+      ],
+      () => null
+    )
+    expect(
+      scan.references.map((ref) => [ref.kind, ref.sourceId, ref.blockId, ref.subBlockKey])
+    ).toEqual([['env-var', 'OPENAI_API_KEY', 'b2', 'apiKey']])
+    expect(scan.unmapped.map((ref) => ref.sourceId)).toEqual(['OPENAI_API_KEY'])
+  })
+
+  it('still rewrites a mapped {{ENV}} inside a Note on promote, so the note names the target key', () => {
+    vi.mocked(getBlock).mockReturnValue(
+      blockWith([{ id: 'content', title: 'Content', type: 'long-input' }])
+    )
+    const result = remapForkSubBlocks(
+      { content: entry('content', 'long-input', 'Set {{OPENAI_API_KEY}} before running.') },
+      (kind, sourceId) =>
+        kind === 'env-var' && sourceId === 'OPENAI_API_KEY' ? 'OPENAI_KEY_PROD' : null,
+      'promote',
+      { blockType: 'note' }
+    )
+    expect(result.subBlocks.content.value).toBe('Set {{OPENAI_KEY_PROD}} before running.')
+    expect(result.references).toEqual([])
+    expect(result.unmapped).toEqual([])
+  })
+
   it('an active manual member keeps its RESOURCE-id escape hatch while its {{ENV}} is detected', () => {
     vi.mocked(getBlock).mockReturnValue(
       blockWith([

apps/sim/ee/workspace-forking/lib/remap/remap-references.ts

@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { isRecordLike, omit } from '@sim/utils/object'
 import type { SubBlockType } from '@sim/workflow-types/blocks'
+import { isWorkflowAnnotationOnlyBlockType } from '@sim/workflow-types/workflow'
 import type { z } from 'zod'
 import type { forkRemapKindSchema } from '@/lib/api/contracts/workspace-fork'
 import { readFolderPaths, replaceFolderPath } from '@/lib/folders/selection'
@@ -580,7 +581,10 @@ export function createCanonicalModeGates(
 export interface RemapForkContext {
   blockId?: string
   blockName?: string
-  /** Block type, to build the canonical index for active-member DETECTION gating (rewrite unaffected). */
+  /**
+   * Block type, to build the canonical index for active-member DETECTION gating and to recognise
+   * an annotation-only block, whose values are never detected. Rewrite is unaffected by either.
+   */
   blockType?: string
   /** Canonical-mode overrides (`block.data.canonicalModes`), picking the active member per pair. */
   canonicalModes?: CanonicalModeOverrides
@@ -1126,9 +1130,9 @@ export function remapForkSubBlocks(
   // active ADVANCED (manual) member - and every dependent scoped to it - passes through VERBATIM
   // (user-owned, never remapped, never a mapping requirement); a DORMANT member's value is
   // CLEARED outright (below) so no stale id ever survives in an inactive slot. A condition-hidden
-  // subblock is still rewritten but not detected. Needs `blockType` for the config; an unknown
-  // block type gets no gating (everything detected, nothing passed through - the conservative
-  // default).
+  // subblock is still rewritten but not detected, and so is every field of an annotation-only
+  // block (see `annotationOnly` below). Needs `blockType` for the config; an unknown block type
+  // gets no gating (everything detected, nothing passed through - the conservative default).
   const blockSubBlocks = context?.blockType ? getBlock(context.blockType)?.subBlocks : undefined
   const configByBaseKey = new Map(
     (blockSubBlocks ?? []).filter((config) => config.id).map((config) => [config.id, config])
@@ -1140,6 +1144,15 @@ export function remapForkSubBlocks(
     context?.triggerMode === true
   )
 
+  /**
+   * An annotation-only block (a Note) never executes, so nothing in it is a reference. Its
+   * `{{KEY}}` is prose about a secret, not a use of one: it must not become a mapping entry or a
+   * sync blocker, and the References tab — which walks blocks through this same policy — must not
+   * send someone rotating a key to edit a note. The rewrite side is unchanged, exactly like a
+   * condition-hidden field's: a mapped key is still renamed so the note names the target's key.
+   */
+  const annotationOnly = isWorkflowAnnotationOnlyBlockType(context?.blockType)
+
   for (const [subBlockKey, subBlock] of Object.entries(subBlocks)) {
     if (!subBlock || typeof subBlock !== 'object') {
       result[subBlockKey] = subBlock
@@ -1169,7 +1182,8 @@ export function remapForkSubBlocks(
     const verbatimManual =
       !dormant &&
       (gates.isActiveManualMember(subBlockKey) || gates.isManualParentDependent(subBlockKey))
-    const detectionSkipped = dormant || verbatimManual || gates.isConditionHidden(subBlockKey)
+    const detectionSkipped =
+      annotationOnly || dormant || verbatimManual || gates.isConditionHidden(subBlockKey)
     // `{{ENV}}` detection is gated on EXECUTION, not on ownership. A dormant member and a
     // condition-hidden field never execute, so their refs must not become sync blockers - but an
     // ACTIVE MANUAL member is exactly the value that DOES execute, and its `{{KEY}}` is a live
@@ -1179,7 +1193,7 @@ export function remapForkSubBlocks(
     // missing that secret silently passed the required-env gate instead of blocking the sync.
     // Resource-id detection keeps `verbatimManual` (a hand-typed id stays a user-owned escape
     // hatch); only env refs, which are never workspace-scoped ids, are detected here.
-    const envDetectionSkipped = dormant || gates.isConditionHidden(subBlockKey)
+    const envDetectionSkipped = annotationOnly || dormant || gates.isConditionHidden(subBlockKey)
     if (dormant && isNonEmptyValue(value)) {
       value = ''
     }

apps/sim/hooks/queries/credentials.ts

@@ -319,11 +319,13 @@ export function useSecretUsage({ workspaceId, name, scope }: SecretUsageParams,
 }
 
 /**
- * References only move when someone edits a workflow, a custom tool, or an MCP server — far
- * less often than the usage trail, which every run appends to. A longer window keeps the scan
- * (which reads every candidate block in the workspace) off the wire on tab switches.
+ * Always stale: the list mirrors the canvas, and the reader has usually just come from editing
+ * it — deleting the block a row pointed at, then returning here to see it gone. A stale window
+ * served the old list for its whole length, and no invalidation can cover a workflow someone
+ * else changed. Cached data still shows while the scan refreshes, so a tab switch costs one
+ * bounded, prefiltered query rather than a blank panel.
  */
-export const SECRET_REFERENCES_STALE_TIME = 5 * 60 * 1000
+export const SECRET_REFERENCES_STALE_TIME = 0
 
 interface SecretReferencesParams {
   workspaceId?: string
@@ -334,6 +336,11 @@ interface SecretReferencesParams {
  * Reads where one secret is wired in. Takes no scope: a reference names a key, not a scope, and
  * the server authorizes against what the name resolves to. Only credential admins of a workspace
  * secret — or the owner of a personal one — are authorized server-side.
+ *
+ * Refetches on window focus even on the web, where the app default leaves it off: the edit that
+ * moves a reference happens on a canvas, often in another tab, and this panel has no other
+ * signal for it. `'always'`, not `true`: `true` refetches only a STALE query, which would tie
+ * this guarantee to `SECRET_REFERENCES_STALE_TIME` staying exactly 0.
  */
 export function useSecretReferences({ workspaceId, name }: SecretReferencesParams, enabled = true) {
   return useQuery({
@@ -345,5 +352,6 @@ export function useSecretReferences({ workspaceId, name }: SecretReferencesParam
       }),
     enabled: Boolean(workspaceId && name) && enabled,
     staleTime: SECRET_REFERENCES_STALE_TIME,
+    refetchOnWindowFocus: 'always',
   })
 }

apps/sim/lib/secrets/references/scan.test.ts

@@ -114,6 +114,65 @@ describe('scanSecretReferences', () => {
     expect(scan.workflows).toEqual([])
   })
 
+  /**
+   * A Note is documentation on the canvas: its text never resolves at run time, so a `{{KEY}}`
+   * in it is prose about the secret, not a use of it. The remapper drops it for the same reason
+   * it drops a condition-hidden field, and the tab has to agree — rotating a key does not mean
+   * editing every note that mentions it.
+   */
+  it('drops a Note block that mentions the secret', async () => {
+    queueTableRows(schemaMock.workflowBlocks, [
+      blockRow({
+        blockId: 'block-1',
+        blockName: 'Setup notes',
+        blockType: 'note',
+        workflowId: 'workflow-1',
+        workflowName: 'Nightly sync',
+        subBlocks: {
+          content: { id: 'content', type: 'long-input', value: 'Uses {{API_KEY}} for auth.' },
+        },
+      }),
+    ])
+
+    const scan = await scanSecretReferences({ workspaceId: 'workspace-1', name: 'API_KEY' })
+
+    expect(scan.workflows).toEqual([])
+  })
+
+  it('lists only the executing block when a Note and a block both name the secret', async () => {
+    queueTableRows(schemaMock.workflowBlocks, [
+      blockRow({
+        blockId: 'block-1',
+        blockName: 'Setup notes',
+        blockType: 'note',
+        workflowId: 'workflow-1',
+        workflowName: 'Nightly sync',
+        subBlocks: {
+          content: { id: 'content', type: 'long-input', value: 'Uses {{API_KEY}} for auth.' },
+        },
+      }),
+      blockRow({
+        blockId: 'block-2',
+        blockName: 'Fetch orders',
+        workflowId: 'workflow-1',
+        workflowName: 'Nightly sync',
+        subBlocks: shortInput('apiKey', '{{API_KEY}}'),
+      }),
+    ])
+
+    const scan = await scanSecretReferences({ workspaceId: 'workspace-1', name: 'API_KEY' })
+
+    expect(scan.workflows).toEqual([
+      {
+        workflowId: 'workflow-1',
+        workflowName: 'Nightly sync',
+        blocks: [
+          { blockId: 'block-2', blockName: 'Fetch orders', blockType: 'agent', field: 'apiKey' },
+        ],
+      },
+    ])
+  })
+
   /**
    * The fork remapper collapses a block's references to one per `(kind, sourceId)`, so a block
    * naming the secret twice yields one entry, not two. Pinned here because the panel renders