[procmgr] Windows spawn profiles foundation - #54731
Conversation
jose-manuel-almaza
added
the
qa/done
dd-octo-sts
Bot
added
internal
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: b27edf4195
ℹ️ 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".
This comment has been minimized.
This comment has been minimized.
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 92f6b05 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_security_no_fs_load | memory utilization | +0.45 | [+0.30, +0.60] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +0.13 | [+0.03, +0.24] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.01 | [-0.13, +0.11] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.05 | [-0.09, -0.01] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.23 | [-0.30, -0.16] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | -0.41 | [-0.53, -0.28] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.59 | [-0.83, -0.35] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -1.41 | [-2.28, -0.54] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 173.26MiB ≤ 178MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 745.01KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 516.47MiB ≤ 538MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.13MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 17 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 205.34MiB ≤ 229MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.79MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 360.48 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 408.62MiB ≤ 439MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.94GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 72.06MiB ≤ 76MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.43 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 325.69MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 60.28 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 303.79MiB ≤ 314MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.39 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 312.34MiB ≤ 343MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bef4144e47
ℹ️ 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".
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
18 successful checks with minimal change (< 2 KiB)
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dcf3ac4fb
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e2715b142
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcc2e35663
ℹ️ 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".
On job-assignment or ResumeThread failure, terminate via the retained process handle, block until exit is observed, then clear spawn resources so UnloadUserProfile does not run while the child still exists.
Align restart-policy log assertion with the current message format and resolve rustfmt/clippy failures on the spawn-profiles branch.
Replace request_stop/wait_for_stop with a single stop() that sends graceful shutdown, force-kills on timeout, and synchronously drains Windows job members before unloading profiles. Use ProcessState::Stopping instead of stop_requested and remove deferred job-drain machinery from the manager.
Restructure stop() with an inline state match, extract wait_for_stop_exit and shared force-kill helpers, and add debug timing logs for stop duration.
Use proc_handle and watcher_handle so the ProcessHandle and JoinHandle are not both named handle.
Skip exit wait when TerminateProcess or job terminate fails, and cap the wait at FORCE_KILL_TIMEOUT so spawn abort cannot block startup or reload under the manager write lock.
These helpers are only referenced by agent-profile tests landing in the stacked list/describe PR; keeping them here fails golangci-lint unused.
Extract remove, apply, and reconcile reload phases into ProcessManager methods so handle_reload_config reads as a short orchestration step. Also drop an unused hyper_util import from grpc integration tests.
Clone the process name and capture pid before abort_before_supervision consumes SuspendedChild, so clear_windows_spawn_resources can run on failure paths. Remove unused named_pipe imports that fail Clippy on Windows CI.
Drop the reload-time clear_secret_caches hook and config_gate stub from spawn-profiles; secret-backend-gates owns cache invalidation.
Drop the config_gate stub module and condition_config_any wiring from spawn-profiles; config-gates owns gate parsing and evaluation.
Log unmet path conditions from condition_path_exists_met and have may_auto_start delegate to start_conditions_met after the auto_start check.
PartialEq already compares &ProcessConfig with ProcessConfig directly.
Separate process state from the daemon event loop so handlers take a single RuntimeHandles bundle and reload logic lives in its own module.
Keeps the manager module split intuitive: process_manager.rs owns the struct and handlers, while mod.rs retains shared helpers and tests.
Move recompute_startup_order into process_manager.rs since only ProcessManager uses it; mod.rs keeps cross-module spawn/reload helpers.
Apply rustfmt to the manager split and remove Windows-only dead code that failed clippy with -D warnings.
Agent-profile spawn needs dd-procmgr-service as LocalSystem to read the agent password from LSA. Re-add the test in #54735 with the Wix change.
Re-add the test dropped from #54731. Agent-profile spawn for PAR requires dd-procmgr-service as LocalSystem (Wix change in this PR) to read the agent password from LSA.
750b479 to
a5ef62f
Compare
…les PR Restore procmgr_win_test.go to main and drop the extracted helpers file. The only E2E delta on this PR is removing the PAR test for #54735.
Re-add the test dropped from #54731. Agent-profile spawn for PAR requires dd-procmgr-service as LocalSystem (Wix change in this PR) to read the agent password from LSA.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de957c68d
ℹ️ 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".
Split stop() into request_stop and wait_for_stop so shutdown_ordered signals every child before waiting, bounding total delay to the slowest process instead of the sum of per-process stop timeouts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d7171359
ℹ️ 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".
When a manually started auto_start=false process crashes during a restart delay, reload now re-evaluates restart policy and respawns if still allowed. Capture pre-reload context before set_config and skip bootstrap retries only when auto_start was toggled off.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f370c8a806
ℹ️ 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".
| if proc.state() == ProcessState::Failed { | ||
| return should_retry_failed_after_reload(proc, ctx); |
There was a problem hiding this comment.
When a process exits successfully under restart: on-success or restart: always and its configuration changes during the restart delay, its state is Exited, so this Failed-only branch does not start a replacement. The queued retry is then discarded because its configuration generation is stale, leaving the process down despite the current restart policy; re-evaluate both Exited and Failed states during reload.
AGENTS.md reference: AGENTS.md:L190-L192
Useful? React with 👍 / 👎.
2 participants
What does this PR do?
Introduces Windows spawn profiles in dd-procmgr so managed children can run under different security contexts:
ddagentuser)Adds the Windows spawn stack (token logon, user profile load, supervision job, suspended
CreateProcessAsUserW), COAT catalog enforcement for allowed profiles, and gRPC pipe caller authentication.Stack context: PR 1/4 split from #53249. Config gates, secret backend resolution, and process-agent dual-mode integration land in follow-up PRs (
jose/procmgr-config-gates,jose/procmgr-secret-backend-gates,jose/procmgr-windows-process-agent). This PR includes a stubconfig_gatemodule (gates always open) so the crate compiles until PR 2.Motivation
Process-agent on Windows must run as LocalSystem and other agent children should stay on the agent user. Spawn profiles make that explicit and enforceable at spawn time, and are a prerequisite for moving process-agent supervision off legacy SCM in later PRs.
Describe how you validated your changes
pkg/procmgr/coat/...)test/new-e2e/tests/agent-runtimes/procmgr/...)Additional Notes