Skip to content

[Fix] Setup command timeouts leave child processes running - #3601

Open
roomote-roomote[bot] wants to merge 2 commits into
developfrom
fix/foreground-timeout-cleanup-1pn0qy1hk5krr
Open

roomote-roomote[bot] wants to merge 2 commits into
developfrom
fix/foreground-timeout-cleanup-1pn0qy1hk5krr

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

​Opened on behalf of Daniel Riccio. Follow up by mentioning @roomote-roomote, in the web UI, or in Telegram.

Related issue

Internal runtime reliability follow-up; no separate issue.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

Foreground setup commands terminate their isolated process group at their deadline, so inherited child pipes cannot prevent timeout reporting, retry or recovery. Bounded TERM/KILL cleanup reuses the existing executor/callback path. Ordinary output, environment, working directory, retries and detached/PM2 contracts remain covered; no dependency, schema or durable state is added.

How it was tested

  • Current develop 71ee50ca: actual WorkspaceManager one-second deadline still unsettled/live child at 8.013s. Same stimulus at exact head 8d49aee1bdf048ab8aaa1d5ae671d03194cdb6c4: timeout 1.011s/no child, next command writes the recovery file. Actual sudo/bash/sleep 1.020s/no child; node 24.22.1, pnpm 10.29.3 and mise controls succeed.
  • All 44 executor cases and direct worker types pass, including already-exited shells, TERM-resistant children/grandchildren, independent concurrent work, retry ordering and normal output. Native/PM2 fixtures cleaned. No escaped-daemon, API/DB receipt-persistence or hosted-provider parity claim.
  • Normal push gates and exact-head hosted Test/types, both Docker builds, CodeQL, lint, Knip, upgrade/backup and Judgement passed. Exact-head review is clean/zero threads; mergeability rechecked. Browser proof not applicable.
  • Conflict-free develop sync incorporates independent merged [Chore] Align runtime configuration tests with shipped Kimi routes #3603; the net diff remains executor cleanup and native tests only. Earlier incomplete old-base cold-root attempts are not claimed green. A separate multi-PR compatibility run has explicit fixture/path/cold-import limits; it does not replace exact-head evidence.

Related PRs

Independent companions: cache correctness, review debounce, Users recovery, native watcher, credential fixture isolation.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally (normal broad fast equivalents passed)
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset (internal runtime reliability)

@roomote-community

roomote-community Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

No new code issues found. See session

The latest update only merges develop with the independently merged Kimi test expectation repair. Both PR files are unchanged since the prior review, and the base-to-head patch is identical; the prior clean review carries forward. There are no outstanding review threads.

On this head, lint, type check, Knip, upgrade compatibility, and backup restore checks passed. Tests and Docker builds and JavaScript/TypeScript CodeQL were still running when inspected.

Reviewed 8d49aee

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