Skip to content

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

Open
icecrasher321 wants to merge 5 commits into
stagingfrom
staging-v98
Open

fix(secrets): ignore Note blocks in fork mapping and secret references, refetch references on return#7426
icecrasher321 wants to merge 5 commits into
stagingfrom
staging-v98

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • A {{KEY}} that only appears inside a Note block is no longer a reference: the shared fork remapper skips detection for annotation-only blocks, so it never lands in the fork mapping table, never blocks a sync, and no longer shows in the secret's References tab. A mapped key inside a note is still rewritten on sync, same as a condition-hidden field.
  • A key named by both a Note and an executing block is still detected, attributed to the executing block.
  • The References tab now reflects the canvas when you come back to it: stale time is 0 and the query refetches on window focus, so deleting a referencing block and returning no longer shows the cached list.
  • A sync's background resource copy no longer shows up as a separate row badged "Fork". promoteFork records the sync's Activity row itself (the route no longer does), keeps it processing while copied tables, knowledge bases, and files are filled, and the content-copy runner finishes that same row. The push or pull row shows "Copying …" while it runs and "Copied …" plus any losses when done. A clean fill can't erase the sync's own warnings: the runner gets the sync's completion status and only ever upgrades it. A deploy that succeeded with its cutover or a side effect still pending now also marks the sync with warnings and lists the reason on the row.
  • Forks docs: a {{KEY}} inside a Note never needs mapping; a sync's copy progress shows in its own row.
  • Ship skill: add CI's docs-manifest:check to the pre-ship audits.
  • Fork-copied tables now get the same tbl_ + dash-stripped UUID id as created tables: one generateTableId() helper in lib/table/ids.ts, used by both the create service and the fork copy.

Type of Change

  • Bug fix

Testing

  • Note tests: not detected by the fork scan, still rewritten when mapped, dropped by the References scanner, and a key in both a Note and a real block is reported for the real block only. All fail against the pre-change walker.
  • Sync activity tests: one terminal fork_sync row when nothing needs a fill; a processing fork_sync row handed to the runner (no fork_content_copy row) when it does; a lost trigger URL is kept as a warning through the fill; the runner honours the caller's completion status and still downgrades to warnings when an item is lost; the panel renders "Copying"/"Copied" and losses inside the push row. All six behavioural tests fail against the previous code.
  • Table id shape pinned for the helper and for a fork-copied table (the latter fails against the previous copy path).
  • bun run lint, block-registry check, check:audits, docs-manifest:check, and type-check pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 3, 2026 3:23am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR updates secret-reference scanning, fork-sync activity tracking, reference-query freshness, and forked table ID generation.

  • Excludes annotation-only Note blocks from secret-reference detection while retaining mapped-key rewriting.
  • Tracks background resource copying within the originating push or pull activity row.
  • Refreshes secret references after returning to the References view or refocusing the window.
  • Standardizes created and fork-copied table IDs through a shared generator.
  • Updates tests, documentation, and pre-ship audits for the changed behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/ee/workspace-forking/lib/promote/promote.ts Consolidates sync and background-copy activity into one lifecycle while preserving deployment warnings.
apps/sim/ee/workspace-forking/lib/copy/content-copy-runner.ts Allows a successful content fill to retain the sync phase's warning status.
apps/sim/ee/workspace-forking/lib/remap/remap-references.ts Skips dependency detection for annotation-only Note blocks without disabling mapped-value rewriting.
apps/sim/lib/secrets/references/scan.test.ts Covers exclusion of Note-only secret references from the References scanner.
apps/sim/hooks/queries/credentials.ts Makes secret-reference results immediately stale and refetches them on window focus.
apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx Renders content-copy progress, outcomes, and warnings inside the associated sync row.
apps/sim/lib/table/ids.ts Introduces the shared canonical table-ID generator used by creation and fork-copy paths.
apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts Uses canonical table IDs for copied table containers.

Sequence Diagram

sequenceDiagram
  participant User
  participant Route as Promote route
  participant Promote as promoteFork
  participant Activity as Activity store
  participant Runner as Content-copy runner
  User->>Route: Push or pull fork changes
  Route->>Promote: Sync with partner display name
  Promote->>Promote: Write workflows and deploy
  alt Background content remains
    Promote->>Activity: Start processing fork_sync row
    Promote-->>Runner: Schedule content fill with row ID
    Runner->>Activity: Finish same row with copy outcome
  else No background content
    Promote->>Activity: Record terminal fork_sync row
  end
  Promote-->>Route: Return sync result
  Route-->>User: Sync response
Loading

Reviews (5): Last reviewed commit: "fix(forks): label a failed fill honestly..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/docs/content/docs/platform/enterprise/forks.mdx Outdated
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 15 files

Confidence score: 3/5

  • In apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx, failed background-copy scheduling can still render planned counts as Copied, misrepresenting content that never ran; use a failure-specific label for this state.
  • In apps/sim/ee/workspace-forking/lib/promote/promote.ts, syncs containing only skills or mapped-KB documents can remain in processing without visible counts, leaving activity progress unclear; add metadata and panel support for those content types.
  • In apps/sim/ee/workspace-forking/lib/promote/promote.ts, successful deploys with warnings are recorded as completed and hide deferred side-effect warnings from the Activity row; carry result.warnings into the sync status and display.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx">

<violation number="1" location="apps/sim/ee/workspace-forking/components/fork-activity-panel/fork-activity-panel.tsx:191">
P2: When background-copy scheduling fails, the sync row is marked `failed` but still contains its planned content counts, so this branch displays `Copied` for content that never ran. Use a failure-specific label for failed rows instead of treating every non-processing status as completed.</violation>
</file>

<file name="apps/sim/ee/workspace-forking/lib/promote/promote.ts">

<violation number="1" location="apps/sim/ee/workspace-forking/lib/promote/promote.ts:226">
P2: When a deploy succeeds with `result.warnings`, this predicate still records the sync as `completed`; `deployWarnings` is only logged later, so the Activity row hides deferred side-effect warnings. Carry those warnings into `buildSyncActivity` and mark the row `completed_with_warnings`.</violation>

<violation number="2" location="apps/sim/ee/workspace-forking/lib/promote/promote.ts:1199">
P2: When a sync copies only skills or mapped-KB documents, `hasForkContentToCopy` still keeps this row processing, but this metadata writes no visible count for either kind. Add metadata and panel support for those plan kinds so the row shows `Copying` and `Copied`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/ee/workspace-forking/lib/promote/promote.ts
Comment thread apps/sim/ee/workspace-forking/lib/promote/promote.ts
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 21 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

… and surface deploy warnings on the sync row
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 22 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant