Skip to content

[2/3] fix: protect background sessions and record resource history - #435

Closed
slashdevcorpse wants to merge 5 commits into
LodyAI:mainfrom
slashdevcorpse:fix/background-session-resource-history
Closed

slashdevcorpse wants to merge 5 commits into
LodyAI:mainfrom
slashdevcorpse:fix/background-session-resource-history

Conversation

@slashdevcorpse

@slashdevcorpse slashdevcorpse commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

Refs #429

Problem / pressure

Idle and memory-pressure eviction can discard a session after its parent turn finishes even when a background task or ACP terminal watch remains active. Current resource snapshots also disappear without a bounded record of process identity, resource use, or cleanup outcome.

Summary

  • Protect the latest pending/in-progress first-class background, subagent, and scheduled task states. Completed/failed states release that guard, and later resumptions protect it again.
  • Protect pending terminal starts and live ACP terminals until observed exit. Retained output from an exited terminal does not prevent eviction. History-read failures prevent eviction.
  • Retain up to 120 observed samples for ten minutes, with at most 256 attributed processes and 100 session rows per sample. Include PID/start identity, session association, CPU/memory metrics, cleanup state, truncation, and unavailable-sample receipts.
  • Keep sampling observer-driven and history in memory. The local machine/get-resource-history RPC reads the buffer without probing, rejects session-scoped access, and advertises resourceHistory v1. No command lines, environment values, or raw error strings enter history.

Replacement native stack

Native GitHub stack #6 in slashdevcorpse/Lody, created with gh stack link:

  1. [1/3] fix: bound Windows ACP shutdown and retain failed cleanup slashdevcorpse/Lody#2 — bounded shutdown and retained cleanup ownership; base main.
  2. [2/3] fix: protect background sessions and record resource history slashdevcorpse/Lody#3 — background-aware eviction and bounded resource history; base fix/windows-process-tree-cleanup.
  3. [3/3] fix: own Windows process trees with native job supervision slashdevcorpse/Lody#4 — native Windows Job Object ownership; base fix/background-session-resource-history.

Replaces the closed cross-fork #430 and #435. Each PR has an incremental diff against its predecessor. GitHub native stacks require all branches in the same repository, so this stack is in the contributor fork. It has not merged or landed upstream; #429 remains open.

Test plan

  • 132 focused tests passed across GC/background history, terminal liveness and handler integration, cleanup receipts, resource history/monitoring, local RPC, and shared schemas.
  • CLI and shared-package typechecks passed; changed TypeScript formatting, whitespace checks, and root pnpm check:quick passed.
  • Independent review identified the missing live-terminal guard; the fix now covers pending starts, concurrent failed startup, observed exit, retained output, and the real handler path.
  • History tests verify sample/process bounds, immutable receipts, observer gaps, cgroup/CLI metrics, unavailable probes, and PID reuse.
  • This does not prove real provider memory returns to baseline or expose provider-internal scheduler state. Raw CronCreate/ScheduleWakeup history is not treated as authoritative pending work or completed by elapsed time. Windows crash/process-lifetime verification belongs to part 3.
  • Full GitHub CI must be assessed at this PR head; part 1's head passed static, test, and desktop smoke checks.

Context handoff

Instructions for reviewing agents

  • Review focus: Trace idle/pressure eligibility through task history and observed terminal liveness; verify bounded immutable history and local RPC scope.
  • Decisions to challenge: Explicit pending task state protects sessions conservatively; history remains observer-driven and diagnostic PID attribution never authorizes killing.
  • Plausible failures / evidence gaps: Provider-internal schedules without first-class task updates remain unobservable; historical running snapshots may outlive provider execution. Initial root attribution is observational, with subsequent PID reuse rejected.

Authoring context

  • User goal / directives: Implement a three-PR process-lifecycle stack, with background-aware eviction and resource history as the second dependent PR.
  • Constraints / non-goals: Preserve unrelated work; avoid permanent OS probes, persisted diagnostics, raw process commands/environment, and inferred cron completion.
  • Risk-bearing decisions: Active work prevents pressure eviction even when memory is low; failed cleanup remains visible; strict limits deliberately truncate large observations.
  • Destructive or irreversible behavior: Existing GC termination remains the only destructive path in this slice; the new guards prevent termination and the history RPC only reads memory.
  • Deliberately not done or tested: No provider-internal scheduler API, all-provider residency claim, continuous background sampling, remote history transport, or real user-session termination.
  • Unknowns / confidence: Focused tests and source review support the scoped behavior. Native crash ownership and repeated real process lifetimes are covered by the third PR.

Copilot AI lite review requested due to automatic review settings September 6, 2026 05:06

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2afce75259

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread apps/cli/src/session/session.ts
Comment thread apps/cli/src/session/terminal-manager.ts
@slashdevcorpse

Copy link
Copy Markdown
Contributor Author

Closing to replace the cumulative main-targeted PR with the second layer of a native three-PR GitHub stack in slashdevcorpse/Lody. The implementation is preserved on fix/background-session-resource-history; replacement links will follow. #429 remains open.

@slashdevcorpse

Copy link
Copy Markdown
Contributor Author

Native GitHub stack #5 in slashdevcorpse/Lody, created with gh stack link:

  1. [1/3] fix: bound Windows ACP shutdown and retain failed cleanup slashdevcorpse/Lody#2 — bounded shutdown and retained cleanup ownership; base main.
  2. [2/3] fix: protect background sessions and record resource history slashdevcorpse/Lody#3 — background-aware eviction and bounded resource history; base fix/windows-process-tree-cleanup.
  3. [3/3] fix: own Windows process trees with native job supervision slashdevcorpse/Lody#4 — native Windows Job Object ownership; base fix/background-session-resource-history.

Replaces the closed cross-fork #430 and #435. Each PR has an incremental diff against its predecessor. GitHub native stacks require all branches in the same repository, so this stack is in the contributor fork. It has not merged or landed upstream; #429 remains open.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants