feat(cli): pull per-workflow deletion dependencies into transcend.yml - #403
Conversation
@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: |
| * With no per-workflow overrides, global dependencies stay as a list of titles so existing | ||
| * configurations round-trip unchanged. When overrides exist, the whole field is a list of | ||
| * objects (`{ titles }` for global, `{ workflow, titles }` for each override). |
There was a problem hiding this comment.
This is not ideal long-term, but I think we have to make a choice in terms of which way to return the data (list of strings or list of objects) and I think this is best to be non-breaking. But long-term, we would want to transition users over to use the list of objects no matter what.
Maybe we have a flag that they pass in that tells us if they want a list of global-only dependencies to be returned as objects? But at some point code would have to be updated to add/remove that flag, so I'm not sure it's a huge benefit (for us or them) over supporting both.
mason-hale
left a comment
mason-hale
left a comment
There was a problem hiding this comment.
looks good except for one high severity finding cursor found after i did my manual review
| ); | ||
| const workflowConfigsById = hasWorkflowScopedDependencies | ||
| ? keyBy( | ||
| await fetchAllWorkflowConfigs(client, { |
There was a problem hiding this comment.
i wasn't seeing any issues so had cursor take a pass which found:
TR_PULL_RESOURCE_SCOPE_MAP for data silos is still only:
[TranscendPullResource.DataSilos]: [
ScopeName.ViewDataMap,
ScopeName.ViewDataSubjectRequestSettings,
ScopeName.ViewApiKeys,
],Fix: add ViewWorkflows to the data-silos pull (and push, if needed) scope map, and note it in comments like the preference-workflow entry.
There was a problem hiding this comment.
If we don't add ViewWorkflows to the data-silos pull scope map, a default transcend inventory pull can start failing for API keys that only have the currently documented data-silo scopes (ViewDataMap, ViewDataSubjectRequestSettings, ViewApiKeys). As soon as any silo has a per-workflow deletion-dependency override, pull calls workflows to resolve IDs → internal names, and that query is gated on ViewWorkflows. Orgs that pulled successfully before this change would hit an auth error once overrides exist, even though they never asked to pull workflow configs.
There was a problem hiding this comment.
Thanks for the callout! I think I would prefer to solve this differently - I was already on the fence about retuning the internalName alongside the id. It was responsible for the extra call to workflowConfigs which I wouldn't mind eliminating. If we don't make the extra call, we don't need the extra scope. So I decided to add it (PR here). I'll update this one once that is merged.
There was a problem hiding this comment.
Okay, this is updated now. A re-review would be appreciated!
The base branch was changed.
Reads `dependedOnDataSilosPerWorkflow` and writes each workflow that overrides the global dependencies as its own entry. Global dependencies keep the list-of-titles shorthand so existing configs round-trip unchanged. Overrides on workflows with no internal name are skipped with a warning, since transcend.yml references workflows by internal name.
Match the either/or codec: keep string[] for global-only configs, and write
global deps as `{ titles }` alongside overrides when any override exists.
c631cf2 to
c02170d
Compare
Read the internal name from dependedOnDataSilosPerWorkflow so pull no longer needs a separate workflowConfigs fetch (or ViewWorkflows scope).
3 participants
Part 2 of 2 for WAL-10452. Stacked on #402 — review that one first, and retarget this to
mainonce it merges.What
transcend inventory pullnow writes per-workflow deletion dependencies back intotranscend.yml, so a config with workflow overrides round-trips.{ titles }for global,{ workflow, titles }for each override)Overrides on a workflow with no internal name are skipped with a warning, because
transcend.ymlidentifies workflows by internal name and there would be nothing valid to write.Notes
fetchAllWorkflowConfigsto mapworkflowConfigIdback to an internal name, and only calls it when some silo actually has an override.dependedOnDataSilosPerWorkflow) is already live on staging, so unlike feat(cli): support per-workflow deletion dependencies in transcend.yml #402 this half isn't waiting on a backend deploy.Test plan
pnpm run --dir packages/cli testpnpm run --dir packages/sdk testMade with Cursor