fix: preserve truthful process and session state

[heinbrian]

What changed

• treat EPERM from process.kill(pid, 0) as proof that the process exists
• remove CLI activity only when the process probe returns ESRCH
• reconcile persisted tab state with live managed-process and CLI activity
• preserve unknown session state in the CLI instead of coercing it to idle

Why

Read-only and sandboxed monitors can receive EPERM while probing a live
Maestro process. The existing catch-all behavior reported stale discovery,
removed valid CLI activity, and exposed working tabs as idle. That made health
and recovery decisions depend on a permission failure rather than live state.

Impact

Process-restricted monitors no longer turn EPERM into a false crash. Session
list output reports busy when a matching managed process or active CLI job is
present, and preserves uncertainty when neither persisted nor live evidence is
conclusive.

Validation

• four focused Vitest files: 149 tests passed
• tsc -p tsconfig.main.json --noEmit
• tsc -p tsconfig.cli.json --noEmit
• Prettier check passed for all nine changed files
• targeted ESLint completed with no errors

This PR changes source and tests only. It does not modify an installed Maestro
application or request a release.

Summary by CodeRabbit

• New Features

  • Session listings now preserve and display an unknown state instead of incorrectly showing it as idle.
  • Session activity now reflects live desktop processes and CLI activity more accurately.

• Bug Fixes

  • Improved detection of active CLI sessions when process checks encounter permission errors.
  • Prevented active sessions from being incorrectly cleared due to transient or unexpected process-check errors.
  • Improved CLI server detection for running processes with restricted permissions.

• Tests

  • Added coverage for session states, live activity detection, and process-check error handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4b79a13b-4f2c-4b94-bf44-b9f7f724e5bd

📥 Commits

Reviewing files that changed from the base of the PR and between c8ccdf5 and c6a8c0f.

📒 Files selected for processing (9)

• src/__tests__/cli/commands/session.test.ts
• src/__tests__/main/web-server/web-server-factory.test.ts
• src/__tests__/shared/cli-activity.test.ts
• src/__tests__/shared/cli-server-discovery.test.ts
• src/cli/commands/session.ts
• src/main/web-server/types.ts
• src/main/web-server/web-server-factory.ts
• src/shared/cli-activity.ts
• src/shared/cli-server-discovery.ts

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

Desktop session states now include unknown. Session listing reconciles persisted state with managed processes and CLI activity. Process probes distinguish missing, permission, and unknown errors. Tests cover state derivation, rendering, and liveness behavior.

Changes

Desktop session state handling

Layer / File(s)	Summary
Session state contract and rendering src/main/web-server/types.ts, src/cli/commands/session.ts, src/__tests__/cli/commands/session.test.ts	Desktop session entries now support unknown. Text output preserves the entry state instead of mapping it to idle.
Process liveness error handling src/shared/cli-activity.ts, src/shared/cli-server-discovery.ts, src/__tests__/shared/*	EPERM, ESRCH, and unknown process-probe errors now produce distinct activity and cleanup behavior.
Desktop session state reconciliation src/main/web-server/web-server-factory.ts, src/__tests__/main/web-server/web-server-factory.test.ts	Session listing marks tabs busy for active managed processes or CLI activity, while retaining explicit idle states and reporting unknown states when evidence is unavailable.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c6a8c

The PR corrects process and session-state reporting for permission-limited monitors, with focused tests and standard checks passing; no actionable merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant listDesktopSessionsCallback
  participant processManager
  participant isSessionBusyWithCli
  listDesktopSessionsCallback->>processManager: Check managed tab process
  processManager-->>listDesktopSessionsCallback: Return process state
  listDesktopSessionsCallback->>isSessionBusyWithCli: Check CLI activity
  isSessionBusyWithCli-->>listDesktopSessionsCallback: Return activity state
  listDesktopSessionsCallback-->>listDesktopSessionsCallback: Emit idle, busy, or unknown state

Loading

Possibly related PRs

• RunMaestro/Maestro#923: Introduced the session-listing functionality extended by this change.
• RunMaestro/Maestro#1319: Related to shared session and tab busy-state handling.
• RunMaestro/Maestro#1340: Related to treating EPERM as evidence of a live process.

Suggested reviewers: pedramamini

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the changes to preserve accurate process and session state.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

The PR makes process probes distinguish permission failures from missing processes and reconciles persisted tab state with live process evidence.

• Treats EPERM as evidence that a probed process exists.
• Cleans up CLI activity only after an ESRCH result.
• Reports live managed-process or agent-wide CLI activity as busy.
• Preserves unknown session state in the WebSocket response and CLI output.
• Adds focused coverage for permission errors and state reconciliation.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

The changed process probes preserve live state on permission failures, cleanup remains limited to confirmed missing processes, and all in-repository consumers handle the expanded session-state contract consistently.

Important Files Changed

Filename	Overview
src/shared/cli-activity.ts	Distinguishes EPERM, ESRCH, and inconclusive probe failures so only confirmed missing processes cause activity cleanup.
src/shared/cli-server-discovery.ts	Treats EPERM as process-existence evidence while leaving authenticated connection establishment to determine reachability.
src/main/web-server/web-server-factory.ts	Reconciles persisted tab state with managed-process and agent-wide CLI activity, preserving unknown when evidence is inconclusive.
src/main/web-server/types.ts	Extends the desktop session response contract with the new unknown state.
src/cli/commands/session.ts	Preserves the state supplied by the desktop instead of coercing every non-busy state to idle.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Persisted tab state] --> D[Session-state reconciliation]
    B[Managed process entry] --> D
    C[CLI activity and PID probe] --> D
    C --> E{Probe result}
    E -->|alive or EPERM| F[CLI activity is busy]
    E -->|ESRCH| G[Remove stale activity]
    E -->|other error| H[Preserve activity without claiming busy]
    F --> D
    D -->|persisted busy or live evidence| I[busy]
    D -->|persisted idle and no live evidence| J[idle]
    D -->|no conclusive evidence| K[unknown]
    I --> L[desktop_sessions_list]
    J --> L
    K --> L
    L --> M[maestro-cli session list]

Loading

_{Reviews (1): Last reviewed commit: "fix: preserve truthful process and sessi..." | Re-trigger Greptile}

[pedramamini]

Thanks for this, @heinbrian - genuinely good catch. Conflating EPERM with "process is gone" is exactly the kind of thing that makes a monitor lie about state, and splitting ESRCH (proof of absence) from EPERM (proof of presence) from everything else (no proof either way) is the right decomposition. I also appreciate that the cleanup write is now gated on confirmed absence rather than on any failure.

I verified locally against c6a8c0f:

• the four test files: 149 passed, matching your description
• no merge conflicts against main
• the ${sessionId}-ai-${tabId} and legacy ${sessionId}-ai process id convention matches the real spawn sites in agentStore.ts and useInputProcessing.ts
• ProcessManager deletes map entries in ExitHandler, so get() returning truthy really does mean "spawned and not yet exited"

Two small things I would like addressed before merge, plus one note.

1. unknown breaks the one aligned column in session list

src/cli/commands/session.ts prints:

`${state} ${star} ${s.tabId}  ${s.agentName} (${s.agentId})  ${name}  ${created}`

busy and idle are both 4 characters, so the tabId column currently lines up across rows. unknown is 7, so a single unknown row jags the tabId column for the whole listing. Padding keeps the grep busy behavior the comment calls out and restores alignment:

const state = s.state.padEnd(7);

2. isSessionBusyWithCli is called once per agent, and each call re-reads the same file

In web-server-factory.ts, hoisting the call out of the tab loop is right, but it is still inside the session loop. isSessionBusyWithCli -> getCliActivityForSession -> readCliActivities performs a synchronous fs.readFileSync plus JSON.parse of cli-activity.json on every call. With N agents that is N redundant synchronous disk reads of the same file on the Electron main thread per session list, and the CLI polls this endpoint.

Since cli-activity.ts already has a readCliActivities(), a bulk helper there (something like getBusyCliSessionIds(): Set<string>) would collapse it to a single read and keep the EPERM/ESRCH liveness logic in one place rather than duplicating the probe per session.

Note (not blocking)

managedProcessActive treats "a process is present in the ProcessManager map" as "busy". That holds for batch-mode agents, which spawn a fresh process per message, and every shipping agent (claude-code, codex, opencode, factory-droid, copilot-cli) has supportsBatchMode: true. It does not hold for stdin agents, which keep a long-lived process for the life of the tab and would therefore report busy forever. Today that is only gemini-cli and qwen3-coder, both hidden: true and not shipping, so nothing is actually broken here.

That said, the correctness of this line rests entirely on that capability. A one-line comment recording the assumption would stop the next person from wiring up a stdin agent and reintroducing exactly the untruthful state this PR sets out to remove.

Nice, well-scoped change overall - the test coverage on the error-code branches in particular is the right shape. Happy to re-review once the two items above are in.