Skip to content

perf(cli): reduce Windows process probes during session shutdown - #2046

Open
techotaku39 wants to merge 3 commits into
tiann:mainfrom
techotaku39:perf/windows-process-probes
Open

techotaku39 wants to merge 3 commits into
tiann:mainfrom
techotaku39:perf/windows-process-probes

Conversation

@techotaku39

Copy link
Copy Markdown
Contributor

Summary

Windows session shutdown repeatedly launches PowerShell/CIM and tasklist probes for the same process tree, adding seconds to archive requests.

  • Build the process tree from one CIM enumeration and batch generation and exit observations per shutdown phase.
  • Preserve immediate generation rechecks before signalling survivors and fall back to individual checks when a snapshot is unavailable. Unknown process state does not count as a verified exit.
  • Avoid adding bulk generation snapshots for single-process trees, and cover parent cycles and the full Windows PID range.

Validation

  • From cli/, ran the following command: 154 tests passed, including generation changes, stale or unavailable snapshots, unknown identities, single-process cleanup, and actual Windows PowerShell query execution.

    bunx vitest run src/codex/shared/registry.test.ts src/utils/process.test.ts src/runner/orphanReap.test.ts src/runner/sharedSessionStop.test.ts src/runner/buildCliArgs.test.ts src/utils/windowsProcessTreeQuery.test.ts
  • bun run typecheck — passed.

  • bun run build and bun run --cwd cli build:exe:allinone — passed.

  • bun run test:e2e -- terminal-wrap-fidelity.spec.ts, executed through the isolated task wrapper — 2 passed.

  • Local packaged Windows acceptance, using an isolated Hub, Runner, fresh database and mock model:

    Invoke-HapiTaskPlaywright.ps1 -Name windows-process-probe-perf -Suite Root --config playwright.process-perf.config.ts

    2 passed using a workspace-local acceptance harness:

    • Three fresh create/reply/archive cycles returned HTTP 200 with verified process-tree exits.
    • With 245 runtime records, archiving one shared session preserved its sibling’s process and subsequent reply; model discovery and Runner requests remained available.

All validation above ran on #2035 plus this patch, without #2036.

Related Issues

Depends on #2035.

AI Assistance

Implemented and validated with assistance from OpenAI Codex (GPT-6).

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review mode: initial

Requirement — Pass

Reducing repeated Windows process probes addresses a supported shutdown bottleneck while preserving process-tree termination and shared-session protection.

Evidence

  • The snapshot PR description identifies archive latency from repeated PowerShell/CIM and tasklist probes.
  • Base cli/src/utils/process.ts:230 queries CIM for each parent; :406 and :418 perform individual generation and exit probes.

Approach — Pass

One tree enumeration and fresh snapshots per shutdown phase reduce probe counts. Generation rechecks, conservative handling of unknown identities, and individual fallback checks preserve the termination safeguards.

Evidence

  • cli/src/utils/process.ts:259 builds parent links from one CIM enumeration.
  • cli/src/utils/process.ts:482 batches generation observations; :517 rechecks survivors before signalling; :549 distinguishes PID reuse from unknown identity.
  • cli/src/runner/run.ts:1127 reads registry records before obtaining a fresh process snapshot for sibling protection.

Code — Reviewed

Reviewed the entire fixed-head merge-base diff and relevant callers. No concrete introduced defects found. Runtime behavior was assessed statically without executing PR code.

No reportable code issues found.

Testing

Not run (automation; PR code execution is prohibited).

  • At inspection, fixed-head integration CI passed; test and windows-codex-mcp jobs remained in progress.
  • Added coverage addresses snapshot validation, PID reuse, unknown generations, individual fallback behavior, single-process cleanup, parent cycles, and large Windows PIDs.
  • The PR reports 154 focused tests, typecheck/build, and Windows acceptance checks passing. These results were not independently reproduced; no tests or scripts were run during review.

HAPI Bot

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review mode: follow-up after new commits

Requirement — Pass

Reducing repeated Windows process probes addresses shutdown latency while preserving verified process-tree termination and shared-session protection.

Evidence

  • The snapshot PR description identifies repeated PowerShell/CIM and tasklist probes as an archive bottleneck.
  • Base cli/src/utils/process.ts:230 enumerates children separately for each parent; :406 captures generation markers individually.

Approach — Pass

A single tree enumeration and fresh snapshots per shutdown phase reduce probe costs. Individual fallback checks, generation rechecks before signalling survivors, and conservative handling of unknown identities preserve the relevant safeguards.

Evidence

  • cli/src/utils/process.ts:259 builds parent links from one enumeration; :482 batches generation observations; :517 rechecks survivors before signalling; :549 distinguishes generation changes from unknown identities.
  • cli/src/runner/run.ts:1127 reads registry records before obtaining a fresh snapshot for runtime liveness and sibling protection.

Code — Reviewed

Reviewed the entire fixed-head merge-base diff and relevant surrounding code, including the updated PowerShell fixtures. No concrete introduced defects found. Runtime behavior was assessed statically without executing PR code.

No reportable code issues found.

Testing

Not run (automation; PR code execution is prohibited).

  • At inspection, fixed-head integration and windows-codex-mcp CI checks passed; the test check remained in progress.
  • Added coverage addresses snapshot validation, unknown generations, PID reuse, individual fallback, single-process cleanup, parent cycles, and large Windows PIDs.
  • The PR reports 154 focused tests, typecheck/build, and Windows acceptance checks passing on #2035 plus this patch. These results were not independently reproduced; no tests, builds, or scripts were executed during review.

HAPI Bot

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant