This is mutating the delta object right? I'm not keen on mutation - part of the reason the old engine was frustrating to work with was because of the mutation.

Yes we are overriding the logic to compose the delta for resources and mutating the delta before it is returned upstream. I understand mutations can become tedious though. To avoid mutations I would probably mark the members of these deltas as finals, and then compute the references on demand rather than binding it to the resource delta. I moved away from this as it meant that downstream from the preproc are responsible for choosing where to sample this physical id from given the logical id (before and after). But it's definitely a good alternative to consider.

I think I am happy with this as we mutate the delta for a resource type in the one function that computes it; and it's also true that all the logics upstream from this point only need this entity as read-only (the resource was executed and others may depend on its computed values).

Can I propose I attempt the previous design again in a follow-up branch for us to reevaluate?

After discussion we think this approach is ok as the mutation is really hidden from the caller, and is an implementation detail. A low effort alternative would be to treat delta objects as immutable and construct new ones but at this stage this change is not critical.

I worry that this might lead to us incorrectly getting the physical resource id for a resource that has been deleted. Should this not enforce that the physical id is of a resource in the current stack? What is the use case for getting the physical resource of an object that is no longer in the stack?

I think this should not happen. In a delta, deleted resources are represented as the current stack value in the before , and null values in the after. Therefore, we should never be able to sample the a physical id to assign it to the after of a resource delta, as the after of a delete resource delta does not need computing: it is null, doesn't exists (also in l77).
The executor may still record a duplicate of the deleted resource in the resources, which may be useful for the provider after execution (I'm not sure about this atm). Regardless, this is also fine for two reasons as far I can see as: (1) before definitions of resources are sampled strictly from before stores of resolved resources (as they should), and (2) even in the incorrect scenario that these are sampled from the 'current' or 'most updated value' (_current_resource_physical_id), the physical id can only be fore one for the resource being deleted.

I’ve excluded the physical ID from the delta’s comparison logic and now explicitly sample the physical resource ID from the after object of updated resources where necessary. This should make the intent behind which version of the physical ID is being used more clear in the code.

I'm personally not too fond of "hiding" these properties. What is the motivation here?

Here (and in general) I make member variables private when they exist solely to support the class’s (or extensions) internal logic, and exposing them externally has no semantic purpose. By restricting visibility this way, I see it helps maintain clear abstraction boundaries it is in an effort to adhere to standard practices in this context where sensible. That being said, I'd be happy to revise this if you feel it's too different from the rest of this area of code, or it is simply needed