* test(tools): add shim protection test for tool_implementations split
Covers all 48 top-level functions (33 do_* + 15 _helpers) extracted from
the original module. Guards the upcoming split: the shim must re-export
every symbol so existing 'from src.tool_implementations import X' imports
keep working. Passes on baseline (pre-split).
* refactor(tools): add src/tools/ package with shared _common
Slice 1 Task 2 (#4082/#4071). Adds the package skeleton and moves the
shared _parse_tool_args helper into src/tools/_common.py. Domain modules
will import from here. tool_implementations.py is untouched at this step.
* refactor(tools): extract system domain into src/tools/system.py
Slice 1 (#4082/#4071), Task 3: move the system-domain tool functions
(do_manage_skills/_skill_dump/do_manage_tasks/do_manage_endpoints/
do_manage_mcp/do_manage_webhooks/do_manage_tokens/do_manage_settings/
do_api_call/do_app_api) and the app_api blocklist constants out of
tool_implementations.py into a new src/tools/system.py module.
tool_implementations.py re-imports all of them so it stays a working
backward-compatible facade (shim test stays green).
- do_manage_mcp resolves get_mcp_manager via a function-local import
from tool_implementations so the test that patches
src.tool_implementations.get_mcp_manager still applies post-move.
- do_app_api imports _internal_headers and _INTERNAL_BASE (still in
tool_implementations) function-locally to avoid a circular import.
- Repoint test_context_budget introspection assertion to the moved
code's new home in src/tools/system.py.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(tools): extract cookbook domain into src/tools/cookbook.py
Moves the model-serving (cookbook) tool domain out of tool_implementations.py
into src/tools/cookbook.py as part of slice 1 (#4082/#4071):
- 13 do_* tools: download/serve/list/stop/tail/search/adopt/cached models,
list downloads/cancel, list cookbook servers, serve presets
- 9 private helpers: _cookbook_servers, _resolve_cookbook_host,
_cookbook_env_for_host, _infer_serve_{port,host}, _ensure_served_endpoint,
_cookbook_register_task, _cookbook_apply_retry_suggestion,
_scan_running_model_processes, _cookbook_kill_session
- _MODEL_PROCESS_PATTERNS constant (used only by _scan_running_model_processes)
tool_implementations.py stays a backward-compatible facade via a re-import
from src.tools.cookbook; src/tools/__init__ re-exports the same symbols.
_internal_headers and _INTERNAL_BASE stay in tool_implementations.py (shared
by system.py's do_app_api and many cookbook funcs). Each cookbook function
that needs them does a function-local import to avoid a top-level circular
dependency, matching the system-domain split.
Verified: compileall clean; shim test green; cookbook-touching suite
(652 passed, 1 skipped); full suite 3587 passed, 2 failed
(pre-existing test_api_chat_security, unrelated).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(tools): extract search domain into src/tools/search.py
* refactor(tools): extract notes domain into src/tools/notes.py
* refactor(tools): extract calendar domain into src/tools/calendar.py
Repoints tests/test_caldav_bidirectional_sync.py source-introspection
to src/tools/calendar.py (do_manage_calendar moved there).
* refactor(tools): extract image domain into src/tools/image.py
* refactor(tools): extract research domain into src/tools/research.py
* refactor(tools): extract contacts domain into src/tools/contacts.py
* refactor(tools): extract vault domain into src/tools/vault.py
Repoints tests/test_vault_password_not_in_argv.py source-introspection
to src/tools/vault.py (the vault do_* helpers moved there).
* refactor(tools): collapse tool_implementations to clean re-export shim
Move shared _INTERNAL_BASE/_internal_headers to src/tools/_common.py and
drop the duplicate _parse_tool_args (already in _common). tool_implementations.py
is now a pure re-export facade (+ 3 pre-existing email-context helpers, out of
scope). Domain files' function-local imports of these names still resolve via
the facade re-export.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tools): port upstream cookbook workflow changes to split module
Rebase onto dev dropped c504214 ("Cookbook model workflow fixes") edits
to do_serve_model / do_tail_serve_output: the extraction commit moved
the pre-edit bodies into src/tools/cookbook.py and git auto-accepted the
deletion from tool_implementations.py, losing dev's changes. Restore them
in their post-split home:
- do_serve_model: add where/log_path/next_tools and the expanded
"Next required check" output message
- do_tail_serve_output: empty-output fallback message replacing
"(empty pane)"
(do_manage_settings web_fetch alias edit was already applied to
src/tools/system.py during the system-extract conflict resolution.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tools): break admin_tools circular import in split facade
After rebasing onto dev (#3629 moved the admin manage_* tools into
src/agent_tools/admin_tools), the facade re-exported them via a top-level
`from src.agent_tools.admin_tools import ...`. But src.agent_tools.__init__
imports this facade at top level, so the eager import re-entered the
partially-initialized agent_tools package and broke collection.
Re-export the admin symbols (do_manage_endpoints/mcp/webhooks/tokens/
settings, _MCP_DENIED_COMMANDS, _validate_mcp_command) lazily through
module __getattr__ instead, and drop them from src/tools/__init__ (they
no longer live in the src.tools package). system.py now holds only the
skills/tasks/api bridges; admin tools live solely in admin_tools.py,
matching upstream.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tools): re-export dropped helpers through the split shim
Address review finding from #4423: the compatibility facade claimed to
preserve every original top-level symbol but omitted three helpers the
old src.tool_implementations exposed. Re-export them and pin them in
the shim protection test:
- _string_arg, _validate_cookbook_ssh_target <- src/tools/cookbook.py
- _mcp_allowed_commands <- src/agent_tools/admin_tools.py (lazily via
__getattr__, to keep the agent_tools.__init__ <-> facade import acyclic
after the #3629 admin-tools migration)
All three added to tests/test_tool_implementations_shim.py _EXPECTED so
the test contract now matches its "every original top-level function"
comment.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(tools): self-verify shim re-exports every domain do_*
The hand-maintained _EXPECTED list in the shim protection test can drift
silently when a new tool is added to a domain module but not re-exported
by the facade — exactly the omission a reviewer flagged post-split.
Add an auto-discovering test that enumerates every do_* from the domain
modules (incl. admin_tools) and asserts reachability through the shim,
so a forgotten re-export fails the build automatically.
Uses hasattr (not dir(ti)) because the admin symbols are re-exported
lazily via module __getattr__ and don't appear in dir(ti).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(tools): self-verify every in-repo facade import resolves
RaresKeY's P3 on the shim test was a claim-vs-reality gap: the docstring
said it protected "every from src.tool_implementations import X" but the
hand-maintained _EXPECTED list omitted three underscore helpers, so the
claim wasn't enforced. Re-exporting the three (cf1f5e3) fixed the known
gap; this closes the structural one.
Add test_every_facade_import_in_repo_resolves: ast-enumerate every
`from src.tool_implementations import X` site in src/ and tests/ and
assert hasattr(ti, X) for each. A forgotten re-export that anything in
the repo imports now fails the build automatically — including underscore
helpers, which the do_* discovery test does not cover.
Together with test_shim_reexports_every_domain_do_function, the shim
contract is now self-verifying. Demote _EXPECTED in the docstring to the
curated historical/downstream surface (the three helpers have no in-repo
consumer, so they stay manual by necessity) instead of "ground truth".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(tools): dedupe _parse_tool_args + align shim guard with route consumers
Addresses two P3s from review (RaresKeY, 2026-06-26):
1. maintainability — _common carried a full copy of _parse_tool_args
alongside the canonical src.tool_utils one; future parser fixes could
diverge. The two bodies were byte-identical in logic, so _common now
re-exports from tool_utils (a leaf module, no circular-import risk).
The single-source test is extended to assert _common._parse_tool_args
and tool_implementations._parse_tool_args are the same object as
tool_utils._parse_tool_args.
2. test — the shim guard's import-site scan only walked src/ and tests/,
missing routes/chat_routes.py's clear_active_email/set_active_email
imports, and _EXPECTED omitted the active-email facade helpers. The
scan now walks every first-party Python dir (pruning venvs/caches/data
in-place), and set/get/clear_active_email are added to _EXPECTED
(get_active_email has no in-repo importer, so the scan alone can't see
it).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: yuandonghao <yuandonghao@cohl.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Part of #3629 (the `admin_tools.py` bullet). Moves the config/integration admin
tools off the legacy elif dispatch chain in tool_implementations.py onto the
agent_tools registry:
manage_endpoints, manage_mcp, manage_webhooks, manage_tokens, manage_settings
The do_* implementations (and manage_mcp's command-allowlist / RCE guard:
_validate_mcp_command, _mcp_allowed_commands, and the _MCP_* constants) move
verbatim into the new src/agent_tools/admin_tools.py. They register through a
single ADMIN_TOOL_HANDLERS map that TOOL_HANDLERS.update()s, and the five elif
branches plus their imports are dropped from tool_execution.py, so these tools
now flow through _direct_fallback like the other migrated clusters. The names
are re-exported from src.agent_tools for back-compat.
Dedup:
- _parse_tool_args was duplicated in tool_implementations.py and
document_tools.py. It now lives once in src.tool_utils (which imports nothing
from the project beyond src.constants, so this introduces no cycle) and both
call sites import it from there. The orphaned `import json` in document_tools
is removed with it.
- The five tools share one _owner_adapter(fn) factory that threads ctx["owner"]
into the owner-taking do_* signature, instead of five near-identical wrappers.
Tests: new tests/test_admin_tools_registry.py pins the registration, the
re-export back-compat, the owner-threading adapter, and the single-source
_parse_tool_args (across admin_tools and document_tools). Existing MCP /
settings / webhook suites are repointed at the new module.
* fix(security): allowlist manage_mcp 'add' to close the agent-path RCE
do_manage_mcp('add') passed model- and prompt-injection-controlled command,
args, and env straight to a stdio subprocess spawn with no validation, and it
persisted an enabled server row before connecting (so a payload also survived
to re-execute on restart). A string smuggled into a skill description, memory
entry, fetched page, or email body could register a server running arbitrary
code as the app UID, e.g. command='sh' args=['-c','...'].
Add _validate_mcp_command, applied on the agent path before any DB write or
spawn:
- Hard-deny interpreters, runtimes, package runners, shells, and exec-wrappers
(even if an operator lists one in ODYSSEUS_MCP_ALLOWED_COMMANDS).
- Require a bare basename (no path components, no shell metacharacters) that is
present in the operator allowlist (empty by default).
- Reject code-exec argv flags by prefix so glued forms are caught too
(-c/-e/-m/--eval/--exec/--print/--module/--command/--require), remote-URL
args, and env keys that inject code into the child (LD_PRELOAD, NODE_OPTIONS,
PYTHONPATH, DYLD_*, PATH, ...).
A rejected registration returns an error, writes no row, and makes no
connection. The trusted admin route is unchanged. Mirrors the policy intent of
_validate_serve_cmd but inverted for the model-reachable surface.
Supersedes #438; incorporates the bypass forms found in its review (interpreter
script paths, -m pip, glued -c/-e, --eval=, eval subcommands, package runners,
remote URLs) and adds integration coverage on the real do_manage_mcp path.
Closes#2891
* fix(security): deny versioned/alias runtimes in manage_mcp allowlist
Addresses RaresKeY's review on #4433. The hard-deny matched command names
exactly, so versioned or alias runtime forms (python3.11, node18, pip3,
ruby3.2, java, javac, bunx, tsx, ts-node, pypy3, ...) slipped past and, if an
operator allowlisted one, re-opened the prompt-injection-controlled MCP
registration path.
- Canonicalize a trailing version suffix before the deny check so versioned
forms collapse to the family (python3.11 -> python, node18 -> node, pip3 ->
pip); both the raw basename and the canonical form are denied.
- Broaden the denied-family set (java/javac/jshell/jbang/kotlin/dotnet/mono/
swift/osascript/tsx/ts-node/bunx/pypy/jruby/raku/luajit/wish/expect/iex).
Deny runs before the operator allowlist, so an alias cannot be allowlisted back
in. Canonicalization only feeds the deny check, so a legit name that ends in a
digit still reaches the normal allowlist check rather than being mis-denied.
Adds validator + integration regressions for versioned/alias runtimes asserting
no DB row and no connection, including the allowlisted-anyway case.
When a CardDAV contact matched the search query but had no email
address (only phone numbers), the tool silently dropped it and
returned 'No contacts found'. Fall back to the contact's phone
number(s) so the caller still receives usable information.
Refs: #4178 (the contacts-domain classifier fix that made the model
actually call resolve_contact for contacts queries, surfacing this
pre-existing gap)
_reminder_minutes matched the offset with (?:m|min|minute|minutes)\b and
(?:h|hr|hour|hours)\b. The trailing \b makes the common plural
abbreviations "mins"/"hrs" fail to match (after "min" the "s" is a word
char, so no boundary), so reminder_minutes "5 mins" or "2 hrs" returned
None and the event was created with no reminder, silently.
Widen the two unit regexes and the matching reminder_only description
regex to a strict superset that also accepts mins/hrs. The sibling
duration parser already accepts these forms (it has no \b), so this only
brings the reminder parser in line.
- Agent: pass the open email reader (uid/folder/account/from/subject/body
preview) on every chat submit so 'reply to this' / 'write email saying
hi' route to ui_control open_email_reply with the right UID instead of
inventing a new .md draft. Code-level enforcement (chat_routes strips
create_document + send_email when active_email is set); cross-session
active_doc_id is now trusted instead of being silently dropped.
set_active_email/clear_active_email tool-layer helpers in
tool_implementations.
- ui_control open_email_reply: optional body argument so the agent can
open-and-write in one call; envelope now forwards uid/folder/account/
body/panel through tool_output. Tool description sharpened and the
parser rejects empty bodies on reply/reply-all (forces the agent to
write rather than open an empty draft).
- Email library: search now runs against [Gmail]/All Mail when the
current folder is INBOX (archived emails surface). Whirlpool spinner
+ 'Searching…' placeholder while in flight. Each search result is
stamped with its source folder so clicks open the right email instead
of whatever shares its UID in INBOX. Search no longer re-applies the
same text pill locally (which only checks subject/from/snippet, never
body) so body-only matches don't get dropped after IMAP returns them.
Initial inbox load bumped 100→500.
- Email favorites: 'Favorite (pin to top)' / 'Unfavorite' in both the
card menu and the open-reader more menu, backed by a new
/api/email/flag/{uid}?on=true|false endpoint. Flagged emails always
bubble to the top of the grid regardless of active sort.
- AI reply in doc editor: never overwrites existing draft text or the
quoted history. AI suggestion is prepended; AI-generated 'On …
wrote:' re-quotes are stripped so the original quote isn't visually
edited.
- Cookbook serve: pre-launch GPU driver / has_gpu / install / version-
floor checks (vllm minimax_m2 needs 0.10.0+, deepseek_r1 needs 0.7.0
etc.) before the launch chain starts. Detect 'another model already
running on this host' and offer Stop & launch (with graceful then
force tmux kill helpers, port release wait). Per-vendor deep-link
buttons (vLLM recipe / SGLang cookbook) with hardware hash. Backend
picker is now a custom dropdown with accent-coloured logos for vLLM,
SGLang, llama.cpp, Ollama, Diffusers; same glyphs added next to
package names in Dependencies. Runtime-readiness note moved inside
the panel (green when ready, red when missing) with an × dismiss.
Esc collapses the expanded card; expanded card scrolls when it
overflows; Trust Remote / Auto Tool / Reasoning Parser / Enforce
Eager / Prefix Caching / Expert Parallel / Speculative / MoE Env on
one row (Reasoning Parser auto-detected per model family).
Dtype→Row 1, GPUs→Row 2 (rightmost). Removed redundant GPU 'auto'
input — command builders read from the GPU button strip. Default
cookbook open is Download tab.
- Cookbook hwfit: 'Model (latest)' / 'Model (oldest)' header sorts by
release_date; release dates can be backfilled with the new
scripts/backfill_model_release_dates.py and recipe metadata pulled
with scripts/import_from_vllm_recipes.py against the upstream
vllm-project/recipes catalog (vllm_recipe + min_vllm_version stamped
on entries).
- Calendar: Quick add hint cycles a random Odysseus-themed example per
open (wooden horse Friday, crew muster 10am daily, council on
Ithaca, …). Typing a time like '11pm' in the event title updates
the hero clock live.
- Doc editor: email-mode Reply button (sparkle icon, accent) opens the
same Fast/Full + context popover the email reader uses; Ctrl+Alt+M
toggles markdown preview.
- Memories panel: custom sort picker with per-option icons, default
'Latest', visible Enabled/Disabled toggle text matching the section
description style.
Closes the gap that pushed the agent into manage_memory when the user
pasted an address and said 'save this for X'. manage_contact now
accepts an optional address arg end-to-end:
- routes/contacts_routes.py:
- _normalize_contact carries an 'address' field
- _build_vcard emits ADR:;;<address>;;;; (street component of the
RFC-6350 7-part ADR), only when address is non-empty
- _parse_vcards reads ADR, joins non-empty components with ', '
- _create_contact and _update_contact thread address through;
update preserves existing address when caller passes empty
- src/tool_implementations.py do_manage_contact:
- add accepts address; require at least name+address or email
(was: email required) so address-only contacts are addable
- update accepts address; require name OR emails OR address
- src/tool_schemas.py: schema gets a single 'address' string field
- src/tool_index.py + src/agent_loop.py: descriptions get one
'address' arg mention and a 'use this for save-X-for-person /
address pastes / phone-with-name' steering line. Net: a few
bytes added, not a paragraph.
Also: removed a stray name from the schema's manage_contact example
strings ('save Jonathan's email…') — no real names in the codebase.
* fix: handle batch events format in manage_calendar tool
Models like deepseek-v4-flash emit batch events array instead of individual create_event calls. The tool defaulted to list_events (no action key), so events were never created despite the model confirming success.
- Add batch normalization in do_manage_calendar
- Map start/end objects to flat dtstart/dtend strings
- Add tests for both object and flat string formats
* fix: surface partial batch failures in manage_calendar
Partial failures were silently dropped - batches with mixed success/failure would report only created count with no error visibility.
- Return non-zero exit code for any failures
- Surface both created and failed counts in response
- Include first error message for debugging
- Add test for partial failure case
* chore: strip trailing whitespace in batch normalization block
* chore: strip whitespace-only blank lines in batch events test
Two months of iteration on the Settings panel, integration forms, and
small visual nudges across the app. Highlights:
Settings restructure
- Add Models: split into separate Local + API cards (no more in-card
tabs); each fuses Type/Provider with the URL input.
- Added Models: new dedicated sidebar tab, with Probe + Clear-offline
pulled into its header; Local/API sub-section icons accent-tinted.
- Search: Web Search and a new Deep Research card (Model + tuning),
with a cross-link to AI Defaults. Provider hints use real clickable
anchors; Web Search Test button shows a whirlpool spinner.
- AI Defaults: Image Generation card returns; Research Model card
carries only Endpoint+Model with a cross-link to Search; Vision /
Default / Utility fallbacks unified under one numbered-row design
matching Search's chain.
- API Permissions (was 'API Tokens'): per-row rename, inline
Permissions toggle that expands the scope-edit panel, in-field
copy icons (icon→check on success). Empty state accent-tinted.
- Integrations: + Add Integration drops a type-picker menu directly
under the button (drop-up on tight viewports); each integration
form (API, CalDAV, CardDAV, Email, Codex/Claude, Vault, MCP) uses
the same accent-outlined Save/Test/Cancel buttons right-aligned.
- Danger Zone: Wipe→Delete with trash icons; new 'Delete everything'
row at the bottom that loops every category.
AI Synthesis (Reminders)
- Persona dropdown sourced from PROMPT_TEMPLATES + custom preset.
- src/reminder_personas.py mirrors the five built-ins for the
server-side synthesis path.
- dispatch_reminder() reads reminder_llm_persona and uses the
persona's system prompt; empty/unknown falls back to warm-neutral.
Esc handling
- Kebab menus and the provider picker intercept Esc in capture phase
so dismissing a popup no longer closes the whole Settings modal.
Accent tinting
- Scoped CSS rule across data-settings-panel=ai/services/added-models/
search/integrations/reminders for card h2 icons + the Added Models
sub-section icons.
Codex/Claude integration form
- No more auto-creation on form open — explicit Create token button.
- New tokens start with every scope granted; existing tokens move out
of the integration form into the API Permissions card.
- Setup reveal: copy buttons inline inside the token + setup code
blocks; shorter subtitle wording.
Misc visual polish
- Save/Test/Cancel uniformly accent-outlined and right-aligned on
every integration form.
- Provider logos render inline next to the search fallback selects
and the Deep Research Search dropdown.
- Trash icons in fallback rows bumped to 20x20 so they fill the 32px
button.
- Image generation default flipped to off.
#3322 renamed the loopback base to _INTERNAL_BASE, but a later Cookbook
commit reintroduced one call site using the old _COOKBOOK_BASE name,
raising NameError whenever the agent registers a model endpoint for a
running serve session.
Fixes#3669
Surface a lot of accumulated cookbook + UI work as a single non-agent
commit so the agent rework lands cleanly.
Highlights:
- Ollama as a first-class backend in the Cookbook:
* Download input accepts ollama-style names (name:tag) → backend=ollama
* /api/cookbook/ollama/library (cached scrape of ollama.com + curated
fallback so classic models like qwen2.5 stay reachable)
* "Browse Ollama library" toggle below Download with size chips
* Engine=Ollama in hwfit toolbar merges the Ollama library into the
main scan list as per-tag rows with the same Fit/Param/Quant/VRAM
columns; click → fills Download input
- API Tokens form added to Integrations panel (matching wired
loadTokens()/initTokenForm() that had no HTML)
- Serve panel polish: Advanced fold tightening (-8px nudges on vLLM
checks, Extra args, Spec row), n_cpu_moe + Split Mode controls
pulled up 8px to align with the row's checkboxes, GGUF File dropdown
exposed for Ollama backend, GPU re-render on Edit serve restore,
_forceBackend flag so saved serveState wins over backend detection,
cookbook:servers-changed CustomEvent so panels don't need refresh
- Models page redesign: Add Models row (URL + hidden API key reveal +
Type select + Scan/Ollama/Key/Test/Add icon buttons), Probe All +
Clear-offline buttons in Added Models toolbar, offline-pill removed
(opacity already conveys state), Engine dropdown gains Ollama option
- _ping_endpoint probes /v1/models then base, accepts 4xx as
reachable (vLLM returns 404 on bare /v1, fully working endpoints
were showing offline)
- Diagnosis card: × dismiss + Copy bundle buttons restored on the
serve error feedback card
- Orphan tmux sweep re-enabled behind a 60s rate-limit + background
Thread (off the main event loop) so dead serves get discovered
- cookbook_routes auto-register watchdog: drops the endpoint if the
serve session exits non-zero within the first ~3min
- ollama-rocm sidecar awareness in download wrapper (`docker exec
ollama-rocm ollama pull` when host ollama isn't installed)
- Skill extractor sets initial_status="published" when
auto_approve_skills pref is on (audit demotes later)
- Skill list / model list / cookbook scan misc polish
* refactor(tools): consolidate duplicated _truncate and get_mcp_manager into src/tool_utils
Move all copies of _truncate(), get_mcp_manager(), and set_mcp_manager()
into a single leaf module (src/tool_utils.py) that imports only from
src.constants. This eliminates the lazy-import hack
('from src import agent_tools' inside function bodies) in tool_execution.py
and tool_implementations.py, and fixes a latent bug: the _truncate copy in
tool_execution.py was missing the isinstance guard and would crash on None.
Also deletes mcp_servers/_common.py — it was dead code with zero callers
anywhere in the codebase, containing its own copy of truncate() and
constants that already exist in src/constants.py.
* fix(tools): route remaining get_mcp_manager imports to src.tool_utils
The maintainer's feedback flagged src/task_scheduler.py:1857 and
routes/task_routes.py:977. A project-wide search found a third call site
in src/agent_loop.py that also imported get_mcp_manager from
src.agent_tools instead of src.tool_utils.
All three are now sourced from the canonical location in src.tool_utils.
---------
Co-authored-by: mcnoliveira <mcnoliveira@gmail.com>
* refactor(constants): single source of truth for data dir + merge core/src constants
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(contributing): use named src.constants for data paths, drop core/constants references
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#2753 made the agent loopback base port-configurable but only for
_COOKBOOK_BASE in tool_implementations. Several other in-process loopback
calls still hardcoded http://localhost:7000 and broke off port 7000:
cookbook_serve_lifecycle (model-endpoints x2, shell/exec), builtin_actions
(model/serve), task_routes (calendar x3), and the gallery/email calls in
tool_implementations.
Extract the resolution (ODYSSEUS_INTERNAL_BASE / APP_PORT / 7000 fallback,
127.0.0.1 to avoid IPv6 ambiguity) into core.constants.internal_api_base()
and route every call site through it. Rename the now-misnamed _COOKBOOK_BASE
to _INTERNAL_BASE since it serves gallery/email/calendar/serve too. Adds a
test for the resolver plus a regression guard against reintroducing the
literal.
Part of #2752.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
_COOKBOOK_BASE was hardcoded to http://localhost:7000 with no env-var
override anywhere in the codebase. Tools that do an internal HTTP
loopback (app_api, trigger_research, cookbook state read/write) silently
fail with "All connection attempts failed" whenever the running uvicorn
isn't on port 7000 — which is most non-default deployments and any
side-by-side multi-instance setup. The misleading "Task triggered"
message from manage_tasks during a research request hides that the
underlying research never starts.
Resolution order, lowest to highest priority:
1. Fallback http://127.0.0.1:7000 (preserves legacy default).
2. APP_PORT — derive http://127.0.0.1:$APP_PORT (matches docker-compose
which already reads APP_PORT).
3. ODYSSEUS_INTERNAL_BASE — explicit override (e.g. behind a TLS proxy
where loopback isn't 127.0.0.1).
127.0.0.1 instead of "localhost" avoids IPv6/DNS ambiguity for a
strictly-local call.
No API or schema change. Defaults preserved: existing setups on port
7000 are unaffected.
Caught by #2752.
Co-authored-by: pewdiepie-archdaemon <pewdiepie-archdaemon@users.noreply.github.com>
1. routes/personal_routes.py: os.path.exists() then os.remove() is a
classic TOCTOU race — another request or cleanup can delete the
file between the check and the remove, raising FileNotFoundError.
Replace with try/except FileNotFoundError.
2. src/tool_implementations.py: cmd.split()[0] crashes with IndexError
when cmd is a non-empty whitespace-only string (split() returns []).
Guard with (cmd.split() or [''])[0].
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MAX_OUTPUT_CHARS, MAX_READ_CHARS, and MAX_DIFF_LINES are now
defined once in src/constants.py and imported by the three files
that previously duplicated them (tool_execution.py,
tool_implementations.py, agent_tools.py). agent_tools.py re-exports
them for backward compatibility.
Co-authored-by: mcnoliveira <mcnoliveira@gmail.com>
Replaces any Discord-specific reminder channel with a generic outbound
webhook channel. Users pick any saved Integration as the target and
supply a JSON payload template with {{title}} and {{message}}
placeholders — values are JSON-escaped before substitution. Works with
Discord, Slack, Teams, ntfy (JSON mode), or any service that accepts a
POST with a JSON body.
- `src/settings.py` — reminder_webhook_integration_id +
reminder_webhook_payload_template defaults
- `routes/note_routes.py` — webhook delivery block; Integration lookup,
template rendering, auth wiring; built-in preset defaults so
discord_webhook works out of the box without a configured template;
settings_override kwarg avoids test-button race condition
- `routes/auth_routes.py` — discord_webhook preset test handler
- `src/integrations.py` — discord_webhook preset with description +
example templates; hides auth/key fields in the Integration form
- `src/builtin_actions.py` — webhook_sent delivery check
- `src/tool_implementations.py` — webhook aliases + enum updated
- `static/index.html` — Webhook channel option; Integration picker +
payload template textarea
- `static/js/settings.js` — Integration list, populateWebhookIntegrations,
syncChannelRows, hints, load/save, auto-fill preset templates,
test-button override payload, hide auth/key for URL-auth presets
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- Calendar overnight events render proportionally across day boundaries
via --start-frac / --end-frac CSS vars instead of bleeding as full-day
on day 2.
- Recurring-event delete strips the master uid + all master::* sibling
instances optimistically so the row clears immediately instead of
waiting for the next sync re-render.
- manage_notes(create) now returns note_id + open_url, and agent_loop
appends a markdown [View note](#note-<id>) link mirroring the
deep-research pattern.
- chatRenderer's hash-link router (already wired for #note-id) reaches
the new notes.openNote(id) helper, which force-closes/reopens the
Notes panel, polls for the target card, and runs a brief outline
flash so the user can locate it on long lists.
Three converging fixes so the chat agent + external Codex/Claude skills can actually debug a crashed serve instead of staring at a post-crash neofetch banner:
* Serves now `tee` to /tmp/odysseus-tmux/SESSION.log on the host running them. Runner saves fds 3/4 before the tee and restores them right before `exec ${SHELL}`, so the post-crash interactive zsh banner does NOT pollute the log file.
* `tail_serve_output` (chat agent) and `/api/codex/cookbook/output/{sid}` (Codex+Claude skills) both prefer the persistent log file over the tmux pane. Pane is fallback for sessions predating the tee runner. Default tail bumped 150 -> 400.
* `list_served_models` "recent log" snippet seeks to the Traceback line instead of showing the last 6 lines (which was always the bash prompt).
Cookbook auto-adoption sweep on `/api/cookbook/tasks/status`: every 20s (rate-limited) the cookbook SSHes each configured server, finds `serve-*` / `cookbook-*` tmux sessions running an actual model process (vllm/python/llama-server/etc., filtered via `pane_current_command`), and writes them into state.tasks. So when the agent falls back to raw ssh+tmux, the session appears in the Cookbook UI on the next poll.
`serve_model` error path now reads `data["detail"]` in addition to `data["error"]` so the FastAPI HTTPException message ("Invalid characters in cmd") actually reaches the agent instead of being swallowed as a generic "Serve failed". Tool description updated to warn against `cd …`/`source …`/`&&` prefixes.
Intent-without-action supervisor in agent_loop: when the model writes "Let me tail the output" / "I'll check the logs" / "Let me investigate" and ends the turn without emitting a tool call, the loop injects a sharp system nudge ("You said you would X — DO IT NOW") and continues. Capped at 2 nudges per chat so a model that genuinely cannot use the tool does not pin the loop.
Codex/Claude skill parity: adds `/cookbook/cached`, `/cookbook/presets`, `/cookbook/preset/{name}`, `/cookbook/adopt` so external agents have the same surface as the chat agent. SKILL.md docs + odysseus_api.py wrapper updated for both bundles.
`adopt_served_model` promoted to the always-on tool set so the agent has a documented fallback when serve_model rejects a cmd.
Also various cookbook UI tweaks accumulated alongside the above (cookbook.js, cookbookRunning.js, cookbookServe.js, cookbook-diagnosis.js, settings.js, style.css).
The blocklist prefixes had trailing slashes, so path.startswith() only
matched /api/tokens/{id} but not /api/tokens itself — the bare GET (list)
and POST (mint) endpoints were reachable via app_api. Same gap on
/api/users (list/create/delete). Drop trailing slashes so both bare and
sub-resource forms are blocked. /api/auth and /api/admin had no bare
endpoints today but get the same treatment to prevent future drift.
Caught by #1462.
* fix: MCP reconnect via tool passes only server_id to connect_server
connect_server requires name, transport, command, args, env, and url
but the reconnect path in do_manage_mcp only passed the server_id,
causing a TypeError on every reconnect attempt. Mirror the pattern
used in mcp_routes.py reconnect_server.
* test: verify MCP reconnect passes full server config to connect_server
Mocks the MCP manager and DB to assert that do_manage_mcp reconnect
passes name, transport, command, args, env, and url — not just the
server_id.
* fix: closed document no longer stays active and leaks into new chats (#1160)
Closing a document tab calls _detachDocFromSession: a doc with content is
PATCHed to session_id="" (unlinked, session_id -> NULL, is_active stays True),
an empty one is DELETEd. But the in-memory active-document pointer
(tool_implementations._active_document_id) was never cleared on either path.
The chat doc-injection last-resort looks up that pointer by id and injects it
when `not cand.session_id or cand.session_id == session`. An unlinked doc has
session_id NULL, so the stale pointer re-surfaced a closed document in later,
unrelated chats — the agent kept reading/suggesting edits to a doc the user
had closed.
Fix: add clear_active_document(doc_id) and call it when a document is unlinked
(PATCH session_id="") or deleted, so the pointer no longer resurrects a closed
document. clear_active_document only clears when the id matches (or no id), so a
different active doc is left untouched.
Covered by tests/test_active_document_clear.py (4 cases).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: add route-level regression for #1160 (detach/delete clears active doc)
Per review: prove the actual API path, not just the helper. Drives
PATCH /api/document/{id} (session_id="") and DELETE /api/document/{id}
through TestClient against a temp SQLite DB under real owner routing, and
asserts get_active_document() is cleared (and untouched when a different
document is closed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: make #1160 route regression hang-proof and dev-DB-independent
The route test could hang in other environments: it set DATABASE_URL at import
time, which is ignored if core.database was already imported, so it fell back to
the real dev DB and could contend for its locks (maintainer saw it hang, exit
124).
Rebind to a DEDICATED temporary SQLite engine (NullPool) and patch the document
route module's SessionLocal to it via an autouse fixture — so the test never
touches the dev DB and is independent of import order. Runs in ~0.3s.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: drive #1160 route regression without TestClient (fixes local hang)
The route test used Starlette TestClient (middleware app + threadpool), which
hung in the maintainer's environment. Rework it to call the async route handlers
directly — extracted from the router — with a minimal fake request against a
temp-SQLite-patched SessionLocal. Same real coverage (handler + DB + owner
routing), but it completes reliably (~0.3s) with no TestClient/threadpool.
Verified the maintainer's exact batch now passes:
pytest tests/test_document_close_clears_active_route.py \
tests/test_active_document_clear.py \
tests/test_document_tool_owner_scope.py -> 14 passed
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the reviewer requirement from PR #1190 review that was carried
over but not implemented in #1230:
> "The hard max is a function-local constant. For this setting, the ceiling
> should be configurable or at least represented as a named setting/default
> with tests."
— review on #1190#1230 shipped the adaptive auto-derivation but left `DEFAULT_HARD_MAX = 200_000`
as a hardcoded module constant in src/context_budget.py. Admins on premium
APIs with large context windows (kimi-k2 / minimax-m3 at 1M, etc.) can use
their full window today only by setting `agent_input_token_budget`
explicitly — which then takes them off the adaptive auto-path entirely.
## What this PR changes
- src/settings.py: register `agent_input_token_hard_max` in
DEFAULT_SETTINGS, default 200_000 (matches `DEFAULT_HARD_MAX`). Inline
comment documents the no-op semantics in the explicit branch.
- src/agent_loop.py: read the setting at the call site and pass it as the
`hard_max` kwarg of `compute_input_token_budget`. Defensive parsing —
missing / non-int / zero values fall back to `DEFAULT_HARD_MAX`, so a
misconfig cannot silently zero the budget.
- src/tool_implementations.py: three friendly aliases for `manage_settings`:
- "hard max" -> agent_input_token_hard_max
- "token budget cap" -> agent_input_token_hard_max
- "input budget cap" -> agent_input_token_hard_max
Plus the existing "token budget" -> agent_input_token_budget keeps a
matching shorter alias "input budget".
- tests/test_context_budget.py: 6 new tests on top of the existing 6:
- hard_max raises the auto ceiling (1M ctx + raised cap -> 85% of ctx)
- hard_max lowers the auto ceiling (128K ctx + 50K cap -> 50K)
- hard_max has no effect on the explicit branch
- DEFAULT_SETTINGS contains the new key
- manage_settings aliases are registered
- the live get_setting path returns the override value, and malformed
values fall back per the agent_loop defensive parsing
12 passed in 0.04s. No changes to the pure helper signature or semantics;
#1230's behavior is the default when the new setting is unset.
## How it lets users drop the explicit override
Before this PR, on a 1M-context model:
agent_input_token_budget = 900_000 (explicit) -> 900K [user override]
agent_input_token_budget = <unset> (auto) -> 200K [HARD_MAX]
After this PR, same model:
agent_input_token_budget = <unset>
agent_input_token_hard_max = 900_000
-> min(1M * 0.85, 900K) = 850K [auto, no override needed]
The explicit-override path keeps working unchanged for users who prefer it.
* fix: use SQL false() for owner-less document query (filter(False) raises in SQLAlchemy 2.x)
* test: owner-less document query doesn't pass a bare False to filter
The 'add' action runs due_date through parse_due_for_user (natural
language like 'tomorrow at 9am', plus user-tz anchoring for naive ISO),
but 'update' stored the raw value verbatim. A reminder edited with
natural language was saved as an unparseable literal the frontend's
new Date() can't read, so it never fired. Route update's due_date
through the same parser as add.
The agent's RAG tool selector retrieves manage_notes as relevant for
note / todo / reminder requests, but two gaps stopped it from actually
firing on local llama.cpp / vLLM endpoints:
1. FUNCTION_TOOL_SCHEMAS had no entry for manage_notes. Even when the
tool was marked relevant, no JSON schema was sent on the function
tools list, so native-function-calling models had nothing to call.
In practice the model would describe creating the note in prose
while the actual note stayed blank — the symptom reported in #713
("checklist hallucinated as blank").
2. _API_HOSTS only listed hosted providers (OpenAI, Anthropic, etc.).
For local endpoints like http://localhost:8080 or
http://host.docker.internal:8000, _is_api_model fell back to
keyword-sniffing the model name, so any model whose slug didn't
happen to match the keyword list silently lost native tool
schemas entirely.
Fixes:
- src/tool_schemas.py: add a manage_notes function schema covering
list/add/update/delete/toggle_item with the full Keep-style field
set. note_type is exposed as an enum ("note" | "checklist") so the
model picks the mode explicitly instead of inferring it from
content shape. Items are named checklist_items in the schema —
consistent with the description's wording and avoiding the
Python-built-in name clash that #713 calls out.
- src/tool_implementations.py: do_manage_notes accepts both
checklist_items (new, schema-exposed) and items (legacy /
internal). Direct API callers and existing code paths keep
working unchanged; native function calls following the new
schema route through the same path.
- src/agent_loop.py: add localhost, 127.0.0.1, and
host.docker.internal to _API_HOSTS so the function-tool path is
not gated behind model-name guessing for local servers.
Closes#174.
Closes#713.
read_skill_md and read_skill_reference walk all skill files via
_iter_skill_files and return the first match by slug, regardless
of owner. In a multi-user deployment where two users have skills
with the same slug under different categories, a caller scoped
to owner='alice' can read Bob's skill content.
This is the same cross-tenant leak class as the update_skill /
delete_skill fix (PR #755, merged), but on the read path.
Changes:
- read_skill_md / read_skill_reference accept owner= param (default
None = match ownerless only, matching the write-path convention).
- 7 callers updated: tool_implementations.py (view, view_ref, patch),
builtin_actions.py (test_skills), skills_routes.py (audit, source,
test routes).
- Tests: read scoping (alice reads hers, not bob's), positive update
scoping (alice can mutate her own), ownerless-match default.
SkillsManager.update_skill walks every SKILL.md on disk and matches by
slug only; the 'owner' key in its scalar_keys whitelist meant a caller
could pass updates={'owner': 'attacker', 'description': 'pwned'} and the
first matching file on disk got silently re-owned. Two users with the
same slug under different category directories (which is supported by
the on-disk layout <category>/<name>/SKILL.md) could each stomp the
other's skill via the manage_skills tool or the in-process callers in
tool_implementations.py (edit, patch, publish, delete).
update_skill and delete_skill now require the caller's owner and only
match a file whose parsed owner field matches. The default of None
means 'no scope' and only matches ownerless skills, so an unsafe call
without an explicit owner is now a no-op. 'owner' is also removed from
scalar_keys so the updates dict cannot be used to reassign ownership
even when the manager is called from an in-process path that didn't
supply the owner argument.
The in-process callers in tool_implementations.py are updated to pass
owner=owner (which was already in scope at every call site) so the
HTTP and agent paths both go through the scoped check. The HTTP route
at routes/skills_routes.py:1499 was already owner-scoped via
sm.load(owner=user); the fix brings the in-process path up to the
same standard.