vxcontrol/pentagi
1
0
Fork 0
mirror of https://github.com/vxcontrol/pentagi.git synced 2026-07-21 08:15:23 -04:00
Code Issues 45 Packages Projects Releases 9 Wiki Activity

[PR #179] [MERGED] fix: isolate detached command context from parent cancellation #216

New Issue
Closed
opened 2026-06-06 22:09:44 -04:00 by yindo · 0 comments
yindo commented 2026-06-06 22:09:44 -04:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/179
Author: @mason5052
Created: 3/5/2026
Status: ✅ Merged
Merged: 3/11/2026
Merged by: @asdek

Base: feature/next_release ← Head: fix/detached-cmd-context-isolation

📝 Commits (3)

  • 8701f15 fix: isolate detached command context from parent cancellation
  • 55aef6a test: add context cancellation tests for detached command isolation
  • 1e2cf11 Rename mock type to avoid cross-PR symbol collision

📊 Changes

2 files changed (+188 additions, -1 deletions)

View changed files

📝 backend/pkg/tools/terminal.go (+2 -1)
➕ backend/pkg/tools/terminal_context_test.go (+186 -0)

📄 Description

Description of the Change

Problem

Detached terminal commands (detach=true) inherit the parent context. When the parent context is canceled (e.g., agent delegation timeout after ~2.5 minutes), the detached goroutine's ctx.Done() fires in getExecResult and kills the background command, even though the command has its own timeout (300-1200 seconds).

This makes agent delegation effectively unusable for commands that take more than ~2.5 minutes (which is most penetration testing tasks like nmap scans).

Closes #176

Solution

Use context.WithoutCancel(ctx) (Go 1.21+) for the detached goroutine. This:

  • Preserves context values (tracing spans, logrus fields, observability data)
  • Prevents parent cancellation from propagating to the background command
  • Keeps the command's own timeout working (context.WithTimeout in getExecResult)

Non-detached commands are unchanged -- they still use the original ctx and respect parent cancellation as before.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Areas Affected

  • Core Services (Backend API)

Testing and Verification

Test Configuration

  • PentAGI Version: master
  • Go Version: 1.24

Test Steps

  1. Confirmed context.WithoutCancel is available in Go 1.24 (introduced in Go 1.21)
  2. Verified non-detached path (last line of ExecCommand) still uses original ctx
  3. Verified detached goroutine now uses isolated context with own timeout
  4. Confirmed context values (observability, logging via t.tlp.PutMsg) are preserved through WithoutCancel
  5. Verified getExecResult's defer cancel() still prevents goroutine leak

Security Considerations

No security impact. The change only affects context propagation for background commands. Commands still terminate when their own timeout expires.

Checklist

  • My code follows the project's coding standards
  • All new and existing tests pass
  • I have run go fmt and go vet
  • Security implications considered
  • Changes are backward compatible

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/179 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 3/5/2026 **Status:** ✅ Merged **Merged:** 3/11/2026 **Merged by:** [@asdek](https://github.com/asdek) **Base:** `feature/next_release` ← **Head:** `fix/detached-cmd-context-isolation` --- ### 📝 Commits (3) - [`8701f15`](https://github.com/vxcontrol/pentagi/commit/8701f1573dceb4955835234c4394cac1713b8e62) fix: isolate detached command context from parent cancellation - [`55aef6a`](https://github.com/vxcontrol/pentagi/commit/55aef6aa951830e7cacd4c5514fe702a088834fd) test: add context cancellation tests for detached command isolation - [`1e2cf11`](https://github.com/vxcontrol/pentagi/commit/1e2cf11c79bfda676f4e20a5bc063db1d41c4d7d) Rename mock type to avoid cross-PR symbol collision ### 📊 Changes **2 files changed** (+188 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/tools/terminal.go` (+2 -1) ➕ `backend/pkg/tools/terminal_context_test.go` (+186 -0) </details> ### 📄 Description ### Description of the Change #### Problem Detached terminal commands (`detach=true`) inherit the parent context. When the parent context is canceled (e.g., agent delegation timeout after ~2.5 minutes), the detached goroutine's `ctx.Done()` fires in `getExecResult` and kills the background command, even though the command has its own timeout (300-1200 seconds). This makes agent delegation effectively unusable for commands that take more than ~2.5 minutes (which is most penetration testing tasks like `nmap` scans). Closes #176 #### Solution Use `context.WithoutCancel(ctx)` (Go 1.21+) for the detached goroutine. This: - **Preserves** context values (tracing spans, logrus fields, observability data) - **Prevents** parent cancellation from propagating to the background command - **Keeps** the command's own timeout working (`context.WithTimeout` in `getExecResult`) Non-detached commands are unchanged -- they still use the original `ctx` and respect parent cancellation as before. ### Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ### Areas Affected - [x] Core Services (Backend API) ### Testing and Verification #### Test Configuration - PentAGI Version: master - Go Version: 1.24 #### Test Steps 1. Confirmed `context.WithoutCancel` is available in Go 1.24 (introduced in Go 1.21) 2. Verified non-detached path (last line of `ExecCommand`) still uses original `ctx` 3. Verified detached goroutine now uses isolated context with own timeout 4. Confirmed context values (observability, logging via `t.tlp.PutMsg`) are preserved through `WithoutCancel` 5. Verified `getExecResult`'s `defer cancel()` still prevents goroutine leak ### Security Considerations No security impact. The change only affects context propagation for background commands. Commands still terminate when their own timeout expires. ### Checklist - [x] My code follows the project's coding standards - [x] All new and existing tests pass - [x] I have run `go fmt` and `go vet` - [x] Security implications considered - [x] Changes are backward compatible --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-06-06 22:09:44 -04:00
yindo closed this issue 2026-06-06 22:09:44 -04:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
enhancement
help wanted
pull-request
Mirrored from GitHub Pull Request
 question
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#216
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?