fix: private client dir, backgrounded port-forward, non-connecting readiness, listener host in remote-debugging skill by myakove · Pull Request #752 · RedHatQE/mtv-api-tests

myakove · 2026-10-18T09:22:08Z

Closes #751

Mirrors the verified fixes from RedHatQE/openshift-virtualization-tests#6583, where the ported remote-debugging skill went through further review rounds. All four findings were confirmed against our copy before fixing.

Changes (`skills/remote-debugging/SKILL.md`, `README.md`)

Private per-session client directory — the attach client used fixed /tmp/remote-pdb-${port}-in|out paths with rm -f; on sticky /tmp another local user could pre-create them and read debugger output or inject commands through the FIFO. Now: CLIENT_DIR="/tmp/remote-pdb-${port}-$$" with atomic mkdir -m 700 (fails closed on pre-existing paths), FIFO and output inside it, cleanup rm -rf "$CLIENT_DIR".
Backgrounded pod port-forward — the pod attach block ran oc port-forward in the foreground, so nc never ran when the block was executed as documented. Now backgrounded with PID capture and post-session kill.
Non-connecting readiness check — nothing waited for the forwarded port, and a connecting probe cannot be used: RemotePdb accepts a single client, so the probe consumes the session (probe exits → EOF → pdb quits → the debugged run fails). Now polls ss -ltn + kill -0 liveness, bounded 30 × 1s, cleanup and exit on timeout.
Actual listener host — the client hardcoded nc 127.0.0.1 $port while REMOTE_PDB_HOST exists; with e.g. 127.0.0.2 the client waited forever. Now resolves host and port from the run's RemotePdb session open at <host>:<port> line (README attach snippet updated the same way).

Not applicable here: the Python 3.14 interaction(tb_or_exc=...) keyword rename from the upstream review — our utilities/debug.py calls interaction() positionally, which is version-agnostic.

Docs-only change: .pi/skills/remote-debugging is a symlink to skills/remote-debugging, so the single file covers both copies.

Assisted-by: PI (rits/zai-org/glm-5-3)

Summary by CodeRabbit

Documentation
- Updated the remote debugging example and instructions to use the host and port reported by the active run, including configured host and port overrides and worker-specific port offsets.
- For pod-based runs, instructions now cover starting port forwarding and waiting for it to become available, with an error if it fails or times out.
- Remote debugging files are now described as stored in a private per-session directory that is removed during cleanup.

…adiness, listener host in remote-debugging skill Closes #751 Mirror the verified fixes from RedHatQE/openshift-virtualization-tests#6583: - Attach client uses a per-session private directory (mkdir -m 700, fails closed on pre-existing paths) instead of predictable /tmp FIFO paths - Pod port-forward runs in the background with PID capture and kill - Readiness poll uses ss -ltn + kill -0 only (a connecting probe would consume RemotePdb's single client session) - Client resolves host/port from the run's "RemotePdb session open at" line instead of hardcoding 127.0.0.1 Assisted-by: PI (rits/zai-org/glm-5-3) <noreply@pi.dev> Signed-off-by: Meni Yakove <myakove@gmail.com>

redhat-qe-bot2 · 2026-10-18T09:22:22Z

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
Issue Creation: Disabled for this repository
Branch Labeling: Branch-specific labels are applied to track the target branch
Auto-verification: Auto-verified users have their PRs automatically marked as verified
Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

/wip - Mark PR as work in progress (adds WIP: prefix to title)
/wip cancel - Remove work in progress status
/hold - Block PR merging (PR author or approvers)
/hold cancel - Unblock PR merging (PR author or approvers)
/verified - Mark PR as verified
/verified cancel - Remove verification status
/reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
/regenerate-welcome - Regenerate this welcome message
/security-override - Set security check runs to pass (maintainers only)
/security-override cancel - Re-run security checks

Review & Approval

/lgtm - Approve changes (looks good to me)
/approve - Approve PR (approvers only)
/automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
/automerge cancel - Remove automerge label (maintainers and approvers only)
/assign-reviewers - Assign reviewers based on OWNERS file
/assign-reviewer @username - Assign specific reviewer
/check-can-merge - Check if PR meets merge requirements

Testing & Validation

/retest tox - Run Python test suite with tox
/retest build-container - Rebuild and test container image
/retest conventional-title - Validate commit message format
/retest all - Run all available tests

Container Operations

/build-and-push-container - Build and push container image (tagged with PR number)
- Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

/cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
- Multiple branches: /cherry-pick branch1 branch2 branch3
/cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

/rebase - Rebase this PR branch onto its base branch

Label Management

/<label-name> - Add a label to the PR
/<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

Approval: /approve from at least one approver
Status Checks: All required status checks must pass
No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

krcmarik
myakove
solenoci

Reviewers:

krcmarik
myakove
solenoci

Available Labels

hold
verified
wip
lgtm
approve
automerge

AI Features

Conventional Title: Mode: fix (cursor/cursor-grok-4.6-high)
Cherry-Pick Conflict Resolution: Enabled (cursor/cursor-grok-4.6-high)

Security Checks

Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
Committer Identity Check: Verifies last committer matches PR author
Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

WIP Status: Use /wip when your PR is not ready for review
Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
Cherry-picking: Cherry-pick labels are processed when the PR is merged
Container Builds: Container images are automatically tagged with the PR number
Permission Levels: Some commands require approver permissions
Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

coderabbitai · 2026-10-18T09:22:25Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: RedHatQE/mtv-api-tests/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 177d27e2-b350-4a24-8cf7-e71b21065b2b

📥 Commits

Reviewing files that changed from the base of the PR and between a23b60f and 8e4dd60.

📒 Files selected for processing: 2

README.md
skills/remote-debugging/SKILL.md

+-------------------------------------------------------------------+
| That's not technical debt - that's technical *predatory lending*. |
+-------------------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

🧪 Generate unit tests (beta)

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.

❤️ Share

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

qodo-code-review · 2026-10-18T09:22:41Z

PR Summary by Qodo

Fix remote debugger attach instructions and client isolation

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

AI Description

• Isolate debugger client files in a private directory to prevent local interference and disclosure.
• Background pod port-forwarding and check readiness without consuming the debugger’s single
 connection.
• Use the run’s reported listener address instead of assuming the default host.

Diagram

graph TD
  A["Reported address"] --> B{"Pod run?"}
  B -->|Yes| C["Background forward"] --> D{"Socket ready?"} -->|Yes| E["Private client dir"] --> F["Single nc attach"] --> G["PID cleanup"]
  B -->|No| E
  D -->|No| G

High-Level Assessment

The documentation-only approach fits an issue in the attach instructions without changing debugger behavior. A connecting readiness probe is unsuitable because RemotePdb accepts one client; checking the listening socket preserves that connection for the actual attach.

Files changed (2) +42 / -19

Bug fix (1) +39 / -18

SKILL.mdMake agent attach instructions private and pod-safe +39/-18

Make agent attach instructions private and pod-safe

• The client example places its FIFO and output in an owner-only per-session directory and uses the reported listener host. Pod instructions background port-forwarding, wait for socket readiness without connecting, and specify forward cleanup.

skills/remote-debugging/SKILL.md

Documentation (1) +3 / -1

README.mdDocument attachment using the reported listener address +3/-1

Document attachment using the reported listener address

• The basic attach example now takes the host and port from the run’s session-open message rather than hardcoding the default loopback address.

README.md

qodo-code-review · 2026-10-18T09:25:01Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~An occupied port sends clients elsewhere~~ ✓ Resolved

Description

The readiness loop treats any socket listening on $port as proof that the newly started
port-forward is ready, and breaks before checking its PID. If another process already owns that
port, the forward can fail while both socket checks succeed, so the subsequent debugger client
connects to the unrelated process instead of the pod.

Code

skills/remote-debugging/SKILL.md[R97-99]

+for _ in $(seq 1 30); do
+  ss -ltn | grep -q ":$port " && break
+  kill -0 "$FORWARD_PID" 2>/dev/null || { echo "port-forward died"; exit 1; }

Relevance

●●● Strong
Accepted readiness feedback established port-forward correctness as important; unrelated listeners
can invalidate this readiness check.
PR-#733

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The skill starts a specific background forward but checks only for a matching port. A match skips
the liveness check, and the final check repeats the same unowned-port test before the user attaches.
skills/remote-debugging/SKILL.md[91-103]
skills/remote-debugging/SKILL.md[104-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The readiness check can mistake an existing listener for the newly started pod port-forward, allowing the client to connect elsewhere.
## Fix Focus Areas
- skills/remote-debugging/SKILL.md[91-102]
## Recommended Fix
Check that the forward remains alive and that the listening socket belongs to the newly started forward before declaring readiness; fail and clean up if the port is occupied by another process.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Configured hosts get lost during attach~~ ✓ Resolved

Description

The listener-address lookup still depicts the session line with a literal 127.0.0.1 and supplies
only a port in its fallback steps, although the revised client requires $host. When
REMOTE_PDB_HOST selects another loopback address, following that lookup can miss the actual
session line or leave the client without the configured host, preventing attachment to the paused
run.

Code

skills/remote-debugging/SKILL.md[77]

+nohup bash -c "exec tail -f $FIFO | exec nc $host $port > $OUT 2>&1" < /dev/null > /dev/null 2>&1 &

Relevance

●●● Strong
Accepted host-resolution guidance is explicit; losing REMOTE_PDB_HOST contradicts the PR’s stated
listener-host fix.
PR-#733

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The debugger accepts and binds other loopback addresses, but the lookup example names only 127.0.0.1
and its remaining steps determine a port. The changed client command interpolates a host that those
paths do not establish.
utilities/debug.py[41-68]
skills/remote-debugging/SKILL.md[47-57]
skills/remote-debugging/SKILL.md[65-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The client now requires a host, but the skill's listener lookup retains a default-host-only output pattern and port-only fallback instructions.
## Fix Focus Areas
- skills/remote-debugging/SKILL.md[47-57]
- skills/remote-debugging/SKILL.md[65-77]
## Recommended Fix
Show the session line with a host placeholder, instruct the agent to extract both values, and have fallback steps explicitly determine the configured host or use the documented default before starting the client.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

❌ Tickets: failed to load — Ticket collection could not establish complete evidence

✅ Compliance rules (platform): 103 rules

✅ Skills: remote-debugging

✅ Cross-repo context — repo relationships

Explored: repo: myk-org/docsfy (sha: 0a96d7de) — View relationship

Review mode: Auto: ⚖️ Balanced: Multiple security-sensitive shell workflow changes require careful single-pass review.

✅ Fix-loop round: fix loop

Tip of the day

💡 Did you know, you can move or hide resolved findings to focus on what still needs action

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 8e4dd60

Results up to commit 3fc04b0 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~An occupied port sends clients elsewhere~~ ✓ Resolved

Description

The readiness loop treats any socket listening on $port as proof that the newly started
port-forward is ready, and breaks before checking its PID. If another process already owns that
port, the forward can fail while both socket checks succeed, so the subsequent debugger client
connects to the unrelated process instead of the pod.

Code

skills/remote-debugging/SKILL.md[R97-99]

+for _ in $(seq 1 30); do
+  ss -ltn | grep -q ":$port " && break
+  kill -0 "$FORWARD_PID" 2>/dev/null || { echo "port-forward died"; exit 1; }

Relevance

●●● Strong
Accepted readiness feedback established port-forward correctness as important; unrelated listeners
can invalidate this readiness check.
PR-#733

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The skill starts a specific background forward but checks only for a matching port. A match skips
the liveness check, and the final check repeats the same unowned-port test before the user attaches.
skills/remote-debugging/SKILL.md[91-103]
skills/remote-debugging/SKILL.md[104-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The readiness check can mistake an existing listener for the newly started pod port-forward, allowing the client to connect elsewhere.
## Fix Focus Areas
- skills/remote-debugging/SKILL.md[91-102]
## Recommended Fix
Check that the forward remains alive and that the listening socket belongs to the newly started forward before declaring readiness; fail and clean up if the port is occupied by another process.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Configured hosts get lost during attach~~ ✓ Resolved

Description

The listener-address lookup still depicts the session line with a literal 127.0.0.1 and supplies
only a port in its fallback steps, although the revised client requires $host. When
REMOTE_PDB_HOST selects another loopback address, following that lookup can miss the actual
session line or leave the client without the configured host, preventing attachment to the paused
run.

Code

skills/remote-debugging/SKILL.md[77]

+nohup bash -c "exec tail -f $FIFO | exec nc $host $port > $OUT 2>&1" < /dev/null > /dev/null 2>&1 &

Relevance

●●● Strong
Accepted host-resolution guidance is explicit; losing REMOTE_PDB_HOST contradicts the PR’s stated
listener-host fix.
PR-#733

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The debugger accepts and binds other loopback addresses, but the lookup example names only 127.0.0.1
and its remaining steps determine a port. The changed client command interpolates a host that those
paths do not establish.
utilities/debug.py[41-68]
skills/remote-debugging/SKILL.md[47-57]
skills/remote-debugging/SKILL.md[65-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The client now requires a host, but the skill's listener lookup retains a default-host-only output pattern and port-only fallback instructions.
## Fix Focus Areas
- skills/remote-debugging/SKILL.md[47-57]
- skills/remote-debugging/SKILL.md[65-77]
## Recommended Fix
Show the session line with a host placeholder, instruct the agent to extract both values, and have fallback steps explicitly determine the configured host or use the documented default before starting the client.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

…mote-debugging skill Qodo review follow-ups on PR #752: - Readiness poll now matches the listener to THIS forward (ss -ltnp pid= match) — a bare port match could be an unrelated process that already owned the port, sending the client to it - Listener-address lookup no longer shows the session line with a literal 127.0.0.1 and every fallback step now yields a host (REMOTE_PDB_HOST / ss bound address / 127.0.0.1 default), so the client always has $host Assisted-by: PI (litellm/claude-opus-4-6) <noreply@pi.dev> Signed-off-by: Meni Yakove <myakove@gmail.com>

qodo-code-review · 2026-10-18T09:29:37Z

Code review by qodo was updated up to the latest commit 8e4dd60

rh-bot-1 · 2026-10-18T09:36:32Z

New container for ghcr.io/redhatqe/mtv-api-tests:latest published

redhat-qe-bot2 assigned myakove Oct 18, 2026

redhat-qe-bot2 requested review from krcmarik and solenoci October 18, 2026 09:22

redhat-qe-bot2 added branch-main size/M labels Oct 18, 2026

qodo-code-review Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread skills/remote-debugging/SKILL.md Resolved

Comment thread skills/remote-debugging/SKILL.md Resolved

redhat-qe-bot2 added the commented-qodo-code-review[bot] label Oct 18, 2026

rh-bot-1 removed the commented-qodo-code-review[bot] label Oct 18, 2026

myakove merged commit 9c44933 into main Oct 18, 2026
6 of 9 checks passed

myakove deleted the fix/issue-751-remote-debugging-client-fixes branch October 18, 2026 09:35

Navigation Menu

Repository navigation

fix: private client dir, backgrounded port-forward, non-connecting readiness, listener host in remote-debugging skill - #752

fix: private client dir, backgrounded port-forward, non-connecting readiness, listener host in remote-debugging skill#752
myakove merged 2 commits into
mainfrom
fix/issue-751-remote-debugging-client-fixes

myakove commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

redhat-qe-bot2 commented Oct 18, 2026

Uh oh!

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

Uh oh!

qodo-code-review Bot commented Oct 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

Uh oh!

Uh oh!

rh-bot-1 commented Oct 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

myakove commented Oct 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes (skills/remote-debugging/SKILL.md, README.md)

Summary by CodeRabbit

Uh oh!

redhat-qe-bot2 commented Oct 18, 2026

Welcome! 🎉

🔄 Automatic Actions

📋 Available Commands

PR Status Management

Review & Approval

Testing & Validation

Container Operations

Cherry-pick Operations

Branch Management

Label Management

✅ Merge Requirements

📊 Review Process

💡 Tips

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

Uh oh!

Uh oh!

rh-bot-1 commented Oct 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

myakove commented Oct 18, 2026 •

edited by coderabbitai Bot

Loading

Changes (`skills/remote-debugging/SKILL.md`, `README.md`)

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

qodo-code-review Bot commented Oct 18, 2026 •

edited

Loading