Skip to content

refactor(api): remove nine uncalled routes and the orphan observability store (#3434) - #3483

Merged
vybe merged 10 commits into
devfrom
feature/3434-uncalled-routes
Oct 11, 2026
Merged

vybe merged 10 commits into
devfrom
feature/3434-uncalled-routes

Conversation

@vybe

@vybe vybe commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #3434. One of five independent stale-code PRs off dev. Draft. Squash-merge only — see Before merge.

What

  • Nine API routes removed, none with a caller in the frontend, the enterprise submodule, the MCP server, the scheduler, the agent image, the CLI, scripts, CI workflows or any compose/Dockerfile health check:
    • Internal router (gated by the internal-secret dependency): GET /api/internal/health, POST /api/internal/activities/track, POST /api/internal/activities/{id}/complete, POST /api/internal/agent-files/share, GET /api/internal/agents/{name}/sync-health-status.
    • Duplicates of the live GET /api/activities/timeline: GET /api/agents/{name}/activities, GET /api/agents/activities/timeline.
    • GET /api/observability/metrics and GET /api/observability/status — routers/observability.py is deleted. GET /api/ops/costs serves a superset of the same data from the same parser; the two parse helpers moved into services/ops_costs_service.py byte-identical, with a unit test, which also removes a service→router import and a duplicated set of OTEL constants.
  • The three request models only those routes used are deleted; the route census and the human-only baseline shrink to match (359 → 355 entries, shrink only).
  • Frontend: stores/observability.js deleted (no component or view used it); utils/contrast.js moved to tests/unit/helpers/ (only its own spec imported it); two specs trimmed.
  • Tests: new tests/unit/test_3434_removed_routes.py pins the nine routes absent from the census and the runtime route table. tests/test_internal.py, tests/test_observability.py and test_ent706_internal_sync_health.py are deleted with the routes they exercised. test_1808 is re-pinned to the threshold constant in sync_freeze_policy.py.
  • Docs: api-endpoints.md, the architecture and feature-flow docs, and the user docs that named the observability routes now point at /api/ops/costs. Five places that described the internal router as "(no auth)" are corrected: it is gated by the router-level verify_internal_secret dependency.

Rulings carried (operator, recorded on the issue 2026-10-09)

  • The observability router is removed, not kept as an API-only surface; docs repointed in this PR.
  • The dead request models go here rather than in the dead-symbols PR.
  • The integration tests that called GET /api/agents/{name}/activities are rewritten against GET /api/activities/timeline with client-side agent_name filtering.
  • Correction to the original issue text: the internal router was never unauthenticated. The gain here is a smaller secret-gated surface, not a closed hole.

Review + security

/review (reading review, report-only): MERGEABLE AFTER FIXES — the one required fix (the "(no auth)" wording) is in 673101ab. Independent re-proof of no caller for all nine routes, including string-built URL forms; the observability removal is behaviour-preserving; the census, baseline and OWN_AUTH table agree mechanically with the removed routes; the new guard test is proven red against dev's tree. /cso --diff: no findings; net −9 routes, no gate on a surviving route changes.

Tests

  • Backend: 10 targeted unit files, shuffled (seed 12345), one per process — all green (204 tests). test_2996_owner_config_person_only timed out locally at 240 s with no failure and is left to CI.
  • Frontend: npm run test:unit 304 files / 5,141 tests passed; npm run build succeeds.
  • Backend imports with 665 routes (OSS-only build); compileall clean.
  • git merge-tree against dev (21b4a11): clean.
  • Not run here: the full unit island (CI), and the four rewritten integration files — tests/test_activities.py (27), tests/test_parallel_task.py (25), tests/test_access_control.py (41), tests/journeys/ via conftest.py (19) — which compile and collect but need a live backend.

Before merge

Handoffs

  • get_freeze_schedules_if_sync_failing (facade + implementation) has no caller once the sync-health-status route is gone; tests/test_execution_termination.py calls a route that does not exist. Both noted on refactor: remove about 50 backend and scheduler symbols that no production code references #3433.
  • PHASE_14_OPENTELEMETRY.md still describes a Dashboard panel removed earlier; the decrypt-and-inject rows in api-endpoints.md and credential-injection.md describe a route no longer in internal.py (only their auth wording was corrected here).

🤖 Generated with Claude Code

Trinity Agent (trinity) and others added 5 commits October 9, 2026 11:52
…outer (#3434)

Nine routes had no caller in any client this repo ships (frontend, MCP
server, scheduler, agent server, CLI, scripts, healthchecks):

- five internal routes, all behind the router-level internal secret
  (X-Internal-Secret): GET /api/internal/health,
  POST /api/internal/activities/track,
  POST /api/internal/activities/{id}/complete,
  POST /api/internal/agent-files/share,
  GET /api/internal/agents/{name}/sync-health-status
- GET /api/agents/{name}/activities and GET /api/agents/activities/timeline
  (live equivalent: GET /api/activities/timeline)
- routers/observability.py, GET /api/observability/{metrics,status}
  (only caller was an orphan frontend store; live equivalent:
  GET /api/ops/costs). Its two OTel parse helpers move unchanged into
  services/ops_costs_service.py, removing a service->router import.

Also: the three request models of the removed routes, the census
OWN_AUTH entries and four human-only baseline entries
(FROZEN_BASELINE_COUNT 359 -> 355), tests/test_internal.py,
tests/test_observability.py, unit/test_ent706_internal_sync_health.py
(sync_view is covered by ent707), and the access-control pair. Live-tier
callers of the per-agent activities route (test_activities.py,
test_parallel_task.py, journeys/conftest.py) now read the timeline and
filter by agent client-side, as the MCP tool does; py_compile only here.
test_1808 re-pinned to the canonical sync_freeze_policy module.

New unit/test_3434_removed_routes.py went red at HEAD (5 failed) and is
green now; re-adding internal_health turns its AST check red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3434)

Completes the previous commit, which landed only the staged file
deletions (a failed `git add` pathspec on an already-deleted file).

Nine routes had no caller in any client this repo ships (frontend, MCP
server, scheduler, agent server, CLI, scripts, healthchecks):

- five internal routes, all behind the router-level internal secret
  (X-Internal-Secret): GET /api/internal/health,
  POST /api/internal/activities/track,
  POST /api/internal/activities/{id}/complete,
  POST /api/internal/agent-files/share,
  GET /api/internal/agents/{name}/sync-health-status
- GET /api/agents/{name}/activities and GET /api/agents/activities/timeline
  (live equivalent: GET /api/activities/timeline)
- routers/observability.py, GET /api/observability/{metrics,status}
  (live equivalent: GET /api/ops/costs). Its two OTel parse helpers move
  unchanged (AST-identical) into services/ops_costs_service.py, removing a
  service->router import; main.py no longer registers the router.

Also: the three request models of the removed routes, the census
OWN_AUTH entries and four human-only baseline entries
(FROZEN_BASELINE_COUNT 359 -> 355), the access-control pair, the ent706
registry entry. Live-tier callers of the per-agent activities route
(test_activities.py, test_parallel_task.py, journeys/conftest.py) now read
the timeline and filter by agent client-side, as the MCP tool does
(py_compile only here). test_1808 re-pinned to the canonical
sync_freeze_policy module.

New unit/test_3434_removed_routes.py went red at the base (5 failed) and
is green now; re-adding internal_health turns its AST check red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
)

The orphan stores/observability.js (no importer under src/) and the
move of utils/contrast.js to tests/unit/helpers/contrast.js landed in
245bb29; this updates their specs. contrast.spec.js imports the helper
from its new home. refreshTreatment.spec.js drops the store half (axios
mock, pinia, the store describe block and its source read) and keeps every
DashboardPanel.vue assertion; its source-read count stays at the baseline.

npx vitest run (both specs + sourceTextRatchet): 3 files, 40 passed.
npm run test:unit: 304 files, 5141 passed. npm run build: built.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3434)

Repoints every live doc that named GET /api/observability/{metrics,status}
to GET /api/ops/costs (user docs agent-logs.md, GEMINI_SUPPORT.md,
PHASE_14_OPENTELEMETRY.md, the OpenTelemetry feature flow) and describes
the two responses as equivalent, not identical. Drops the removed internal
routes (all were secret-gated) and the per-agent activity reads from the
architecture catalogs, feature flows (activity stream, scheduling, agent
lifecycle, file sharing, effect idempotency, git sync health, execution
queue, internal system agent, testing agents) and requirements; points the
design-system contract at tests/unit/helpers/contrast.js; updates the
test_2726 docstring, whose JS twin went with the store.

No unreleased release-notes file exists in the repo, so no release line
was added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trinity-ability and others added 2 commits October 10, 2026 09:45
Conflicts: docs/testing/phases/PHASE_14_OPENTELEMETRY.md (kept dev's
retired-phase rewrite from #3437, repointed its two mentions of the
removed /api/observability routes at /api/ops/costs) and
tests/registry.json (rebuilt from git stages: dev's file minus the
entry this branch removed plus the entry it added).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t just caused (#3434)

GET /api/activities/timeline has no agent filter, so the rewritten helpers
read the newest 500 rows of the whole fleet and filter by agent client-side.
The tests that compare before/after for one agent could miss their rows on a
busy stack, and a count comparison stops moving once the window is saturated.

Take the newest row's created_at as a watermark before the action and pass it
back as start_time, which the route does support. The bound comes from the
backend's clock, so host/container clock drift does not matter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@github-actions

Copy link
Copy Markdown

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

# Conflicts:
#	docs/memory/architecture.md
#	tests/registry.json
@vybe
vybe marked this pull request as ready for review October 11, 2026 16:37
trinity-ability and others added 2 commits October 11, 2026 17:55
execution-queue.md: both branches rewrote the scheduler's dispatch bullets;
combine into one bullet carrying #3472's dispatch path and this branch's
in-process activity tracking.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@trinity-ability trinity-ability 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.

Reviewed: no live caller of the nine removed routes or the observability store on dev (incl. #3472/#3484 as landed), the frontend, MCP or enterprise; access unchanged (old metrics route was admin-only, /api/ops/costs is admin or ops). Route census shrinks by exactly the removed routes. Frontend 5,586 tests, guard + scheduler suites pass on the merged tree; CI fully green.

@vybe
vybe merged commit 474e00c into dev Oct 11, 2026
27 checks passed
dolho added a commit that referenced this pull request Oct 12, 2026
…ity-enterprise#629)

Merge-train review (2026-10-08, 2026-10-10): the narrowed gate revoked the
cross-agent reach that ent#164 capabilities exist to grant. A
`schedules.manage` holder got 404/403 on a sibling's schedule routes, an
`instructions.manage` holder on its instruction files and git reset, and an
`agents.manage` holder on `PUT /{name}/model`, because `agent_may_reach`
never saw which capability the route had checked.

- The two capability fences (`capability_fence`,
  `require_person_or_capability`) record the target they admitted on the
  request's principal (`User._capability_reach`, a private attribute), after
  the grant check and the owner bound. `agent_may_reach` honours that record
  on both tiers. This replaces `_manages_owners_agent`, which widened every
  owner check for an `agents.manage` holder on every route.
- `files._may_reach_for_path`: the path exception covers `instructions.manage`
  as well as `skills.manage`, needs the grant for each file class the path
  touches, and is bounded to agents the owner OWNS (`owner_owns_agent`). It
  used to reach any agent the owner could access.
- `loops._check_loop_access`: reach is checked before the admin and
  initiator shortcuts, so an agent key cannot read or stop a sibling's loop
  by id.
- Operator queue: the spawned set is one query
  (`list_agents_spawned_by_key`), not two lookups per held agent.

Also merges dev: the agents.py activity routes stay deleted (#3483), and
access.ts takes dev's owner constants alongside this PR's enforce rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Oct 13, 2026
… it spawned — on the raw backend routes too (trinity-enterprise#629) (#3346)

* fix(auth): an agent key reaches only itself, its edges and the agents it spawned — on the raw backend routes too (trinity-enterprise#629)

An agent-scoped MCP key resolves to its OWNER carrying the owner's role
(Invariant #8), so every access/owner predicate said yes for any sibling the
owner holds — on an admin-owned install, every agent. The agent_permissions
edge the MCP layer checks (P-02) did not exist one hop down: with its injected
key and TRINITY_BACKEND_URL, agent A could chat with, task, loop, read the
history and logs of, or replace the git token of any sibling B. Ruling
(operator, 2026-10-06): fix the hole.

Narrowed in the shared gate, not per endpoint (the #1890 lesson):

- dependencies.agent_may_reach(current_user, target, manage=):
  * use tier — get_authorized_agent[_by_name], assert_agent_access, A2A
    inbound: itself, an edge target, an agent it spawned with its current key;
  * manage tier — get_owned_agent[_by_name], assert_agent_owner: itself or an
    agent it spawned. An edge means "may call", never "may manage".
  Outside its reach reads as the uniform 404 (#186). Every other principal is
  unchanged. The edge read fails closed, like the MCP layer's.
- get_skill_managed_agent_by_name keeps the plain owner fence: skills.manage
  (ent#596) IS the grant to manage the owner's other agents.
- Every inline can_user_access_agent / can_user_share_agent call (16 files)
  ANDs the reach in; nevermined's admin shortcut no longer skips it.
- The operator queue had its own predicate (admin role → no filter): an agent
  key now always gets a set narrowed to its reach.
- MCP: every `baselined: ENT629` row in TOOL_ACCESS_POLICY is `enforce` on its
  agent parameter; the "tool-surface gate only" notes now name the backend fence.

Collection reads (the agent list, event-subscription list, MCP key list) are
not narrowed: they disclose names, never act on an agent.

Tests: tests/unit/test_ent629_agent_reach.py — the reach truth table on a real
schema, every helper, A2A inbound, the operator queue, the skills capability
keeping its reach, the dispatch routes resolving through the narrowed gate,
and an AST guard that fails the build on an inline access check without the
reach (exemptions carry their reason). Mutation-checked: reach always-true,
edge-as-manage, the access dependency dropping it, and an inline site
dropping it each turn tests red.

Docs: Invariant #8, security.md §5, P-02, the J10 docstring, mcp-server.md
and the affected feature flows and requirements.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): keep skills.manage's owner-wide reach through the files tab; regression fixes (trinity-enterprise#629)

From the regression run over every unit file touching a changed module
(14 new failures vs dev, all traced):

- The skill fence's owner check is its own seam again,
  `get_capability_owned_agent_by_name` (not narrowed: the capability is the
  grant), so the ent#596 recorder patches a named function.
- files.py: a skills.manage holder writing a sibling's skills path reaches it
  without an edge (`_may_reach_for_path`); every other path is narrowed. The
  capability gate still decides the write.
- share_agent_file checks "an agent key shares only its own volume" before the
  (now narrowed) owner gate, so a sibling gets the specific refusal.
- The depth suites (#2806, #2973) seed the A→B edge their sibling calls need.
- The edge read fails closed (a fixture schema without agent_permissions
  refused instead of raising — the enterprise assignment suites).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(operator-queue): an agent key's queue scope is {self} ∪ edges ∪ spawned, read directly (trinity-enterprise#629)

dev's ent#611/#715/#815 queue suites (29 tests) went red after the merge:
an admin-owned agent key's scope was built from the owner's
accessible-agent list and then filtered agent by agent, so a self or
permitted agent without an ownership row (every one in those fixtures)
dropped out. The scope now starts from {self} ∪ the agent's edges — the
MCP's own rule — bounded by a non-admin owner's set, plus the owner's
agents it spawned. The edge read fails closed. The router also gets the
logger it was missing.

ent#715's heads-ups fixture seeds the agent key's edge to the agent it
reads, so the test still checks that person rows are withheld, not that
the whole queue is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): a capability earns reach per request, not per holder (trinity-enterprise#629)

Merge-train review (2026-10-08, 2026-10-10): the narrowed gate revoked the
cross-agent reach that ent#164 capabilities exist to grant. A
`schedules.manage` holder got 404/403 on a sibling's schedule routes, an
`instructions.manage` holder on its instruction files and git reset, and an
`agents.manage` holder on `PUT /{name}/model`, because `agent_may_reach`
never saw which capability the route had checked.

- The two capability fences (`capability_fence`,
  `require_person_or_capability`) record the target they admitted on the
  request's principal (`User._capability_reach`, a private attribute), after
  the grant check and the owner bound. `agent_may_reach` honours that record
  on both tiers. This replaces `_manages_owners_agent`, which widened every
  owner check for an `agents.manage` holder on every route.
- `files._may_reach_for_path`: the path exception covers `instructions.manage`
  as well as `skills.manage`, needs the grant for each file class the path
  touches, and is bounded to agents the owner OWNS (`owner_owns_agent`). It
  used to reach any agent the owner could access.
- `loops._check_loop_access`: reach is checked before the admin and
  initiator shortcuts, so an agent key cannot read or stop a sibling's loop
  by id.
- Operator queue: the spawned set is one query
  (`list_agents_spawned_by_key`), not two lookups per held agent.

Also merges dev: the agents.py activity routes stay deleted (#3483), and
access.ts takes dev's owner constants alongside this PR's enforce rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(mcp): a backend refusal of the loop read is the uniform loop refusal (trinity-enterprise#629)

J10 `test_without_permission_an_agent_cannot_read_or_stop_a_loop_on_another`
failed on journey-smoke. Since the backend checks an agent key's reach before
the admin shortcut in `_check_loop_access`, `GET /api/loops/{id}` answers 403
for a loop on an agent outside the caller's reach. `get_loop_status` and
`stop_loop` resolve the loop through that read before their own gate, so the
caller got a raw `API error (403)` instead of "Loop '<id>' not found or not
accessible" and the hint.

A 403 on the read now returns the same uniform body as the MCP gate, with the
same log line and audit row. The stop is still never sent. A 404 (no such
loop) keeps the backend's own shape, as the existing test pins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(credentials): an agent key gets the human-only refusal before the owner fence (trinity-enterprise#629)

The backend regression diff caught 4 new failures in
`test_3435_credential_tool_gates::test_agent_key_is_refused_human_only_even_for_its_owner`.
With the narrowed owner fence, `get_owned_agent_by_name` answers an agent key
on a sibling with the uniform 404, so the handler-body
`reject_agent_principal` never ran. The agent was told "Agent not found"
instead of the rule it hit.

`dependencies.human_only` is the route-level form of `reject_agent_principal`.
On export and import it now runs before the ownership dependency. The refusal
depends on the principal alone, before any lookup, so it discloses nothing
about the target (#186). The inline call stays as belt-and-braces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(auth): one owner bound per capability, a DB-decided MCP edge, and the #3346 review (trinity-enterprise#629)

Addresses webmixgamer's review of f9cd838.

1. `skills.manage` reached another user's agent through the skills routes on
   an admin-owned install. `get_skill_managed_agent_by_name` went through the
   unnarrowed owner fence, which `can_user_share_agent` answers True for any
   agent when the owner is admin; the files tab refused the same target. The
   skills fence now applies the bound every capability fence applies (grant,
   then `owner_owns_agent`, then record the reach) and returns the narrowed
   `get_owned_agent_by_name`. `can_manage_agent_skills` takes the same bound.
   The skill-gate writes use `get_owned_agent_by_name` (`_gate_writer`
   records first), so `get_capability_owned_agent[_by_name]` and
   `_owned_agent_or_404(narrow=False)` are gone: one mechanism.
2. The MCP `enforce` gate read the lenient permissions list, which is built
   from the Docker-filtered fleet. A ghost with no container, a child being
   recreated, a spawned child whose edge was removed, and every call during a
   Docker fault all read as "not permitted". New
   `GET /api/agents/{caller}/reach/{target}` answers from the edge or the
   spawn record in the database; `TrinityClient.isAgentPermitted` reads it
   and fails closed.
3. security.md no longer says collection reads "disclose names, never act":
   fleet-level routes still answer an agent key with its owner's reach, some
   of them write, and some return task text and emails. A private follow-up
   tracks them.
4. Tests for the review's surviving mutations:
   - credential export's `human_only`;
   - the queue's non-admin bound;
   - `deleted_at` in the spawned set;
   - a schedules holder through the real route via TestClient (same principal
     object for the fence and the path dependency);
   - another user's agent on the skills fence;
   - the reach endpoint.
   Each mutation now fails a test.
5. An agent key never gets the admin "every user" chat history or sessions
   view: "may call" is not "may read everyone's conversations with it".

Also: learnings fragment (capability bound in one helper).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(ent754): the gate-map world records who owns the gated agent (trinity-enterprise#629)

The backend regression diff caught
`test_probe_is_a_persons_even_for_a_key_that_may_manage_skills[agent_key_with_skills_manage]`.
Since the #3346 review, an agent key's `skills.manage` reaches only agents its
owner OWNS (`owner_owns_agent`), not every agent `can_user_share_agent`
admits. The fixture stubbed "may share" but never said who owns
`gated-agent`, so the bound read "unknown owner" and refused. The test's
premise is unchanged: the key's owner owns the agent, so the manage check
admits it and the probe is still refused for a key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants