Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/pythinker-host/src/pythinker_host/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ async def wait(self) -> int:
return await self._process.wait()

async def kill(self) -> None:
# If the process has already exited (and been reaped), its pid/pgid
# may have been recycled by the OS; signaling it could hit an
# unrelated process group. Skip the group-kill in that case.
if self._process.returncode is not None:
return
if os.name != "nt":
try:
os.killpg(os.getpgid(self._process.pid), signal.SIGKILL)
Expand Down
61 changes: 61 additions & 0 deletions packages/pythinker-host/tests/test_local_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import asyncio
import os
import signal
import sys
from collections.abc import Generator
from pathlib import Path, PurePosixPath, PureWindowsPath
from typing import Any

import pytest

Expand Down Expand Up @@ -196,3 +198,62 @@ async def test_exec_wait_timeout(local_host: LocalHost):
if process.returncode is None:
await process.kill()
await process.wait()


@pytest.mark.skipif(os.name == "nt", reason="POSIX process-group signal path")
async def test_kill_skips_signal_after_process_exit(
local_host: LocalHost, monkeypatch: pytest.MonkeyPatch
):
"""Once a process has exited (and been reaped), its pid/pgid may be recycled
by the OS, so kill() must not send a process-group signal."""
import pythinker_host.local as local_module

process = await local_host.exec(*_python_code_args("import sys; sys.exit(0)"))
await process.wait()
assert process.returncode is not None

# Fail loudly if kill() reaches the signal path at all: with the returncode
# guard it must short-circuit before even resolving the process group.
calls: list[str] = []

def _record_getpgid(pid: int) -> int:
calls.append("getpgid")
return 0

def _record_killpg(pgid: int, sig: int) -> None:
calls.append("killpg")

monkeypatch.setattr(local_module.os, "getpgid", _record_getpgid)
monkeypatch.setattr(local_module.os, "killpg", _record_killpg)

await process.kill()

assert calls == []


@pytest.mark.skipif(os.name == "nt", reason="POSIX process-group signal path")
async def test_kill_signals_running_process(local_host: LocalHost, monkeypatch: pytest.MonkeyPatch):
"""A still-running process is killed via its process group."""
import pythinker_host.local as local_module

process = await local_host.exec(*_python_code_args("import time; time.sleep(30)"))
assert process.returncode is None

# os.killpg / signal.SIGKILL are POSIX-only; this test is skipped on Windows
# but pyright still type-checks the body, so route them through Any holders
# to keep the strict host gate green on the win32 platform stubs.
os_any: Any = local_module.os
signal_any: Any = signal
real_killpg = os_any.killpg
sent: list[int] = []

def _record_killpg(pgid: int, sig: int) -> None:
sent.append(sig)
real_killpg(pgid, sig) # actually terminate so the process is not leaked

monkeypatch.setattr(local_module.os, "killpg", _record_killpg)

await process.kill()
await process.wait()

assert sent and sent[0] == signal_any.SIGKILL
9 changes: 9 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,13 @@ Encrypter = "Encrypter"
# Hex session IDs (e.g. 06ba6c38) contain "ba".
ba = "ba"
uest = "uest"
# Verb stems in the unfinished-intent detection regex (soul/pythinkersoul.py)
# match word prefixes ("prepar" → prepare/preparing); kept as bare stems on
# purpose so they cover every inflection.
prepar = "prepar"
generat = "generat"
provid = "provid"
updat = "updat"
examin = "examin"
continu = "continu"

5 changes: 3 additions & 2 deletions src/pythinker_code/auth/github_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import asyncio
import time
import webbrowser
from dataclasses import dataclass
from typing import Any, cast

Expand Down Expand Up @@ -150,7 +149,9 @@ async def login_github_feedback(
)
if open_browser:
try:
webbrowser.open(auth.verification_uri)
from pythinker_code.utils.term import open_url_in_browser

open_url_in_browser(auth.verification_uri)
except Exception as exc:
logger.warning("Failed to open browser: {error}", error=exc)
yield OAuthEvent("waiting", "Waiting for GitHub authorization...")
Expand Down
5 changes: 3 additions & 2 deletions src/pythinker_code/auth/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import tempfile
import time
import uuid
import webbrowser
from collections.abc import AsyncGenerator, AsyncIterator
from contextlib import asynccontextmanager, suppress
from dataclasses import dataclass
Expand Down Expand Up @@ -660,7 +659,9 @@ async def login_pythinker_code(
)
if open_browser:
try:
webbrowser.open(auth.verification_uri_complete)
from pythinker_code.utils.term import open_url_in_browser

open_url_in_browser(auth.verification_uri_complete)
except Exception as exc:
logger.warning("Failed to open browser: {error}", error=exc)

Expand Down
5 changes: 3 additions & 2 deletions src/pythinker_code/auth/openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import json
import secrets
import time
import webbrowser
from collections.abc import AsyncIterator
from dataclasses import dataclass
from typing import Any, cast
Expand Down Expand Up @@ -329,7 +328,9 @@ async def _wait_for_browser_code(open_browser: bool = True) -> tuple[str, str, s

auth_url = _build_authorize_url(redirect_uri=redirect_uri, pkce=pkce, state=state)
if open_browser:
webbrowser.open(auth_url)
from pythinker_code.utils.term import open_url_in_browser

open_url_in_browser(auth_url)

try:
code, error = await asyncio.wait_for(result, timeout=15 * 60)
Expand Down
44 changes: 26 additions & 18 deletions src/pythinker_code/background/agent_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,35 +99,41 @@ async def run(self) -> None:
id=self._task_id,
t=self._timeout_s,
)
self._runtime.subagent_store.update_instance(self._agent_id, status="failed")
self._manager._mark_task_timed_out(
self._task_id, f"Agent task timed out after {self._timeout_s}s"
self._manager.finalize_agent_task(
self._task_id,
self._agent_id,
outcome="timed_out",
reason=f"Agent task timed out after {self._timeout_s}s",
)
output.error(
_timeout_recovery_message(timeout_s=self._timeout_s, agent_id=self._agent_id)
)
else:
# Internal timeout (e.g. aiohttp request) — treat as generic failure
logger.exception("Background agent runner failed")
self._runtime.subagent_store.update_instance(self._agent_id, status="failed")
self._manager._mark_task_failed(self._task_id, str(exc))
self._manager.finalize_agent_task(
self._task_id, self._agent_id, outcome="failed", reason=str(exc)
)
output.error(str(exc))
except asyncio.CancelledError:
self._runtime.subagent_store.update_instance(self._agent_id, status="killed")
self._manager._mark_task_killed(self._task_id, "Stopped by TaskStop")
self._manager.finalize_agent_task(
self._task_id, self._agent_id, outcome="killed", reason="Stopped by TaskStop"
)
output.stage("cancelled")
raise
except RunCancelled:
# RunCancelled is Exception (not BaseException), so re-raising it from
# an asyncio.create_task would trigger "Task exception was never retrieved".
# Just mark killed and return — cleanup is already done.
self._runtime.subagent_store.update_instance(self._agent_id, status="killed")
self._manager._mark_task_killed(self._task_id, "Run was cancelled")
self._manager.finalize_agent_task(
self._task_id, self._agent_id, outcome="killed", reason="Run was cancelled"
)
output.stage("cancelled")
except Exception as exc:
logger.exception("Background agent runner failed")
self._runtime.subagent_store.update_instance(self._agent_id, status="failed")
self._manager._mark_task_failed(self._task_id, str(exc))
self._manager.finalize_agent_task(
self._task_id, self._agent_id, outcome="failed", reason=str(exc)
)
output.error(str(exc))
finally:
# Whatever happens in approval cleanup below, the dict pop must
Expand Down Expand Up @@ -197,22 +203,24 @@ async def _ui_loop_fn(wire: Wire) -> None:
),
)
if failure is not None:
self._manager._mark_task_failed(self._task_id, failure.message)
self._runtime.subagent_store.update_instance(self._agent_id, status="failed")
self._manager.finalize_agent_task(
self._task_id, self._agent_id, outcome="failed", reason=failure.message
)
output.stage(f"failed: {failure.brief}")
return
output.stage("run_soul_finished")

if final_response is None:
self._manager._mark_task_failed(
self._task_id, "Agent completed but produced no output."
self._manager.finalize_agent_task(
self._task_id,
self._agent_id,
outcome="failed",
reason="Agent completed but produced no output.",
)
self._runtime.subagent_store.update_instance(self._agent_id, status="failed")
output.stage("failed: empty output")
return
output.summary(final_response)
self._runtime.subagent_store.update_instance(self._agent_id, status="idle")
self._manager._mark_task_completed(self._task_id)
self._manager.finalize_agent_task(self._task_id, self._agent_id, outcome="completed")

def _on_approval_runtime_event(self, event: ApprovalRuntimeEvent) -> None:
request = event.request
Expand Down
Loading
Loading