fix(forking): keep dependent overrides editable - #6776
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
PR SummaryMedium Risk Overview Data model: Nested tool params now carry optional Logic: UI: Mapping entries add an Edit configuration chip when the parent is resolved and there are hidden configured fields. That reveals all active selectors under the parent without changing sync blockers; workflow cards opened via edit start expanded. Tool grouping in cards keys off Reviewed by Cursor Bugbot for commit 8e63f80. Configure here. |
Greptile SummaryThis PR keeps saved dependent mappings editable while separating nested dependency chains by tool instance.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx | The edit action now mounts optional-only configuration cards in an expanded state, resolving the previously reported ineffective-click behavior. |
| apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts | Display filtering is separated from Sync actionability, and dependency traversal is constrained to the active tool-instance scope. |
| apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts | Nested tool dependents now receive stable instance scopes and retain canonical provider and consumer relationships. |
| apps/sim/lib/api/contracts/workspace-fork.ts | The dependent-reconfiguration contract adds optional dependency-scope metadata for nested tool instances. |
Reviews (2): Last reviewed commit: "fix(forking): expand configured edit car..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8e63f80. Configure here.
#6776 landed on staging as a competing fix for the same fork-sync defects this branch addressed. Take its work wholesale and keep only the part of ours it does not cover. Kept from upstream (#6776): the `sameDependencyScope` cascade guard, `getDisplayedDependentFields` with the "Edit configuration" chip, per-scope provider indexing, the `blockChainState(block, field, ...)` scope filter, and the `dependent-reconfigs` / `workspace-fork` contract changes that emit `dependencyScope` and per-scope context keys for nested tool params. Kept from this branch: `DEPENDENT_CLEARED_BY_PARENT` and `submittedDependentValue`. The scope guard decides WHICH descendants a re-pick invalidates; the sentinel decides HOW an invalidated one is represented, and those are different layers. Without the sentinel the cascade still writes `''` into the reconfig map and `buildDependentValues` still submits it, so a hidden optional dependent's stored target value is destroyed by a parent re-pick the user never applied to it. The scope guard does not close this: two top-level block subblocks both have `dependencyScope === undefined`, so it is a no-op there. #6776 also widens the exposure by emitting context keys for nested tool params that previously could never be cascaded onto. Also kept: the `previousValue` no-op guard in `applyDependentRepick` (a separate bug - re-selecting the value a field already shows must not invalidate its descendants) and the post-sync reset in `use-fork-sync`. Dropped from this branch: the sticky-visibility predicate in `isDependentConfigurationActionable` and its three tests. It and upstream's edit chip are two mechanisms for one visibility problem, and it is unnecessary - a marked REQUIRED field reads as `''` through `effectiveDependentValue`, so the existing `required && value === ''` arm keeps it on screen and keeps it gating Sync. Only marked OPTIONAL fields drop out of the default view, and those are omitted from the payload, so hiding them costs nothing; the edit chip brings them back. Reconciled the cascade assertions in `dependent-value.test.ts` to the sentinel, including upstream's nested-tool-instance case.
1 participant
Summary
Type of Change
Testing
bunx vitest run ee/workspace-forking/components/fork-sync/dependent-value.test.ts ee/workspace-forking/lib/mapping/dependent-reconfigs.test.ts lib/api/contracts/workspace-fork.test.tsbun run lintbun run type-checkbun run check:auditsbun run apps/sim/scripts/check-block-registry.ts origin/stagingChecklist