fix(secrets): ignore Note blocks in fork mapping and secret references, refetch references on return - #7426
fix(secrets): ignore Note blocks in fork mapping and secret references, refetch references on return#7426icecrasher321 wants to merge 5 commits into
Conversation
…s, refetch references on return
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThe PR updates secret-reference scanning, fork-sync activity tracking, reference-query freshness, and forked table ID generation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| 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
Reviews (5): Last reviewed commit: "fix(forks): label a failed fill honestly..." | Re-trigger Greptile
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
…tead of a separate Fork entry
|
@cubic-dev-ai review this PR |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
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 asCopied, 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; carryresult.warningsinto 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
|
@cubic-dev-ai review this PR |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
… and surface deploy warnings on the sync row
|
@cubic-dev-ai review this PR |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
Summary
{{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.promoteForkrecords the sync's Activity row itself (the route no longer does), keeps itprocessingwhile 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.{{KEY}}inside a Note never needs mapping; a sync's copy progress shows in its own row.docs-manifest:checkto the pre-ship audits.tbl_+ dash-stripped UUID id as created tables: onegenerateTableId()helper inlib/table/ids.ts, used by both the create service and the fork copy.Type of Change
Testing
fork_syncrow when nothing needs a fill; aprocessingfork_syncrow handed to the runner (nofork_content_copyrow) 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.bun run lint, block-registry check,check:audits,docs-manifest:check, andtype-checkpass.Checklist