feat(cli): support per-workflow deletion dependencies in transcend.yml - #402
feat(cli): support per-workflow deletion dependencies in transcend.yml#402iamtheluckyest wants to merge 3 commits into
Conversation
Entries under `deletion-dependencies` may now carry an optional `workflow` to override the global configuration for a single erasure workflow, or `reset-to-global: true` to drop an override. Bare title strings still work. Pushes through the `dependedOnDataSilos` mutation input instead of the deprecated `dependedOnDataSiloTitles`.
@transcend-io/airgap.js-types
@transcend-io/cli
@transcend-io/design-tokens
@transcend-io/internationalization
@transcend-io/privacy-types
@transcend-io/sdk
@transcend-io/type-utils
@transcend-io/utils
@transcend-io/mcp
@transcend-io/mcp-server-admin
@transcend-io/mcp-server-assessment
@transcend-io/mcp-server-base
@transcend-io/mcp-server-consent
@transcend-io/mcp-server-discovery
@transcend-io/mcp-server-docs
@transcend-io/mcp-server-dsr
@transcend-io/mcp-server-inventory
@transcend-io/mcp-server-preferences
@transcend-io/mcp-server-workflows
commit: |
…dencies
Reject mixed lists so global-only configs stay as title strings, while any
per-workflow override uses a full object list including global `{ titles }`.
| * An empty `titles` list scoped to a workflow is an explicit override, meaning | ||
| * that workflow runs the data silo with no dependencies at all. | ||
| */ | ||
| export const DeletionDependencyGroup = t.intersection([ |
There was a problem hiding this comment.
Let's call this DeletionDependency instead of DeletionDependencyGroup
| * Used when the list includes any per-workflow override; global dependencies | ||
| * are written as `{ titles: [...] }` in that form. | ||
| */ | ||
| export const DeletionDependencyObject = t.union([DeletionDependencyGroup, DeletionDependencyReset]); |
There was a problem hiding this comment.
| export const DeletionDependencyObject = t.union([DeletionDependencyGroup, DeletionDependencyReset]); | |
| export const DeletionDependencyInput = t.union([DeletionDependencyGroup, DeletionDependencyReset]); |
| // The API rejects duplicate titles, which are easy to introduce across multiple | ||
| // global `{ titles }` entries |
There was a problem hiding this comment.
throw if we have multiple global { titles } entries.
|
|
||
| When an erasure request runs, `deletion-dependencies` holds off deleting from a data silo until the data silos it lists have finished deleting. | ||
|
|
||
| Use a list of titles when there are no per-workflow overrides: |
There was a problem hiding this comment.
Can we note this as deprecated? Follow other examples in the README for deprecated fields. If there are none, let me know.
| - CRM Warehouse | ||
| ``` | ||
|
|
||
| Once any override is present, use a list of objects for the whole field (global deps as `{ titles }`, overrides as `{ workflow, titles }` or `{ workflow, reset-to-global: true }`). Mixing titles and objects in the same list is not allowed: |
There was a problem hiding this comment.
Don't phrase this as "Once any override is present" - this will be the new way going forward and we want to phrase it as the proper way to use the CLI.
|
|
||
| A workflow that is not listed keeps whatever configuration it already has, so pushing a config that only lists global dependencies never removes existing overrides. Use `reset-to-global: true` to remove one. | ||
|
|
||
| Omitting `deletion-dependencies` entirely clears the global configuration, matching how the rest of `transcend.yml` treats omitted fields. |
There was a problem hiding this comment.
[Not for reviewers, not for Cursor] Even though it says this matches "how the rest of transcend.yml treats omitted fields" this does not seem like obvious behavior to me. If we do want to do it this way, I'm not sure that justifying it by saying "matching how the rest of transcend.yml treats omitted fields." is appropriate within the README.
| it('merges multiple global object entries into one', () => { | ||
| expect( | ||
| normalizeDeletionDependencies( | ||
| [{ titles: ['Identity Service'] }, { titles: ['CRM Warehouse'] }], | ||
| 'Salesforce', | ||
| ), | ||
| ).to.deep.equal([{ titles: ['Identity Service', 'CRM Warehouse'] }]); | ||
| }); |
There was a problem hiding this comment.
This should throw, not merge.
| it('deduplicates global titles across object entries', () => { | ||
| expect( | ||
| normalizeDeletionDependencies( | ||
| [{ titles: ['Identity Service'] }, { titles: ['Identity Service', 'CRM Warehouse'] }], | ||
| 'Salesforce', | ||
| ), | ||
| ).to.deep.equal([{ titles: ['Identity Service', 'CRM Warehouse'] }]); | ||
| }); |
There was a problem hiding this comment.
this should also throw
jefgodesky
left a comment
There was a problem hiding this comment.
Seems well-aligned at the product level, and the code LGTM, but I'll defer to @mason-hale for a proper, Waluigi-internal technical review.
Part 1 of 2 for WAL-10452. Part 2 adds the pull side and is stacked on this branch.
What
deletion-dependenciesis either a list of titles (global config only) or a list of objects (required once any per-workflow override is present). Mixing titles and objects in the same list is not allowed.Global only:
With overrides (whole list is objects; global becomes
{ titles }):A workflow that isn't listed keeps whatever it already has, so pushing a global-only config never silently wipes overrides.
Blocked on
Do not merge before https://github.com/transcend-io/main/pull/46818 deploys. Push now sends
dependedOnDataSilosinstead of the deprecateddependedOnDataSiloTitles, and that input field is not on staging yet, so merging early would breaktranscend inventory pushfor anyone using deletion dependencies at all.Notes
syncDataSiloDependencieschanges signature, hence the major bump on the SDK changeset.workflowmaps toworkflowConfigInternalName, so no ID lookup is needed on push.Test plan
pnpm run --dir packages/cli testpnpm run --dir packages/sdk testMade with Cursor