Add ExecPolicyManager

[pakrym-oai]

Move exec policy management into services to keep turn context immutable.

[bolinfest]

I think you can make this one line if you don't want to use the braces to drop the lock:

let codex_home = self.state.lock().await.session_configuration
                .original_config_do_not_use
                .codex_home
                .clone()

[bolinfest]

So this is a change in behavior? After append_amendment_and_update() is called, anyone who has a reference to the previous value will now have a stale value, right?

I don't know if that's what we want?

[pakrym-oai]

We are holding onto an instance of ExecPolicyManager in codex and create_exec_approval_requirement_for_command gets the latest snapshot and doesn't hold onto it.

Do you see a place that's affected by this behavior? Am I missing something?

[bolinfest]

append_execpolicy_amendment_and_update() returns a new Policy and the method is pub(crate).

I think you're saying the intention of ExecPolicyManager is to never expose the internal Policy directly (which I agree with), though (1) I think that's worth a comment, and (2) I think the visibility of append_execpolicy_amendment_and_update() needs to be changed to honor that.

Also, I haven't heard of ArcSwap before and I don't see existing uses in our codebase. Should we be using it instead of RwLock in more places? It seems weird to only use it here.

[pakrym-oai]

Should we be using it instead of RwLock in more places?

I looked for more usages but it only works when the value is immutable and most things we keep under RWLock are actually mutable.

I see your point about append_execpolicy_amendment_and_update now!

[pakrym-oai]

inlined append_execpolicy_amendment_and_update into the policy manager and implemented Default to make tests cleaner.