Skip to content

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

Closed
slashdevcorpse wants to merge 3 commits into
fix/windows-process-tree-cleanupfrom
fix/background-session-resource-history
Closed

[2/3] fix: protect background sessions and record resource history#3
slashdevcorpse wants to merge 3 commits into
fix/windows-process-tree-cleanupfrom
fix/background-session-resource-history

Conversation

@slashdevcorpse

@slashdevcorpse slashdevcorpse commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Related issue

Refs LodyAI#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

  • Inspect goals and background work from one history read per eligibility check. Protect pending/in-progress task state, canonical CronCreate/ScheduleWakeup records until explicit completion/cancellation, and pending/live ACP terminals. Elapsed fire times do not imply completion; stale task history does not pin an absent runtime.
  • Capture runtime/history identity, metadata version and activity across awaited checks. Recheck immediately before termination, then hold dispatch, execution and manager admission through document teardown and transient-state deletion. Queued RPC/meta work resumes against fresh state; direct start/continue/steer waits before document access.
  • Isolate unreadable-session errors and skip changed sessions without counting an eviction. Coalesce document destruction; failed unloads remain retryable, and reopening cannot return a partially destroyed wrapper or let an old unload invalidate its replacement.
  • Retain up to 120 observed samples for ten minutes, with at most 256 attributed processes and 100 session rows per sample. Record resource use, cleanup status, truncation and unavailable-sample receipts in bounded memory.
  • Pin Windows per-process attribution to precise creation-identity strings. Omit session process rows on POSIX where coarse lstart values cannot establish identity; aggregate resource estimates remain available.
  • Keep sampling observer-driven. The local machine/get-resource-history RPC reads without probing, rejects session-scoped access and advertises resourceHistory v1. Strict nested schemas reject unsupported statuses/fields; command lines, environment values and raw error strings never enter history.

Stack order

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

  1. [1/3] fix: bound Windows ACP shutdown and retain failed cleanup #2 — bounded shutdown and retained cleanup ownership; base main.
  2. [2/3] fix: protect background sessions and record resource history #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 #4 — native Windows Job Object ownership; base fix/background-session-resource-history.

Replaces the closed cross-fork LodyAI#430 and LodyAI#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; LodyAI#429 remains open.

Test plan

  • Final combined run: 223 tests passed across GC, real handler integration, session/execution managers, dispatch admission, real document unload/reopen, terminal liveness and monitoring.
  • Additional focused watcher suites and document lifecycle suites passed. Regressions cover metadata/RPC arrival during preview cleanup, pending direct execution, teardown waits, failed unload retry, concurrent reopen, preserved replacements and per-session inspection failure.
  • CLI/shared typechecks, changed TypeScript formatting, whitespace checks and root check:quick passed (zero lint errors and all boundary guards). Resource-history schemas and the resourceHistory capability registration expectation are covered.
  • Resource-history tests cover bounds, immutable receipts, missing samples, strict nested privacy fields, status validation and same-millisecond Windows identity changes. POSIX coarse timestamps do not authorize per-session process attribution.
  • GitHub Static checks, Tests and Desktop E2E passed at f291ca2. Real-provider memory recovery and unobserved provider-internal task completion remain outside this evidence; Windows crash ownership is implemented in PR [3/3] fix: own Windows process trees with native job supervision #4.

Context handoff

Instructions for reviewing agents

  • Review focus: Trace idle/pressure eligibility through history, metadata and terminal liveness, then follow all admission leases through document destruction and fresh reopen. Verify bounded strict resource 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: Unobserved provider-internal completion remains unknown, so live task state stays conservatively protected. Metadata-version changes can defer cleanup. Windows process identity is observational; coarse POSIX session rows are omitted.

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; admission leases hold through cleanup and release deferred work afterward. Failed unloads stay retryable; strict history limits 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.

Latest review verification

At f291ca2, the stack includes the eight latest PR #2 review resolutions from 720e84c: PTY admission/draining and retry retention, authentication cleanup ownership, startup owner admission, spawn-error handling, explicit Windows termination semantics and non-retryable cleanup failures. PR #4 supplies the requested descendant Job Object ownership. Local composed verification and exact-head GitHub Static checks, Tests and Desktop E2E all passed. The top PR also passed Windows x64, Windows ARM64 and the same-run dual-architecture npm archive verification.

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

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.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 9bbb72ad-26c7-494c-9528-21def8aa636c


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 23 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread apps/cli/src/lib/session-gc-manager.ts Outdated
Comment thread packages/shared/src/machine-monitor.ts Outdated
Comment thread packages/shared/src/machine-monitor.ts Outdated
Comment thread apps/cli/src/lib/message-handler.ts
Comment thread apps/cli/src/monitor/process-tree.ts Outdated
Comment thread apps/cli/src/lib/session-gc-manager.test.ts Outdated
Comment thread apps/cli/src/lib/session-gc-manager.ts Outdated
Comment thread apps/cli/src/lib/session-background-work.ts Outdated
@slashdevcorpse
slashdevcorpse force-pushed the fix/background-session-resource-history branch from 2afce75 to bee57e6 Compare September 6, 2026 06:14
@slashdevcorpse
slashdevcorpse force-pushed the fix/background-session-resource-history branch from bee57e6 to f291ca2 Compare September 6, 2026 06:44
@slashdevcorpse

Copy link
Copy Markdown
Owner Author

Superseded by upstream PR LodyAI#457. The series now lives in LodyAI/Lody as LodyAI#456 -> LodyAI#457 -> LodyAI#458. Source branches and review history are preserved.

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