Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 295d65e4ef | |||
| 0f5d484151 |
@@ -8,7 +8,6 @@ on:
|
|||||||
- '**.py'
|
- '**.py'
|
||||||
- '.pylintrc'
|
- '.pylintrc'
|
||||||
- 'pyrightconfig.json'
|
- 'pyrightconfig.json'
|
||||||
workflow_dispatch:
|
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
update-badges:
|
update-badges:
|
||||||
|
|||||||
@@ -419,8 +419,7 @@ disable=raw-checker-failed,
|
|||||||
too-many-instance-attributes,
|
too-many-instance-attributes,
|
||||||
duplicate-code,
|
duplicate-code,
|
||||||
import-outside-toplevel,
|
import-outside-toplevel,
|
||||||
too-few-public-methods,
|
too-few-public-methods
|
||||||
unnecessary-ellipsis
|
|
||||||
|
|
||||||
# Enable the message, report, category or checker with the given id(s). You can
|
# Enable the message, report, category or checker with the given id(s). You can
|
||||||
# either give multiple identifier separated by comma (,) or put this option
|
# either give multiple identifier separated by comma (,) or put this option
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ follow-up tracked separately)."""
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import fcntl
|
import fcntl
|
||||||
|
import io
|
||||||
import signal
|
import signal
|
||||||
import struct
|
import struct
|
||||||
import subprocess
|
import subprocess
|
||||||
@@ -68,7 +69,11 @@ def _read_winsize() -> tuple[int, int] | None:
|
|||||||
- tmux respawn-pane: tmux sets all three to the pane's PTY.
|
- tmux respawn-pane: tmux sets all three to the pane's PTY.
|
||||||
- non-TTY (someone piped stdin in tests): none are; the
|
- non-TTY (someone piped stdin in tests): none are; the
|
||||||
sync just no-ops, which is the right behavior."""
|
sync just no-ops, which is the right behavior."""
|
||||||
for fd in (sys.stdin.fileno(), sys.stdout.fileno(), sys.stderr.fileno()):
|
for default_fd, stream in enumerate((sys.stdin, sys.stdout, sys.stderr)):
|
||||||
|
try:
|
||||||
|
fd = stream.fileno()
|
||||||
|
except (AttributeError, io.UnsupportedOperation, OSError):
|
||||||
|
fd = default_fd
|
||||||
try:
|
try:
|
||||||
data = fcntl.ioctl(fd, termios.TIOCGWINSZ, b"\x00" * 8)
|
data = fcntl.ioctl(fd, termios.TIOCGWINSZ, b"\x00" * 8)
|
||||||
except OSError:
|
except OSError:
|
||||||
|
|||||||
@@ -42,7 +42,8 @@ def filter_select(
|
|||||||
# Use os.dup() to duplicate the fd so the original file object
|
# Use os.dup() to duplicate the fd so the original file object
|
||||||
# and FileIO in _run_picker each manage independent copies,
|
# and FileIO in _run_picker each manage independent copies,
|
||||||
# preventing double-close errors.
|
# preventing double-close errors.
|
||||||
fd_dup = os.dup(tty_fd.fileno())
|
import os as _os
|
||||||
|
fd_dup = _os.dup(tty_fd.fileno())
|
||||||
return _run_picker(items, title=title, tty_fd=fd_dup)
|
return _run_picker(items, title=title, tty_fd=fd_dup)
|
||||||
finally:
|
finally:
|
||||||
tty_fd.close()
|
tty_fd.close()
|
||||||
|
|||||||
@@ -15,8 +15,8 @@ from datetime import datetime, timezone
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import cast
|
from typing import cast
|
||||||
|
|
||||||
from .log import die
|
from bot_bottle.log import die
|
||||||
from .util import expand_tilde
|
from bot_bottle.util import expand_tilde
|
||||||
|
|
||||||
|
|
||||||
def codex_auth_path(host_env: dict[str, str] | None = None) -> Path:
|
def codex_auth_path(host_env: dict[str, str] | None = None) -> Path:
|
||||||
@@ -153,7 +153,9 @@ def _dummy_jwt_from_host(
|
|||||||
return _dummy_jwt(now, exp_ts=exp_ts)
|
return _dummy_jwt(now, exp_ts=exp_ts)
|
||||||
if not isinstance(payload, dict):
|
if not isinstance(payload, dict):
|
||||||
return _dummy_jwt(now, exp_ts=exp_ts)
|
return _dummy_jwt(now, exp_ts=exp_ts)
|
||||||
return _encode_dummy_jwt(_redact_jwt_payload(cast(dict[str, object], payload), now=now, exp_ts=exp_ts))
|
return _encode_dummy_jwt(
|
||||||
|
_redact_jwt_payload(cast(dict[str, object], payload), now=now, exp_ts=exp_ts)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _encode_dummy_jwt(payload: dict[str, object]) -> str:
|
def _encode_dummy_jwt(payload: dict[str, object]) -> str:
|
||||||
|
|||||||
@@ -161,7 +161,7 @@ class Agent:
|
|||||||
git_raw = d.get("git-gate")
|
git_raw = d.get("git-gate")
|
||||||
if git_raw is not None:
|
if git_raw is not None:
|
||||||
gd = as_json_object(git_raw, f"agent '{name}' git-gate")
|
gd = as_json_object(git_raw, f"agent '{name}' git-gate")
|
||||||
for k in gd:
|
for k in gd.keys():
|
||||||
if k != "user":
|
if k != "user":
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"agent '{name}' git-gate.{k} is not allowed at the "
|
f"agent '{name}' git-gate.{k} is not allowed at the "
|
||||||
|
|||||||
@@ -246,7 +246,7 @@ class GitUser:
|
|||||||
@classmethod
|
@classmethod
|
||||||
def from_dict(cls, bottle_name: str, raw: object) -> "GitUser":
|
def from_dict(cls, bottle_name: str, raw: object) -> "GitUser":
|
||||||
d = as_json_object(raw, f"bottle '{bottle_name}' git-gate.user")
|
d = as_json_object(raw, f"bottle '{bottle_name}' git-gate.user")
|
||||||
for k in d:
|
for k in d.keys():
|
||||||
if k not in {"name", "email"}:
|
if k not in {"name", "email"}:
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{bottle_name}' git-gate.user has unknown key {k!r}; "
|
f"bottle '{bottle_name}' git-gate.user has unknown key {k!r}; "
|
||||||
@@ -281,7 +281,7 @@ def parse_git_gate_config(
|
|||||||
raw: object,
|
raw: object,
|
||||||
) -> tuple[tuple[GitEntry, ...], GitUser]:
|
) -> tuple[tuple[GitEntry, ...], GitUser]:
|
||||||
d = as_json_object(raw, f"bottle '{bottle_name}' git-gate")
|
d = as_json_object(raw, f"bottle '{bottle_name}' git-gate")
|
||||||
for k in d:
|
for k in d.keys():
|
||||||
if k not in {"user", "repos"}:
|
if k not in {"user", "repos"}:
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{bottle_name}' git-gate has unknown key {k!r}; "
|
f"bottle '{bottle_name}' git-gate has unknown key {k!r}; "
|
||||||
|
|||||||
@@ -1,186 +0,0 @@
|
|||||||
# PRD 0052: User-defined agent provider plugins
|
|
||||||
|
|
||||||
- **Status:** Draft
|
|
||||||
- **Author:** claude
|
|
||||||
- **Created:** 2026-06-04
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
The `get_provider()` registry in `bot_bottle/agent_provider.py` is a closed list —
|
|
||||||
only `"claude"` and `"codex"` are valid templates, validated at manifest-load time and
|
|
||||||
again at launch. Users who want to run a different agent (Gemini, Aider, a custom
|
|
||||||
local model wrapper) cannot add a provider without forking the package.
|
|
||||||
|
|
||||||
This PRD opens the registry to user-defined plugins. A plugin placed at
|
|
||||||
`~/.bot-bottle/contrib/<name>/agent_provider.py` is discovered and loaded at launch
|
|
||||||
time. The manifest accepts any non-empty template string that names a built-in or
|
|
||||||
resolves to a user plugin at that path. No changes to the built-in providers or the
|
|
||||||
internal `bot_bottle/contrib/` layout.
|
|
||||||
|
|
||||||
The preceding commit on this PR moves `codex_auth.py` from `bot_bottle/` into
|
|
||||||
`bot_bottle/contrib/codex/` — a clean-up that fits naturally here since this PR
|
|
||||||
also clarifies that `contrib/` is the per-provider home.
|
|
||||||
|
|
||||||
## Problem
|
|
||||||
|
|
||||||
Users building unconventional setups hit a hard wall: the template validation in
|
|
||||||
`manifest_agent.AgentProvider.from_dict` rejects any string not in `PROVIDER_TEMPLATES`.
|
|
||||||
There is no escape hatch short of editing bot-bottle's source.
|
|
||||||
|
|
||||||
PRD 0050 moved provider logic into `contrib/` specifically so a third provider would
|
|
||||||
be "cheap to add" — but "cheap" today still means a pull request against the bot-bottle
|
|
||||||
repo, not a drop-in file in the user's home directory. The filesystem layout is already
|
|
||||||
the right shape; the discovery step is missing.
|
|
||||||
|
|
||||||
## Goals / Success Criteria
|
|
||||||
|
|
||||||
1. A user places `~/.bot-bottle/contrib/<name>/agent_provider.py` — a file that exports
|
|
||||||
a class inheriting `AgentProvider` — sets `agent_provider.template: <name>` in a
|
|
||||||
bottle's frontmatter, and launches a bottle using that provider with no changes to
|
|
||||||
the bot-bottle source.
|
|
||||||
2. The manifest validator accepts any non-empty template string. Unknown templates that
|
|
||||||
resolve to no user plugin still raise a clear error, but at launch (via `get_provider`)
|
|
||||||
rather than at manifest-load time.
|
|
||||||
3. Built-in provider knobs (`auth_token` → claude only; `forward_host_credentials` →
|
|
||||||
codex only) are guarded to built-in template names. Bottles using a user provider
|
|
||||||
may set neither knob.
|
|
||||||
4. `get_provider(template)` checks `~/.bot-bottle/contrib/<template>/agent_provider.py`
|
|
||||||
before the built-ins, so a user can shadow a built-in for local testing.
|
|
||||||
5. A clear `ValueError` is raised if the user plugin file exists but contains no
|
|
||||||
`AgentProvider` subclass.
|
|
||||||
|
|
||||||
## Non-goals
|
|
||||||
|
|
||||||
- Packaging or distributing user plugins as installable Python packages.
|
|
||||||
- A plugin registry, index, or discovery beyond the filesystem path convention.
|
|
||||||
- Adding a third built-in provider.
|
|
||||||
- Changing the `AgentProvider` ABC contract — user plugins implement the same abstract
|
|
||||||
methods as `ClaudeAgentProvider` and `CodexAgentProvider`.
|
|
||||||
- Validating that user plugin images, Dockerfiles, or commands exist before launch
|
|
||||||
(same policy as built-ins).
|
|
||||||
- Sandboxing user plugin code — plugins run with full Python interpreter access.
|
|
||||||
|
|
||||||
## Scope
|
|
||||||
|
|
||||||
### In scope
|
|
||||||
|
|
||||||
- `get_provider(template: str) -> AgentProvider` gains a `_load_user_plugin(template)`
|
|
||||||
step that checks `~/.bot-bottle/contrib/<template>/agent_provider.py` first, then
|
|
||||||
falls through to the built-in look-ups.
|
|
||||||
- `_load_user_plugin` uses `importlib.util.spec_from_file_location` to load the module
|
|
||||||
and returns the first `AgentProvider` subclass found in its `__dict__`. Raises
|
|
||||||
`ValueError` if the file exists but exports no subclass.
|
|
||||||
- `manifest_agent.AgentProvider.from_dict`: the `template not in PROVIDER_TEMPLATES`
|
|
||||||
check is removed; the two built-in-specific knob guards (`auth_token` → claude,
|
|
||||||
`forward_host_credentials` → codex) are tightened to `template in PROVIDER_TEMPLATES`
|
|
||||||
so they are skipped for user-defined names.
|
|
||||||
- `PROVIDER_TEMPLATES` remains in `agent_provider.py` as the set of built-in names for
|
|
||||||
use by tests and any enumeration callers.
|
|
||||||
- Unit tests for the discovery path:
|
|
||||||
- Plugin found and loaded → correct `AgentProvider` instance returned.
|
|
||||||
- Plugin file exists but exports no subclass → `ValueError`.
|
|
||||||
- Unknown template with no user plugin → `ValueError` from `get_provider`.
|
|
||||||
- Built-in template name still works normally even when no user plugin exists.
|
|
||||||
- One paragraph added to `README.md` under a new "Custom providers" section describing
|
|
||||||
the `~/.bot-bottle/contrib/<name>/agent_provider.py` convention and pointing at the
|
|
||||||
existing contrib providers as reference implementations.
|
|
||||||
|
|
||||||
### Out of scope
|
|
||||||
|
|
||||||
- Hot-reloading plugins during a running session.
|
|
||||||
- Plugin versioning or dependency declaration.
|
|
||||||
- Changes to smolmachines or Docker backend provisioning paths.
|
|
||||||
|
|
||||||
## Proposed design
|
|
||||||
|
|
||||||
### Discovery in `get_provider`
|
|
||||||
|
|
||||||
```python
|
|
||||||
import importlib.util
|
|
||||||
|
|
||||||
def get_provider(template: str) -> AgentProvider:
|
|
||||||
user_plugin = _load_user_plugin(template)
|
|
||||||
if user_plugin is not None:
|
|
||||||
return user_plugin
|
|
||||||
if template == PROVIDER_CLAUDE:
|
|
||||||
from .contrib.claude.agent_provider import ClaudeAgentProvider
|
|
||||||
return ClaudeAgentProvider()
|
|
||||||
if template == PROVIDER_CODEX:
|
|
||||||
from .contrib.codex.agent_provider import CodexAgentProvider
|
|
||||||
return CodexAgentProvider()
|
|
||||||
raise ValueError(f"unknown agent provider template: {template!r}")
|
|
||||||
|
|
||||||
|
|
||||||
def _load_user_plugin(template: str) -> AgentProvider | None:
|
|
||||||
plugin_path = (
|
|
||||||
Path.home() / ".bot-bottle" / "contrib" / template / "agent_provider.py"
|
|
||||||
)
|
|
||||||
if not plugin_path.exists():
|
|
||||||
return None
|
|
||||||
spec = importlib.util.spec_from_file_location(
|
|
||||||
f"_user_contrib_{template}.agent_provider", plugin_path
|
|
||||||
)
|
|
||||||
if spec is None or spec.loader is None:
|
|
||||||
raise ValueError(f"user plugin at {plugin_path} could not be loaded")
|
|
||||||
mod = importlib.util.module_from_spec(spec)
|
|
||||||
spec.loader.exec_module(mod) # type: ignore[union-attr]
|
|
||||||
for obj in vars(mod).values():
|
|
||||||
if (
|
|
||||||
isinstance(obj, type)
|
|
||||||
and issubclass(obj, AgentProvider)
|
|
||||||
and obj is not AgentProvider
|
|
||||||
):
|
|
||||||
return obj()
|
|
||||||
raise ValueError(
|
|
||||||
f"user plugin at {plugin_path} defines no AgentProvider subclass"
|
|
||||||
)
|
|
||||||
```
|
|
||||||
|
|
||||||
### Manifest validation change
|
|
||||||
|
|
||||||
In `manifest_agent.AgentProvider.from_dict`, remove the hard rejection:
|
|
||||||
|
|
||||||
```python
|
|
||||||
# Before
|
|
||||||
if template not in PROVIDER_TEMPLATES:
|
|
||||||
raise ManifestError(
|
|
||||||
f"bottle '{bottle_name}' agent_provider.template {template!r} "
|
|
||||||
f"is not one of {', '.join(sorted(PROVIDER_TEMPLATES))}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# After — removed entirely; get_provider() raises at launch for unknown names
|
|
||||||
```
|
|
||||||
|
|
||||||
Guard the built-in knob checks with `template in PROVIDER_TEMPLATES`:
|
|
||||||
|
|
||||||
```python
|
|
||||||
if auth_token and template == "claude": # unchanged
|
|
||||||
...
|
|
||||||
if auth_token and template not in PROVIDER_TEMPLATES:
|
|
||||||
raise ManifestError(
|
|
||||||
f"bottle '{bottle_name}' agent_provider.auth_token is only "
|
|
||||||
f"supported for built-in templates ({', '.join(sorted(PROVIDER_TEMPLATES))})"
|
|
||||||
)
|
|
||||||
if forward_host_credentials and template == "codex": # unchanged
|
|
||||||
...
|
|
||||||
if forward_host_credentials and template not in PROVIDER_TEMPLATES:
|
|
||||||
raise ManifestError(
|
|
||||||
f"bottle '{bottle_name}' agent_provider.forward_host_credentials "
|
|
||||||
f"is only supported for built-in templates"
|
|
||||||
)
|
|
||||||
```
|
|
||||||
|
|
||||||
## Open questions
|
|
||||||
|
|
||||||
1. **Shadow order.** This PRD puts user plugins before built-ins, allowing local
|
|
||||||
overrides. If the preference is built-ins-first (to prevent accidental shadowing),
|
|
||||||
swap the order and document accordingly.
|
|
||||||
2. **`BOT_BOTTLE_CONTRIB_DIR` env var.** Omitted for now — `~/.bot-bottle/contrib/`
|
|
||||||
is consistent with the rest of the user config layout. Revisit if the need surfaces.
|
|
||||||
|
|
||||||
## References
|
|
||||||
|
|
||||||
- PRD 0050 — agent provider contrib (established `contrib/` as the per-provider home)
|
|
||||||
- PRD 0048 — SSH deploy key provisioning (the `contrib/` convention)
|
|
||||||
- `bot_bottle/agent_provider.py` — `get_provider`, `PROVIDER_TEMPLATES`, `AgentProvider` ABC
|
|
||||||
- `bot_bottle/manifest_agent.py` — template validation that this PRD relaxes
|
|
||||||
Reference in New Issue
Block a user