Regression in the way that conflicts are reported by `force:source:push --json`

[SCWells72]

For the past several years, conflicts have been reported reliably by force:source:push by specifying a name value of sourceConflictDetected in the failureResponse, all part of the JSON response written to stdout. Suddenly I'm seeing it being reported differently as follows:

$ sfdx force:source:push --json > force_source_push.out 2> force_source_push.err

$ echo $?
1

$ cat force_source_push.out
{
  "name": "DeployFailed",
  "actions": [],
  "exitCode": 1,
  "data": [],
  "result": [],
  "commandName": "Push",
  "warnings": [
    "We plan to deprecate this command in the future. Try using the \"project deploy start\" command instead."
  ],
  "message": "Push Failed.",
  "stack": "DeployFailed: Push failed. \n    at PushResultFormatter.getJson (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\formatters\\source\\pushResultFormatter.js:37:27)\n    at Push.catch (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\commands\\force\\source\\push.js:165:23)\n    at Push._run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\command.js:112:29)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async Config.runCommand (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\config\\config.js:328:25)\n    at async run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\main.js:89:16)",
  "context": "Push",
  "status": 1
}

$ cat force_source_push.err
    sourceConflictDetected: We couldn't complete the operation due to
    conflicts. Verify that you want to keep the existing versions, then run
    the command again with the --forceoverwrite (-f) option.
    Code: sourceConflictDetected

This is not only a breaking change overall, but important information about the result is being written to stderr instead of being part of the structured JSON response written to stdout.

I can certainly update to accommodate for this, but it is a breaking change to a long-standing contract for reporting of conflicts, and a more worrying change to the way these CLI commands actually report their structured output.

[github-actions[bot]]

Thank you for filing this issue. We appreciate your feedback and will review the issue as soon as possible. Remember, however, that GitHub isn't a mechanism for receiving support under any agreement or SLA. If you require immediate assistance, contact Salesforce Customer Support.

[SCWells72]

For context, I have the following comment in my Salesforce CLI command executor:

// All JSON-formatted responses--success or error--are sent to stdout now, and stderr is used for free-form messages

that was added with the following commit comment on February 9, 2019:

Spring '19 - Updated SFDX CLI execution to handle JSON-formatted responses only on stdout, whether success or failure, and low-level errors/informational text on stderr.

Please tell me that you're not straying way from that long-standing rule of how CLI commands provide their responses...

[SCWells72]

You'll note that the response also no longer includes the specifics about the actual conflicts which is another regression. Previously all metadata objects which were found to be in conflict were included with a value of Conflict for state.

[SCWells72]

I have verified that force:source:pull still behaves as it has in the past. Here is the output of that command with the expected/proper information written to stdout and stderr (in this case, stderr is empty):

$ sfdx force:source:pull --json > force_source_pull.out 2> force_source_pull.err

$ echo $?
1

$ cat force_source_pull.out
{
  "data": [
    {
      "state": "Conflict",
      "fullName": "Account-de",
      "type": "CustomObjectTranslation",
      "filePath": "C:\\Users\\Scott\\dev\\projects\\ic-test-projects\\NPSP\\force-app\\main\\default\\objectTranslations\\Account-de\\Account-de.objectTranslation-meta.xml"
    },
    ...
  ],
  "code": 1,
  "message": "We couldn't complete the operation due to conflicts. Verify that you want to keep the existing versions, then run the command again with the --forceoverwrite (-f) option.",
  "name": "sourceConflictDetected",
  "status": 1,
  "stack": "sourceConflictDetected: We couldn't complete the operation due to conflicts. Verify that you want to keep the existing versions, then run the command again with the --forceoverwrite (-f) option.\n    at processConflicts (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\trackingFunctions.js:104:17)\n    at trackingSetup (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\trackingFunctions.js:36:9)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async Pull.preChecks (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\commands\\force\\source\\pull.js:41:25)\n    at async Pull.run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@salesforce\\plugin-source\\lib\\commands\\force\\source\\pull.js:27:9)\n    at async Pull._run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\command.js:108:22)\n    at async Config.runCommand (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\config\\config.js:328:25)\n    at async run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.197.8-077a68b\\node_modules\\@oclif\\core\\lib\\main.js:89:16)",
  "exitCode": 1,
  "warnings": [
    "We plan to deprecate this command in the future. Try using the \"project retrieve start\" command instead."
  ]
}

$ cat force_source_pull.err
<nothing>

Note that value for name as sourceConflictDetected and the list of actual conflicting metadata files with a state of Conflict. The force:source:push command should behave identically.

[shetzel]

@SCWells72 - thanks for reporting Scott. I'll fix this asap. I expect to get it into a nightly as well as the next CLI release candidate.

[git2gus[bot]]

This issue has been linked to a new work item: W-13100374

[SCWells72]

Thanks, @shetzel. I've implemented a workaround--at least to detect the conflict state even if the conflict details aren't present--for the next IC2 build (this Thursday morning). Once the original behavior is restored, it should use it automatically.

This is obviously another in a growing string of such regressions. I just asked @mshanemc this, but I'll add the same thought here. Would it be helpful for me to provide a list of the commands executed by IC2 for JSON output and the expected contract (exit codes, JSON shape and contents, etc.) based on historical behavior? You all could incorporate validation for those expected contracts into your automated test suite. That would probably take me a bit to put together--hopefully just hours and not days--but if it would save me time later (and my users grief), I think it might be worth it.

[shetzel]

@SCWells72 - it's a CLI policy to never break command JSON output shape as it's an API contract (caveat: additions are ok but not removals or change in shape). We have lots of tests across all our commands that have some JSON output coverage but it's difficult to cover all the usecases, especially all the variations that can happen with the most popular commands, deploy and retrieve (and all their variants). That said, obviously we missed some.

To answer your question about "Would it be helpful..." - yes, it would but it's also something we should already have and I would not expect that from any customer. I'll create a story to improve command JSON output test coverage. If you want to provide usecases you certainly can, but don't feel obligated.

BTW, the fix for this is being tested now. I expect it will be in the nightly tonight so I'll promote it to the CLI RC tomorrow.

[SCWells72]

Thanks, @shetzel. I certainly appreciate the policy, but as evidenced by the recent regressions in the JSON CLI-as-an-API interface, there must be some gaps in the current test coverage relative to at least some downstream usage patterns, (from my perspective, notably those used by IC2). And don't get me wrong, I understand that you all have been implementing some HUGE changes in the move to open source, the new command naming conventions, the introduction of the sf CLI,, etc., so some regressions are going to happen. If I can share some information that not only helps to minimize them in the future, but specifically to minimize the ones that might impact me (and therefore my users), it's time well-spent.

Yesterday I created a document that covers all of the CLI commands that IC2 executes (well, except for the Salesforce Functions ones that don't really have a JSON interface anyway) with the args passed and the Java POJOs into which the responses are deserialized. I emailed that document to @mshanemc, so you should be able to get it from him, or if you need, just let me know and I can provide directly to you. The one thing that may be missing from would be details about expected values for some of the properties of those POJOs, e.g., Conflict as the value for state when a conflict is reported. I'm happy to provide that additional detail if you have questions upon reviewing what I've sent. Just let me know.

[shetzel]

This fix is in the current sfdx cli release candidate, v7.199.7

[SCWells72]

Hey, folks. Would you expect this to be fixed in the GA builds? I ask because I'm still seeing it. Here's an example from just now:

{
  "status": 1,
  "failureResponse": {
    "result": [],
    "partialSuccess": [],
    "stderr": "(node:3020) Warning: Deprecated environment variable: SFDX_AUTOUPDATE_DISABLE. Please use SF_AUTOUPDATE_DISABLE instead.\r\n(Use `node --trace-warnings ...` to show where the warning was created)\r\n(node:3020) Warning: Deprecated environment variable: SFDX_JSON_TO_STDOUT. Please use SF_JSON_TO_STDOUT instead.\r\n",
    "status": 1,
    "exitCode": 1,
    "message": "We couldn\u0027t complete the operation due to conflicts. Verify that you want to keep the existing versions, then run the command again with the --forceoverwrite (-f) option.",
    "name": "sourceConflictDetected",
    "commandName": "Push",
    "stack": "sourceConflictDetected: We couldn\u0027t complete the operation due to conflicts. Verify that you want to keep the existing versions, then run the command again with the --forceoverwrite (-f) option.\n    at processConflicts (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@salesforce\\plugin-source\\lib\\trackingFunctions.js:104:17)\n    at trackingSetup (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@salesforce\\plugin-source\\lib\\trackingFunctions.js:36:9)\n    at async Push.prechecks (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@salesforce\\plugin-source\\lib\\commands\\force\\source\\push.js:38:25)\n    at async Push.run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@salesforce\\plugin-source\\lib\\commands\\force\\source\\push.js:31:9)\n    at async Push._run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@oclif\\core\\lib\\command.js:117:22)\n    at async Config.runCommand (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@oclif\\core\\lib\\config\\config.js:329:25)\n    at async run (C:\\Users\\Scott\\AppData\\Local\\sfdx\\client\\7.201.6-4edd82d\\node_modules\\@oclif\\core\\lib\\main.js:89:16)",
    "actions": [],
    "warnings": [
      "We plan to deprecate this command in the future. Try using the \"project deploy start\" command instead.",
      "The \"-u\" flag has been deprecated. Use \"--target-org | -o\" instead."
    ]
  }
}

Notice that the name is properly set to sourceConflictDetected, but there are no specific files listed as having a state of Conflict. So while it appears to be partially resolved, it doesn't appear to be completely resolved, at least not in providing the end user a good picture of what exactly is in conflict.

CLI version info:

$ sfdx version
sfdx-cli/7.201.6 win32-x64 node-v18.15.0
$ sf version
@salesforce/cli/1.79.0 win32-x64 node-v18.15.0

[shetzel]

Hi @SCWells72 I'm using sf version @salesforce/cli/1.82.0 darwin-arm64 node-v18.15.0. I changed an apex class in the org and locally, then ran sf force source push --json and the response had a data property that contained the specific metadata conflicts.

My sfdx version is 7.204.6 and it has the same json response when I run the push command.

If you update the cli do you see a good response?

[SCWells72]

@shetzel, I'm on slightly older versions, but they seem to be the latest released versions as they're the result of running sfdx update and sf update immediately before responding here:

$ sfdx version
sfdx-cli/7.203.6 win32-x64 node-v18.15.0
$ sf version
@salesforce/cli/1.81.0 win32-x64 node-v18.15.0

I just tried to reproduce it a few different ways and wasn't able to, but I've been seeing it pretty often over the past week. Given the recent frequency, I'm assuming it will happen again during my normal work/testing this coming week--at least if it will still happen--and will let you know as soon as it does exactly what I was doing. If I make it through the next week without it happening, that's perhaps a good sign that whatever changed in this most recent update addressed it. I'll keep you posted.

[SCWells72]

Okay, I found the issue. Historically the details in the push failure response were in an array called result, but since being restored, they're in an array called data. So IC2 is losing them during JSON deserialization. I can obviously work around that by adding data as an "alias" for result, but that still seems like a regression. Thoughts?

[SCWells72]

@shetzel, for context, this is from the Java POJO into which IC2 deserializes failed push JSON responses:

public class SfdxForceSourcePushSuccessResponse {
    public SfdxForceSourcePushResult result;

    public static class SfdxForceSourcePushResult {
        public List<SfdxForceSourcePushPushedSource> pushedSource = new LinkedList<>();
    }

and that's unchanged since 6/29/2017. If I rename result to data everything works fine against the current CLI, but again, that seems to be a regression in the API contract for this command's output, no?

[shetzel]

I was going by the pull example you provided in your April 25 comment and was hoping the data property would suffice. I'll add result back. Thanks for pointing that out.

[SCWells72]

@shetzel, it's totally up to you. I've already patched IC2 to allow for both. I don't know if others might already have adjusted for this change and might be frustrated by having it revert. Perhaps before you make any further changes you might poll other integrators? Don't know if you have a direct line to them or not.

[shetzel]

I wouldn't replace data with result. I would just have both. Though, if you don't need the result property I'll leave it as is.

[SCWells72]

I wouldn't populate both. In a situation where there are numerous reported errors/conflicts, that could result in quite a large JSON document. I've updated things to work with data or result depending on which is present. I'm good to go. Not sure about others who might be consuming that same output, but my guess is that if you haven't heard from them so far...you're probably okay.

[SCWells72]

@shetzel, note that the same issue is present for force:source:pull. That's why you used data as a reference point...because I included the output of the modified version of that command. Traditionally both commands have used result, and now both use data. I've updated IC2 to allow for both list property names in both the push and pull commands, but anyone relying on the original result list property name is going to miss the details in both commands.