Skip to content

test: isolate terminal tests with ephemeral ports and process-group cleanup (#79) - #81

Merged
geeen-jp merged 3 commits into
mainfrom
issue-79
Oct 9, 2026
Merged

geeen-jp merged 3 commits into
mainfrom
issue-79

Conversation

@geeen-jp

@geeen-jp geeen-jp commented Oct 9, 2026

Copy link
Copy Markdown
Owner

AGENT:
Implemented through the multi-agent pipeline (Claude Sonnet 5.5 medium design/review/triage, OpenAI GPT-6 Luna max implementation, OpenAI GPT-6.1 Sol high audit). Terminal tests (CI=true uv run python -m pytest --forked -p no:cacheprovider tests/tools/terminal) passed twice in a row (349 passed, 15 skipped each); no http.server started by those runs was left, and port 8081 was not listening. make validate rc 0 in 18m24s (1104s); porcelain empty; no leftover listener from that run.

Why

make validate for sdk#73 left python -u -m http.server 8081 listening after session.close(). The next cases that also bind 8081 then failed with address already in use. Two gaps: the tests share a fixed port, and teardown killpgs only the shell's own process group, so a foreground job under job control (and a pooled tmux pane) survives.

Summary

  • Server tests in tests/tools/terminal take a free port (socket bind 0) instead of 8081. They still assert the server serves and that Ctrl+C interrupts the foreground server.
  • Subprocess and tmux close() snapshot descendant process groups before the shell dies, then TERM and KILL those groups. Pooled tmux shutdown snapshots every pane's job groups before the shared session is killed, then reaps them once. A pool-mode regression starts a server that ignores hang-up and asserts close() removes it.
  • scripts/validate-env.sh EXIT trap signals and reaps descendants of the sandboxed run (matched by the sandbox TMPDIR) before rm -rf. If a process survives, the sandbox is kept.

The CI job name stays validate. The allowlist and validate-steps suite list are unchanged.

Issue Number

Closes #79

How to Test

CI=true uv run python -m pytest --forked -p no:cacheprovider tests/tools/terminal

Then confirm no http.server from that run is still alive and nothing is listening on 8081.

make validate

On this host that was rc 0 in 18m24s, with an empty git status --porcelain and no leftover server from the run.

Type

  • Bug fix
  • Feature
  • Refactor
  • Docs / chore

Notes

Do not deselect or skip the terminal server tests. macOS CI is in scope: no unguarded Linux /proc assumptions. The orchestrator did not print secret values.

@geeen-jp geeen-jp closed this Oct 9, 2026
@geeen-jp geeen-jp reopened this Oct 9, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	AGENTS.md
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. The directory will be automatically removed when the PR is approved.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Endpoint audit

⚠️ 18 actionable Agent Server contract divergence(s) · report-only

Contract: pinned release artifact

Category Count
Actionable client-only calls 0
Actionable server-only operations 18
Documented non-divergences 13
Agent Server contract operations 129
Audited handwritten client endpoints 119

Actionable client-only calls (0)

none

Actionable server-only operations (18)

  • DELETE /api/canvas-extensions/installed/{}
  • DELETE /api/llm/provider-connections/{}
  • GET /api/canvas-extensions/installed
  • GET /api/canvas-extensions/installed/{}
  • GET /api/canvas-extensions/installed/{}/bundle
  • GET /api/conversations/{}/events
  • GET /api/file/archive
  • GET /api/git/commits
  • GET /api/git/commits/{}/changes
  • GET /api/init
  • GET /api/llm/provider-connections
  • PATCH /api/canvas-extensions/installed/{}
  • PATCH /api/llm/provider-connections/{}
  • POST /api/canvas-extensions/install
  • POST /api/conversations/{}/load_plugin
  • POST /api/file/create_directory
  • POST /api/init
  • POST /api/llm/provider-connections
Documented non-divergences (13)

Client calls intentionally absent from the filtered contract (11)

  • GET /
  • GET /alive
  • GET /health
  • GET /ready
  • GET /server_info

Reason: Operational Agent Server endpoints intentionally excluded from the filtered public release artifact.
Owner: OpenHands runtime maintainers

  • DELETE /api/meta-profiles/{}
  • GET /api/meta-profiles
  • GET /api/meta-profiles/{}
  • POST /api/meta-profiles/{}
  • POST /api/meta-profiles/{}/activate

Reason: Client-ahead API stacked on the pending Agent Server meta-profiles implementation.
Owner: OpenHands SDK maintainers
Tracking: OpenHands#3744

  • POST /api/profiles/{}/validate

Reason: Client-ahead API stacked on the pending Agent Server pre-flight LLM validation endpoint.
Owner: OpenHands TypeScript client maintainers
Tracking: OpenHands#4422

Server operations covered by an exposed browser URL (2)

  • GET /api/conversations/{}/workspace
  • GET /api/conversations/{}/workspace/{}

Reason: RemoteWorkspace.startWorkspaceSession exposes these authenticated URLs for browser iframe and file requests; they are not HttpClient method calls.
Owner: OpenHands TypeScript client maintainers

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Python API breakage checks — ✅ PASSED

Result: ✅ PASSED

Action log

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

REST API breakage checks (OpenAPI) — ✅ PASSED

Result: ✅ PASSED

Action log

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-tools/openhands/tools/terminal/terminal
   process_groups.py361461%34–35, 41–47, 54–57, 60
   subprocess_terminal.py28723618%16, 70–71, 97–100, 103–108, 114–115, 121–122, 124–126, 133–134, 138–139, 145–146, 149, 152–156, 158, 161, 163–165, 179–182, 184, 187–188, 191, 194–195, 198, 202–203, 205, 207, 211–212, 214–216, 219–222, 224–230, 232–238, 240, 242–247, 249–250, 252–253, 258, 260–268, 272–274, 276–277, 279–280, 283–285, 287–290, 292–293, 295–296, 298–303, 308–310, 312, 315, 318–319, 322–323, 329–331, 333–340, 342–345, 349–357, 392–393, 395–396, 399–400, 402, 404, 406–408, 411–414, 416–418, 420–421, 423–424, 433–436, 445–447, 450–451, 454, 456, 459–460, 462, 469–470, 474, 476–481, 485–486, 488–491, 493–499, 501–502, 504–508, 512–513, 515–521, 525–526, 529–530, 532–533, 535–537
   tmux_pane_pool.py1503179%52–56, 114, 121, 168–169, 178, 232, 236–237, 257–258, 274, 277–278, 280–283, 286–287, 291–295, 297–298
   tmux_terminal.py1146345%77–78, 80, 83–84, 86–88, 90, 92, 94–96, 103–104, 108–109, 112–113, 118–122, 126, 129, 131–133, 141, 145–151, 155–157, 174, 181–182, 187–188, 203, 225, 237–244, 252–253, 255–256, 258, 260–262
TOTAL436321238072% 

@geeen-jp
geeen-jp merged commit 5191d6a into main Oct 9, 2026
34 checks passed
@geeen-jp
geeen-jp deleted the issue-79 branch October 9, 2026 14:30
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.

Isolate terminal http.server tests onto ephemeral ports and reap leaked process groups

1 participant