Repository navigation
Conversation
…that only collides Each Docker host has its own PID namespace, so the PID host A records in the login-home slot can belong to an unrelated live process on host B; kill -0 alone then read it as B running from A's slot. Count a recorded PID only when its process runs from that slot.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly describes the main change: updating the shared-home SSH end-to-end test to distinguish a host's own server from a colliding PID. |
| Description check | |
The description is complete and focused. It explains the failure, the mechanism, the reason for the change, testing, scope, and checklist status. The Linked Issue section describes the flaky failure b… |
Full details: Description check
-
Explanation
The description is complete and focused. It explains the failure, the mechanism, the reason for the change, testing, scope, and checklist status. The Linked Issue section describes the flaky failure but does not include a specific issue number or link.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
ELI5
One end-to-end test checks that two SSH machines sharing a home folder each run their own Orca server. It sometimes failed when nothing was actually wrong, because two separate machines can hand out the same process number. The test now checks that a number really belongs to the server it is looking for.
What Changed
Before:
tests/e2e/ssh-orcad-shared-home-machines.spec.ts:47could fail at its "B never launched into A's slots" check, as it did on main (Received ["229"], run 39083874854, Docker 2/2). This was not a product failure.After: the spec counts a PID from a server slot's
.orcad-pidas running only if that process's command line names that slot directory.Mechanism. The two Docker SSH hosts share
/rootbut each has its own PID namespace, and both boot the same image through the same deploy steps. So the PID that host A records in the login-home slot (/root/.orca-remote/orcad-*/.orcad-pid) can be a live process on host B: B's own server or one of its children.kill -0alone then reports that B runs from A's slot. A managed server is started asnohup <runtime> <slot dir>/<launcher> ..., so its command line always carries its own slot's absolute path. B's server, which runs under/root/.orca-remote/hosts/<id>/..., never matches the login-home slot path.Why
This is a test-only flake, not a regression in the code. None of the PRs that merged between the passing and failing commits touch the managed deploy or slot selection: after
299dcb86fc8athey are #30035, #30125, #30131, #30120, #30129 and #30132. The spec also passed on both bisect probes, #30139 at27cfee6f7ea2and #30138 at299dcb86fc8a. The assertion keeps its intent: if B really launched into a login-home slot, its process would name that slot and the check would still fail. Alternatives I considered:Linked Issue
Fixes the flaky
ssh-orcad-shared-home-machines.spec.ts:47failure on main. I found no product regression. The suspect was #30120, and it is not involved.Visual Proof
N/A: test harness only.
Testing
Checked the shell predicate on a real Linux host (omarchy), isolated under
/tmp. A slot PID file holding a live PID whose process runs from a different slot is now excluded. The process's own slot is still matched.The managed-orcad Docker lane on this PR runs the spec against real Docker SSH hosts.
No unit test: the change is to an e2e spec's own shell probe, and a unit test cannot reproduce a cross-container PID collision.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Codex review (
gpt-6.1-sol, high): no findings.Agent skill upstream boundary
Notes
Test only. The spec is Docker-only.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)