Skip to content

Correct seven built-in catalog entries the OAuth audit found wrong - #1495

Closed
KristianYe wants to merge 1 commit into
mainfrom
fix/catalog-audit-urls
Closed

KristianYe wants to merge 1 commit into
mainfrom
fix/catalog-audit-urls

Conversation

@KristianYe

Copy link
Copy Markdown
Collaborator

What changed, and why

Data-only fixes to internal/clientconfig/builtin_remote_catalog.yaml from the #1006 catalog audit (fleet's own mcpoauth.Discover run against every official OAuth/tenant/open entry, 2026-09-13/14). Each entry was re-probed live with the corrected data and completes discovery.

entry what was wrong fix
expensify URL lacked the trailing slash; the vendor 308s /mcp/mcp/ and fleet refuses redirects, so Add failed https://www.expensify.com/mcp/
cartesia URL was the bare origin; it discovers (via the legacy-origin fallback) but every tool call 404s — measured live https://mcp.cartesia.ai/mcp
octagon the server's own metadata names mcp.octagonai.co as the resource; the entry's mcp.octagonagents.com serves the same server, but a token must be bound to the audience the vendor declares https://mcp.octagonai.co/mcp
globalping auth: open, but initialize now answers 401 with protected-resource metadata (self-registering, public-client AS) auth: oauth
zerodha-kite auth: oauth, but there is no OAuth at the transport — initialize answers 200 and no metadata exists; the user signs in through the server's login tool auth: open + a hint saying so
sage-intacct no dynamic registration, no none auth method, nothing told the user client_registration: manual + client_secret: required + setup hint; pinned in TestBuiltinRemoteCatalogClientSecretRequired
openrouter docs_url answers 404 https://openrouter.ai/docs/mcp

docs/CONNECTOR-ONBOARDING.md gains one clause naming Sage Intacct among the manual-client entries.

How I verified it

  • go test -tags fleet_host_executor ./internal/clientconfig/ (catalog shape, provenance, secret-required pin) — green on the rebased branch.
  • Live re-probe of the seven entries with the edited catalog: all complete discovery (Expensify via the POST pointer since GET on /mcp/ 301s to a help page; Octagon's issuer is login.octagonai.co; Sage Intacct's docs answer 403 to non-browser clients, the flags follow its metadata).
  • Cartesia was connected live from the directory with the old URL and failed at tool time with HTTP 404; with /mcp the vendor answers 401 with a metadata pointer.

Scope and deviations

🤖 Generated with Claude Code

Data-only fixes from running fleet's own discovery against every official
entry (#1006 audit, 2026-09-13/14), each verified live against the vendor:

- expensify: URL is /mcp/ — the vendor 308s the slash-less form and fleet
  refuses redirects, so Add failed.
- cartesia: the MCP endpoint is /mcp; the bare origin discovers (via the
  legacy origin fallback) but 404s every tool call (measured live).
- octagon: the server's own metadata names mcp.octagonai.co as the resource;
  the entry's mcp.octagonagents.com host serves the same server but a token
  must be bound to the audience the vendor declares.
- globalping: no longer open — initialize answers 401 with protected-resource
  metadata and a self-registering, public-client authorization server.
- zerodha-kite: no OAuth at the transport (initialize answers 200, no
  metadata anywhere); users sign in through the server's `login` tool → open,
  with a hint saying so.
- sage-intacct: no dynamic registration and no `none` auth method →
  client_registration: manual + client_secret: required, with a setup hint;
  pinned in TestBuiltinRemoteCatalogClientSecretRequired.
- openrouter: docs_url 404 → https://openrouter.ai/docs/mcp.

Not changed: square and smartlead document SSE-only endpoints, which fleet's
hosted-connector transport (streamable HTTP) cannot speak — a product
decision (hide, or keep with a warning) and possibly a transport gap, kept
out of a data-only change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bradflaugher added a commit that referenced this pull request Sep 14, 2026
Five review findings on #1501, each checked against the code.

The status record still called #1488 and #1495 open; #1488 is merged and
#1495 landed here. The runbook said scheduled runs mount every connected
connection, but BuildRemoteMCPOverlay caps the mount at maxOverlayServers (8)
for chat, scheduled runs and the broker alike, skipping the rest with a log
line, and mcp_selection pins do not lift the cap; it also said any connection
that cannot refresh is marked Reconnect needed, when network errors and 5xx
stay transient and only a terminal OAuth error sets needs_reauth.

Catalog corrections do not reach saved connections: the mount is built from
the saved row's URL and the directory marks an entry added by name, so a
Cartesia, Octagon or Globalping connection added before the fix keeps the
old definition until it is removed and re-added. That is now stated in
CONNECTOR-ONBOARDING.md and recorded as F13 with its deferral.

Zerodha Kite's hint claimed the login session lives on Kite's side. fleet
opens and closes a fresh MCP session per turn, so the login tool's session
does not outlive the turn that called it and a scheduled run cannot sign in;
the description, hint and status rows now say so.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bradflaugher added a commit that referenced this pull request Sep 14, 2026
…Auth pack record (supersedes #1495, #1496, #1499) (#1501)

## What changed, and why

One PR that supersedes the three open PRs, merged locally on top of
current `main` and driven to green together. Supersedes #1495, #1496 and
#1499; each of those is closed in favour of this one.

**Tags on the Operations Center board, and a tag filter** (from #1499).
Every task row and phone card shows its tags as chips; pressing a chip
filters the board by that tag, and the filter bar offers a tag dropdown
fed by a deployment-wide catalogue (`GET /api/orchestrator/tasks/tags`,
proxied to the backend) unioned with the tags on the listed tasks. The
catalogue is refreshed at most every five minutes off the dashboard's
ordinary reload cadence. Design note: `docs/TASK-TAGS.md`; both
Operations Center guide copies updated and `docs/USER-GUIDES.md`
corrected so it no longer says tags do not filter the board.

**Seven built-in catalog entries corrected** (from #1495). Data-only
fixes the #1006 OAuth audit found by running fleet's own discovery
against every official entry: expensify's trailing slash, cartesia's
`/mcp` endpoint, octagon's declared resource host, globalping now OAuth,
zerodha-kite open with a sign-in hint, sage-intacct manual registration
with a required client secret (pinned by test), openrouter's docs URL.
Square and smartlead (SSE-only) are deliberately not changed.

**The #1006 OAuth pack recorded** (from #1496).
`docs/MCP-CATALOG-STATUS.md` (live-run status table for twelve hosted
connectors, the fleet changes the pack produced, and a per-entry
appendix from re-probing all 231 official OAuth/tenant/open entries) and
`docs/HOSTED-CONNECTORS-RUNBOOK.md` (what an operator needs before and
after Connect, per vendor). Both indexed in `docs/README.md`.

**Review fixes on the merged tree** (new here, from a pre-PR review of
the combined diff):
- The tag catalogue's TTL effect was cancelled by any dashboard reload
finishing mid-flight, dropping a valid response and leaving the stamp
set, so the dropdown could stay empty for five minutes. Request lifetime
is now a generation counter, bumped on fetch start and on
deactivation/unmount (which also zeroes the stamp). Three regression
tests.
- Enter/Space on a focused desktop tag chip bubbled to the row handler,
which opened the log viewer and blocked the chip's activation. The row
handler now acts only when the row itself is the target. Regression
test.
- `docs/MCP-CATALOG-STATUS.md` overstated grantee isolation (a grantee's
run does refresh the owner's token row; revocation is checked at mount
time) and embedded the audit's probe tenant hostname in the spacelift
row, now a `<tenant>` placeholder.

**Codex GitHub review of this PR** (five P1 threads, all verified
against the code and fixed in `61f9b9df`): stale #1488/#1495
dispositions in the status record; the runbook now states the
eight-server overlay cap and that transient refresh failures do not mark
a connection for reconnect; saved connections keeping a pre-correction
URL is documented in `CONNECTOR-ONBOARDING.md` and recorded as finding
F13 (reconciliation deferred, remove and re-add); the Zerodha Kite
entry's hint no longer claims the login session persists across turns or
works in scheduled runs.

**Codex re-review** (three P2 runbook threads and one P3 security
finding, fixed in the third follow-up commit): `GET /tasks/tags`
returned the deployment-wide catalogue with counts to any signed-in
user; it is now scoped exactly like `ListTasks` (#1082 own-rows rule,
`view_tasks` required, SQL filter on `created_by` /
`created_by_key_id`), with `TestTagCatalogueScope` and
`TestTagCatalogueAuthz`. The design note and both Operations Center
guide copies describe the scoped catalogue. The runbook no longer
implies a Kubernetes deployment has an Ingress or prescribes where TLS
terminates, names 128 tools as the default disclosure threshold with its
two overrides, and distinguishes terminal from transient refresh
failures in troubleshooting.

**Third Codex pass** (five threads, fixed in the fourth follow-up
commit): the phone card's keyboard focus ring was clipped by the list
item's `overflow: hidden` and is now drawn on the list item via
`:has()`; the guide discloses the catalogue's five-minute refresh bound;
the runbook stops telling operators to keep the OAuth encryption key
with the database backups, says only user-enabled connections mount in
scheduled runs (a disabled one is omitted silently), and qualifies the
401 catalog check as the OAuth shape only.

## How you verified it

Local Postgres (podman `postgres:18`) with the three DSN vars set, so
the DB-backed packages ran rather than skipped:

- `make build`: clean.
- `scripts/go-test.sh --count=1` (the `make test` path): 63 packages
`ok`, 0 failures; `internal/store` 12.7s, `internal/httpapi` 22.0s,
`internal/runner` 7.7s confirm the DB suites executed.
- `make lint`: golangci-lint 0 issues; ruff check and format --check
clean; migration lint 0 files.
- `go test -tags fleet_host_executor ./scripts/
./internal/clientconfig/...`: PASS (docs-index check, guide sync check,
catalog tests including the new `sage-intacct` pin).
- `make sync-guides`: no drift between the two guide copies.
- `make ci-web`: npm audit 0 vulnerabilities; oxlint 0 warnings/errors
on 532 files; typecheck clean; vitest 153 files / 1614 tests passed (4
new); `next build` 74/74 pages.
- `make ci-e2e-mocked`: 98 passed.
- `gitleaks git --log-opts=origin/main..HEAD`: 5 commits scanned, no
leaks.
- Each of the four new regression tests was confirmed to fail against
the code it replaces.
- After the docs/catalog follow-up (`61f9b9df`): `go test -tags
fleet_host_executor -count=1 ./scripts/ ./internal/clientconfig/...`
PASS (docs index, catalog decode and pins); `make lint` clean.
- After the tag-catalogue scoping fix: `scripts/go-test.sh --count=1`
against Postgres again, 63 packages `ok`, 0 failures;
`TestTagCatalogueScope` and `TestTagCatalogueAuthz` print `PASS` (not
`SKIP`) under `-v`; `make lint` clean; `make sync-guides` no drift; `go
test ./scripts/` PASS; `make ci-web` green (1614 tests, build 74/74).
- After the focus-ring and runbook fixes: `make ci-web` green again
(1614 tests, 74/74), `make sync-guides` no drift, `go test ./scripts/`
PASS, `make lint` clean, oxlint clean.
- Each fix was reviewed locally by Codex before pushing.

Reviewed twice by Codex before opening (the four findings above plus two
residual ones on the first fix, all addressed).

## Scope and deviations

The three PR branches are merged unchanged (merge commits, then one fix
commit); the only conflict was two bullets added to the same spot in
`docs/README.md`, both kept. The Codex threads open on #1499 were
addressed by its own follow-up commits (`df1b7e2e`, `96ad1806`) and
verified here. Not done: square and smartlead catalog entries (SSE-only,
a product/transport question per #1495); the catalogue is
deployment-wide while the board is scoped, so no tag counts are shown
(`docs/TASK-TAGS.md`).

---

- [x] The title and "What changed, and why" are written for the release
notes they become
- [x] A design note (`docs/TASK-TAGS.md`) added, if this ships a feature
- [x] An ADR added or superseded in `docs/adr/`, if this adds, weakens
or reverses an invariant — none touched
- [x] The diff is scoped to one change (no unrelated refactors) — three
related changes consolidated deliberately, see above

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Kristian Yendrek <yendrek.kristian@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@bradflaugher

Copy link
Copy Markdown
Contributor

Superseded by #1501, which merges this branch unchanged (merge commit 5ce04d28) together with #1496 and #1499, and adds one follow-up: the Zerodha Kite description and setup hint no longer claim the login session lives on Kite's side, since fleet opens a fresh MCP session per turn. Closing in favour of #1501.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants