Skip to content

Add workspace API endpoints and regression coverage to api_server - #7866

Open
mingefei-cloud wants to merge 2 commits into
NousResearch:mainfrom
mingefei-cloud:restore-stash-20260411
Open

Add workspace API endpoints and regression coverage to api_server#7866
mingefei-cloud wants to merge 2 commits into
NousResearch:mainfrom
mingefei-cloud:restore-stash-20260411

Conversation

@mingefei-cloud

Copy link
Copy Markdown

Summary

This PR expands the API server with workspace-facing endpoints and adds regression coverage for the new behavior.

What’s included

Workspace API endpoints

Add /api/* routes for:

  • sessions list/create/get/update/delete
  • session messages
  • session chat
  • session streaming chat
  • session search
  • session fork
  • skills list/detail/category lookup
  • memory read
  • config read/update
  • cron job management

Behavior fixes and guardrails

  • Make /api/sessions return a total that matches the applied source filter
  • Return 409 title_conflict when session title creation/update collides
  • Return 409 ambiguous_skill_name when a skill slug exists in multiple categories without an explicit category
  • Resolve short session IDs before workspace chat / streaming chat uses session history
  • Harden config patching:
    • reject non-object PATCH payloads with 400
    • ignore unsupported top-level keys
    • use atomic YAML writes
    • preserve key order on write

Regression coverage

Add focused tests for:

  • workspace sessions behavior
  • skill ambiguity handling
  • config redaction and patch semantics
  • session chat behavior
  • SSE streaming chat behavior

Validation

Ran:

env -u API_SERVER_CORS_ORIGINS pytest -q tests/gateway/test_api_server.py tests/gateway/test_api_server_jobs.py tests/gateway/test_api_server_workspace.py tests/gateway/test_api_server_workspace_extra.py tests/gateway/test_sse_agent_cancel.py

Result:

  • 155 passed

Notes

This API is intended as a workspace-facing surface for Hermes workspace/front-end flows, not as a fully generalized third-party public API contract.

Follow-ups

Possible follow-up improvements:

  • add structured error codes/messages across all workspace endpoints
  • validate query params like limit / offset with 400 instead of falling through to 500
  • expand endpoint coverage further as frontend contract evolves

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels Apr 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Overlaps with open #8772 — both add workspace /api/* endpoints to api_server.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the workspace API coverage. The session-control work has since been independently implemented on current main (gateway/platforms/api_server.py:1693-2059, registered at :4777-4785), so this needs a focused salvage rather than a direct cherry-pick.

Problems

  • The added workspace handlers are not registered in the PR's production connect() route table. Inspection of PR head finds only /api/jobs registrations at gateway/platforms/api_server.py:2540-2547; the new handlers beginning at :1153 are unreachable. The added tests manually mount handlers, so they do not catch this.
  • The new state access hardcodes Path.home() / ".hermes" (gateway/platforms/api_server.py:1684, :1846, :1872, :1903). This breaks named-profile isolation; state paths must use get_hermes_home().
  • Current capabilities explicitly advertise admin_config_rw: false and memory_write_api: false (gateway/platforms/api_server.py:1502-1505), so the config/memory portion needs an explicit current-contract decision.

Suggested changes

  • Re-scope onto current main, register accepted routes in connect(), and add a production-route integration test.
  • Replace hardcoded state paths with get_hermes_home() and test an active profile.

Automated hermes-sweeper review.

return hermes_dir
return None

async def _handle_list_skills(self, request: "web.Request") -> "web.Response":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This handler (and the other new workspace handlers) is never registered on the PR's production connect() route table: PR-head inspection finds only /api/jobs registrations at lines 2540-2547. The tests manually mount handlers, so these endpoints would be unreachable after starting APIServerAdapter.

import yaml
from utils import atomic_yaml_write

config_path = Path.home() / ".hermes" / "config.yaml"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use get_hermes_home() / "config.yaml" here. Path.home() / ".hermes" writes the default profile even when the API server runs with a named HERMES_HOME; the same profile-isolation issue also exists in the new skill and memory handlers.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants